Skip to content

fix(ui): guard the repo-picker panels against out-of-order fetch responses - #7896

Closed
shin-core wants to merge 1 commit into
JSONbored:mainfrom
shin-core:fix/repo-picker-stale-response-guard-7784
Closed

fix(ui): guard the repo-picker panels against out-of-order fetch responses#7896
shin-core wants to merge 1 commit into
JSONbored:mainfrom
shin-core:fix/repo-picker-stale-response-guard-7784

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

Closes #7784.

Four maintainer panels bind a free-text owner/repo <input> to a useCallback load run in a useEffect keyed on repoFullName, with no cancellation guard. Every keystroke past the first / is a valid repo and fires a new fetch; if an earlier (shorter, partially-typed) request resolves after a later one, its response silently overwrites the newer repo's state — e.g. ai-review-settings would show the wrong repo's BYOK key status and AI-review mode after the user finished typing a different repo. Same bug class as the already-fixed AuditFeed out-of-order issue, triggered by keystrokes instead of pagination.

The fix

Apply the existing cancelled-flag idiom from use-polled-fetch.ts (the issue's required reuse pattern) to all five load functions — the flag is set in the effect cleanup and checked before any post-await setState:

const load = useCallback(async (isCancelled = () => false) => {
  
  const result = await apiFetch();
  if (isCancelled()) return;   // drop a response invalidated by a newer request
  …setState(result)
}, [repoFullName]);

useEffect(() => {
  let cancelled = false;
  void load(() => cancelled);
  return () => { cancelled = true; };
}, [load]);

Sites guarded: activation-preview.tsx, ams-miner-cohort-card.tsx, ai-review-settings.tsx, and maintainer-settings.tsx (its main load and FocusManifestEditor). maintainer-panel.tsx's runPreview is button-triggered, not auto-fetch-on-keystroke, so it's out of scope per the issue. No rendered output changes — this is a behavioral guard only.

Tests

A representative regression test in activation-preview.test.tsx: it issues two repos' requests, resolves the newer one first, then resolves the stale earlier one, and asserts the stale response is dropped (the newer repo's preview stays on screen). Verified bug-catching: removing the guard makes the stale "FIRST repo" data clobber the UI and the test fails.

Validation

  • apps/loopover-ui: vitest run (touched components) pass; tsc --noEmit exit 0; eslint clean; Prettier clean.
  • Per the issue, apps/loopover-ui is not under the src/** 99% patch gate; the regression test is the coverage deliverable. Branched off current main, mergeable-clean.

…sponses

Each of these panels binds a free-text owner/repo <input> to a load callback run
in an effect keyed on repoFullName, with no cancellation guard: typing fires a
fetch per keystroke, and an earlier (shorter, partially-typed) request resolving
after a later one silently overwrites the newer repo's state -- e.g. showing the
wrong repo's BYOK key status / AI-review mode. Apply the same cancelled-flag idiom
use-polled-fetch.ts already uses (flag flipped in the effect cleanup, checked
before any post-await setState) to all five load functions: activation-preview,
ams-miner-cohort-card, ai-review-settings, and maintainer-settings (its main load
plus FocusManifestEditor). Adds a representative regression test that resolves two
repos' requests out of order and asserts the stale one is dropped.
@shin-core
shin-core requested a review from JSONbored as a code owner July 21, 2026 16:27
@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.

@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 16:35:38 UTC

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

⏸️ Suggested Action - Manual Review

Review summary
This PR applies the existing cancelled-flag idiom from use-polled-fetch.ts to five load functions across four maintainer panels, correctly guarding against out-of-order fetch responses clobbering newer repo state — the guard is placed after the await and before any setState, and the cleanup correctly flips the flag on repoFullName/base change. The regression test in activation-preview.test.tsx genuinely exercises the real out-of-order path (resolves newer request first, then stale older one) and would fail without the guard, so it's a real, non-fabricated test. This is narrow, well-targeted operator-facing work directly closing #7784.

Nits — 3 non-blocking
  • maintainer-settings.tsx is already large (~562 lines per the size-smell note); this diff adds to it rather than extracting the load-guard logic into a shared hook, though duplicating the small idiom across files is arguably fine given the existing use-polled-fetch.ts precedent.
  • Add a companion regression test for at least one of ai-review-settings.tsx or maintainer-settings.tsx since they have slightly different shapes (Promise.all, and two separate load functions in one file) that the activation-preview test doesn't exercise.
  • Consider factoring the `isCancelled` guarded useEffect+useCallback pattern into a small shared hook (e.g., `useCancellableLoad`) given it's now duplicated identically five times — would reduce the maintainer-settings.tsx file size and centralize the idiom.
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 #7784
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 · LLM: significant
Linked issue satisfaction

Addressed
The diff applies the cancelled-flag idiom (isCancelled check after await, flipped in effect cleanup) to all five required load functions across the four listed components, matching the issue's required guard pattern exactly. It also adds a regression test in activation-preview.test.tsx that resolves a newer repo's request before a stale earlier one and asserts the stale response is dropped, satisf

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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (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)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (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 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This pull request changes UI/visual code but its screenshot evidence is incomplete. Every required viewport × theme combination needs its own before/after image pair in a labeled table row (e.g. "Desktop · Light | before | after"). Still missing: Desktop · Dark, Tablet · Dark, Mobile · Dark.

Please resubmit with the remaining rows filled in.

See https://github.com/JSONbored/loopover/blob/main/.claude/skills/contributing-to-loopover/SKILL.md for the exact format and examples. This is an automated maintenance action.

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.

4 repo-picker panels have no stale-response guard - free-text keystrokes race each other

1 participant