Skip to content

fix(miner-ui): stop a stale governor poll from reverting a just-applied pause/resume - #7871

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/governor-pause-stale-poll-guard-7791
Jul 21, 2026
Merged

fix(miner-ui): stop a stale governor poll from reverting a just-applied pause/resume#7871
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/governor-pause-stale-poll-guard-7791

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

Closes #7791.

LedgersPage (apps/loopover-miner-ui/src/routes/ledgers.tsx) syncs the governor pause-state from two independent sources with no ordering guard between them:

  • a usePolledFetch(loadGovernorPauseState, …) GET, synced into pauseState during render, and
  • the operator's Pause/Resume POST, which writes pauseState directly on resolve so it reflects immediately.

The POST doesn't share the poll's single-flight guard. If a poll GET is already in flight when the operator clicks Pause and the POST resolves first (plausible — a small local write vs. a read competing for the same SQLite file), the poll's stale pre-pause response lands after the action's fresh result and clobbers it. The UI visibly reverts to "Not paused" even though the governor is paused server-side, until the next poll tick self-corrects (up to 10s). Same in reverse for Resume. This is a display-only bug — it doesn't change what pause/resume do server-side.

The fix

Add an ordering guard mirroring the generation/cancellation discipline usePolledFetch/useStreamingText already use internally: when an action's result is applied, mark the next poll-sync as to-skip — that poll may have started before the action, so its result is not newer. It's still consumed (so lastPolledPauseState advances and we don't keep skipping), just not allowed to overwrite the action's result. Later ticks — which started after the action — sync normally. The flag is state, not a ref, so the render-phase sync reads it without accessing a ref during render (the react-hooks/refs lint rule). No change to how pauseGovernor/resumeGovernor work server-side.

Tests (ledgers.test.tsx)

  • Regression: reproduces the exact interleaving — first poll resolves "not paused" (Pause button shows), a second poll tick is left in flight, the operator clicks Pause, the action POST resolves first → UI shows Resume, then the stale second poll resolves "not paused"; asserts the UI stays paused (Resume shown, no Pause button). Verified bug-catching: reverting the guard makes this fail (the stale poll reverts the UI to Pause).
  • All existing 31 LedgersPage/live-refresh tests unchanged and green (32 total).

Validation

  • apps/loopover-miner-ui: vitest run 32 pass; tsc --noEmit exit 0; eslint 0 errors (the one react-refresh/only-export-components warning is pre-existing on main); Prettier clean.
  • Per the issue, apps/loopover-miner-ui is not under the src/** 99% patch gate; the regression test is the coverage deliverable. Branched off current main, mergeable-clean.

…ed pause/resume

LedgersPage syncs the governor pause-state from two independent sources with no
ordering guard: a usePolledFetch GET synced during render, and the operator's
pause/resume POST written directly on resolve. The POST doesn't share the poll's
single-flight guard, so a poll GET already in flight when the operator clicks can
resolve after the action's POST and clobber the fresh result with a stale
pre-action value -- the UI reverts to "Not paused" (or "paused") until the next
tick self-corrects. Mark the next poll-sync after an action lands as to-skip (that
poll may predate the action, so its result is not newer); later ticks sync
normally, mirroring the generation/cancellation discipline usePolledFetch and
useStreamingText already use. The flag is state, not a ref, so the render-phase
sync reads it without accessing a ref during render.
@shin-core
shin-core requested a review from JSONbored as a code owner July 21, 2026 15:34
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 21, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

1 similar comment
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-21 15:48:37 UTC

2 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This fix adds a one-shot `skipNextPollSync` flag so a poll response already in flight when an operator clicks Pause/Resume can't clobber the action's fresh result, mirroring the existing generation/cancellation pattern used elsewhere in the file. The logic is correct: the flag is consumed (advancing `lastPolledPauseState`) rather than dropped, so it doesn't get stuck skipping forever, and using state instead of a ref avoids the render-phase ref-read lint issue. The included regression test reproduces the exact race described in #7791 and would have failed before the fix.

Nits — 4 non-blocking
  • The flag only guards a single subsequent poll; if two polls are already in flight when the action resolves (unlikely given the single-flight guard in usePolledFetch, but not structurally impossible if pollIntervalMs is very short), the second stale one could still land after the skip is consumed and revert state.
  • apps/loopover-miner-ui/src/routes/ledgers.tsx has grown past 400 lines per the size-smell note; consider whether GovernorControlSection's poll-sync logic belongs in a small hook if this file keeps growing.
  • Consider extracting the pauseState/lastPolledPauseState/skipNextPollSync trio plus the sync effect into a small custom hook (e.g. `useGovernorPauseState`) for readability, since the comment block explaining the race is now fairly long inline.
  • The FAILED 'Contributor trust' check is unrelated to this diff's content and the branch is 1 commit behind default — likely stale rather than a defect here.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7791
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 90 registered-repo PR(s), 50 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 90 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff adds a skipNextPollSync state flag set when an action resolves, causing the very next poll-sync to be consumed but not applied, directly preventing the stale-poll-clobbers-action race described in the issue, and mirrors the generation-guard discipline requested without touching server-side pause/resume logic. A new regression test in ledgers.test.tsx reproduces the exact interleaving (pol

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
  • Official Gittensor activity: 90 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ledgers desktop before /ledgers
before /ledgers
after /ledgers
after /ledgers
/ledgers mobile before /ledgers (mobile)
before /ledgers (mobile)
after /ledgers (mobile)
after /ledgers (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/ledgers before /ledgers (scroll)
before /ledgers (scroll)
after /ledgers (scroll)
after /ledgers (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot 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.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored
JSONbored merged commit 1454d22 into JSONbored:main Jul 21, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Governor pause/resume UI state can be silently reverted by a stale poll response racing the action

2 participants