fix(harness-init): bound the status poll loop instead of retrying forever (#5157) - #5276
Conversation
…ever (tinyhumansai#5157) `HarnessInitOverlay` polled `openhuman.harness_init_status` every 2s and, on *any* failure, rescheduled unconditionally for the life of the window — no cap, no backoff. Against a core that does not serve the method this became a permanent 30-calls-per-minute loop, and since the core records every miss it produced 64,715 Sentry events (~9k/day) from a single client (CORE-RUST-1PY). The method is not retired: it is a live controller tagged `DomainGroup::Platform`. It legitimately misses on client/core surface skew — an older core behind a newer UI bundle, a runtime `DomainSet` without `Platform` (e.g. `DomainSet::harness()`), or a slim feature build. That first miss is expected; the 64,714 that follow are the client refusing to accept a permanent answer. - classify `unknown method: ` responses as a new `method_not_found` RPC error kind, prefix-anchored to mirror `dispatch::unknown_method_name`'s `strip_prefix`, and expose `isMethodNotFoundCoreRpcError` so pollers branch on `kind` rather than a message regex - stop the overlay poll on `method_not_found` (permanent — retrying an absent method can never succeed), and cap other failures at 5 attempts with 2s→30s exponential backoff so no fault can poll forever - keep the cold-start retry the loop exists for: the failure budget resets on the first success Also corrects the `KNOWN_PROBE_METHODS` rationale, which listed the method as a retired call with no canonical handler. It is served, so that note invited deleting a live controller — and because the allow-list makes the miss debug-only, a genuine regression would have gone silent in Sentry. Pins the registration with a test so the regression fails loudly instead. Regression coverage: without the overlay fix the new tests observe 61 calls in the same window where the fix makes 1 (absent method) and 5 (persistent fault).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9700acace
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…budget The retry cap added for tinyhumansai#5157 could strand the blocking overlay. `shouldShow` includes `running`, so once a run is in progress the overlay covers the app; if the core then had a transient outage lasting more than MAX_TRANSIENT_FAILURES attempts, the loop gave up and left that `running` snapshot on screen with stale progress for the rest of the session — even after the core recovered and the run reached `done`. The pre-tinyhumansai#5157 loop recovered from exactly that, so this was a regression the bound introduced. Split the give-up decision by whether anything blocking is displayed: - nothing on screen (the tinyhumansai#5157 case — a core that never serves the method, no UI, a silent 30-calls-per-minute loop): give up at the cap, unchanged; - a `running` overlay on screen: keep watching, but drop to STALLED_POLL_MS (30s). That is 2 calls/min — 15x below the runaway loop tinyhumansai#5157 fixed — and it only runs while a blocking overlay is actually up. `awaitingTerminalRef` mirrors that condition so the failure branch can read it without re-running the effect, and `isBlockingSnapshot` is now shared between the poll loop and the render path so the two cannot drift. Regression test drives a live run, then an outage well past the budget, then recovery, and asserts the overlay observes the terminal snapshot and clears itself. Verified failing before the fix: polling stopped at 6 calls and the overlay stayed on screen. Addresses the Codex review on tinyhumansai#5276.
There was a problem hiding this comment.
M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
HarnessInitOverlaystatus poll loop, which retried a failing RPC every 2s forever with no cap and no backoff — the source of 64,715 Sentry events (~9k/day) from a single client (CORE-RUST-1PY).method_not_foundCoreRpcErrorkind so pollers can recognise a permanent RPC miss and stop, instead of treating it as transient.KNOWN_PROBE_METHODSrationale insrc/core/dispatch.rs, which describedharness_init_statusas a retired method — it is live and served.Problem
HarnessInitOverlaypollsopenhuman.harness_init_statusevery 2s. On any error it calledsetTimeout(poll, POLL_MS)unconditionally — no attempt cap, no backoff, and no concept of a failure that can never succeed. Against a core that does not serve the method this became a permanent 30-calls-per-minute loop for the life of the window, and because the core records every unknown-method miss it produced 64,715 events.The issue's framing is inverted, and it changes the fix.
harness_init_statusis not a removed RPC. It is a live, registered controller (harness_init::all_harness_init_registered_controllers, taggedDomainGroup::Platform, covered byjson_rpc_harness_init_status_returns_snapshot_envelope). It misses only when caller and core disagree about the served surface:0.57.5;harness_initlanded in0.58.0via feat(harness_init): eager first-run setup with init overlay; pin managed Python to 3.13 #4021),DomainSetwithoutPlatform— e.g.DomainSet::harness(),Those are legitimate configurations, so the first miss is expected and correct. The amplification from 1 to 64,715 is entirely client-side.
This also means the existing server-side allow-list entry (added in #5171) cannot have fixed the reported events: the affected cores are
0.57.5, versions that shipped long before that allow-list existed.Solution
Client — stop the amplification at its source.
classifyRpcErrormaps anunknown method:response to a newmethod_not_foundkind, prefix-anchored to mirrordispatch::unknown_method_name'sstrip_prefix, so the two classifiers cannot drift. Exposed asisMethodNotFoundCoreRpcErrorso callers branch onkindrather than a message regex — which is what that module's own doc-comment already requires.method_not_foundand renders nothing: a core withoutharness_inithas no init run to report.MAX_TRANSIENT_FAILURES(5) attempts with exponential backoff capped at 30s, so even an unforeseen persistent fault decays and stops rather than running forever.Core — correct a misleading note and close the observability hole it opened.
The
KNOWN_PROBE_METHODScomment listedharness_init_statusalongside genuinely retired calls, as a "retired feature call ... when no safe canonical handler exists". It is served, so that note invited deleting a live controller. And because the allow-list makes the miss debug-only, a genuine regression — the controller dropped from the registry — would have gone completely silent: no error, no warn, no Sentry event. The comment now states the real reason (surface skew), andharness_init_status_is_registered_in_a_full_buildpins that the method really is served.Tradeoff considered: removing the allow-list entry instead. Rejected — the miss is legitimate under domain-gated and slim builds, so it should stay debug-only; the correct guard is a test that fails on regression, not a Sentry event on a supported configuration.
Submission Checklist
N/A: behaviour-only change; feature row 0.2.5 already coversharness_init_statusand no feature was added, removed, or renamed.## RelatedN/A: no release-cut surface changed; the overlay's visible states are unchanged.Closes #NNNin the## RelatedsectionImpact
harness_init_statusthe overlay now stays hidden instead of silently retrying forever — the same rendered result, without the traffic.method_not_foundis a newCoreRpcErrorKindvariant; no existing classification changes, and the JSON-RPC response to callers is untouched.Related
0.2.5(First-Run Harness Init)harness_init.harnessInitService's doc-comment states it mirrorsdaemonHealthService's polling contract — worth auditing that poller for the same unbounded-retry shape. Not touched here to keep this change scoped to unknown method: openhuman.harness_init_status — old client calling removed RPC #5157.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/5157-harness-init-statuse9700acaced960304c1a238d9381d6957d5a5b37Validation Run
pnpm --filter openhuman-app format:check— Prettier applied to all four changed frontend files; ESLint clean.pnpm typecheck—tsc --noEmitclean.vitest run HarnessInitOverlay.test.tsx coreRpcClient.test.ts→ 108 passed.rustfmt --check src/core/dispatch.rsclean. Fullcargo check/cargo testwas not run locally — that verification is delegated to CI and is itemised under Validation Blocked below.N/A — no change under app/src-tauri/.Validation Blocked
command:cargo test --lib core::dispatcherror:not run — full core builds were intentionally skipped locally on this machine.impact:the new Rust testharness_init_status_is_registered_in_a_full_buildis unverified by compilation. Its two APIs (core::all::all_controller_schemas,core::all::rpc_method_name) were confirmed present and type-compatible by inspection, andDomainSetscoping istokio::task_local!so the test cannot be contaminated by the domain-gating tests in the same binary. CI is the gate.Behavior Changes
Parity Contract
UNKNOWN_METHOD_PREFIXmirrors the Rust constant insrc/core/dispatch.rs, and the match is prefix-anchored exactly asunknown_method_nameusesstrip_prefix. A test pins that a nested/quoted occurrence does not classify asmethod_not_found. The JSON-RPC error envelope returned to callers is unchanged.Duplicate / Superseded PR Handling
Regression evidence
The new overlay tests fail against the pre-fix code and pass after — reverting only
HarnessInitOverlay.tsxand re-running:61 calls in the same window is the 64,715-event generator, reproduced.