https://github.com/GautamTalksDev/keyring/pull/new/fix/stale-scan-refresh

[GautamTalksDev]

Addresses Qodo finding on #4. A card refresh from a previous scan could resolve
after a new scan began, overwriting the cleared queue and restoring "completed"
status — producing a headline with mismatched grant and system counts during a
running scan. In-flight fetches are now aborted on new scans and responses are
discarded when scanId no longer matches. Regression test added.

[qodo-code-review]

PR Summary by Qodo

Prevent stale card refreshes from overwriting active scans

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Abort in-flight card requests whenever a new scan starts or supersedes a refresh.
• Reject card results whose scan ID no longer matches the active session.
• Add regression coverage for stale responses before and after current results.

Diagram

sequenceDiagram
  actor User
  participant Hook as Scan Session
  participant Abort as Abort Controller
  participant API as API Client
  participant Reducer
  User->>Hook: Start new scan
  Hook->>Abort: Abort old refresh
  Hook->>API: Fetch cards with signal
  API-->>Hook: Return cards
  Hook->>Hook: Validate scan ID
  alt Current scan
    Hook->>Reducer: Apply cards and status
  else Stale or aborted
    Hook--xReducer: Discard response
  end

Loading

High-Level Assessment

The combined cancellation and scan-identity validation is the appropriate approach. AbortController reduces obsolete work, while hook and reducer guards protect against requests that complete before cancellation or actions dispatched through another path; using either safeguard alone would be weaker.

Files changed (3) +94 / -15

Enhancement (1) +5 / -2

client.tsSupport cancellation of card fetches +5/-2

Support cancellation of card fetches

• Extends 'fetchCards' with an optional 'AbortSignal' and forwards it through the shared request helper to the browser fetch call.

apps/web/src/api/client.ts

Bug fix (1) +40 / -12

useScanSession.tsIsolate card refreshes by active scan +40/-12

Isolate card refreshes by active scan

• Tracks the active scan and current refresh controller, aborting obsolete requests on replacement and unmount. Card actions now carry a scan ID, and both the async refresh path and reducer reject stale results.

apps/web/src/hooks/useScanSession.ts

Tests (1) +49 / -1

useScanSession.test.tsCover stale card refresh rejection +49/-1

Cover stale card refresh rejection

• Adds a reducer regression test proving old-scan card payloads cannot replace the current queue or status, whether they arrive before or after the current result.

apps/web/src/hooks/useScanSession.test.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Stale starts become active ✓ Resolved 🐞 Bug ≡ Correctness

Description

beginScan assigns every startScan response to activeScanIdRef without verifying that it
belongs to the latest invocation, so an earlier request resolving late can replace the newer scan
and pass the new refresh freshness check. It also installs that stale scan's SSE subscription,
allowing its events and cards to overwrite the current session.

Code

apps/web/src/hooks/useScanSession.ts[520]

+      activeScanIdRef.current = started.scanId;

Evidence

Each invocation clears current resources before awaiting the non-abortable POST, but there is no
ownership check after the await. Thus, invocation A can remain pending while B begins; if B resolves
and then A resolves, A unconditionally dispatches scan_started, overwrites the newly added active
ID, replaces the stored unsubscribe callback, and starts its stream/refresh. Stream events are
dispatched without a scan-ID guard, while the reducer only guards cards, so the stale subscription
can continue mutating activity.

apps/web/src/hooks/useScanSession.ts[504-520]
apps/web/src/hooks/useScanSession.ts[521-537]
apps/web/src/hooks/useScanSession.ts[133-161]
apps/web/src/api/client.ts[20-42]
apps/web/src/api/client.ts[96-149]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Overlapping `beginScan` calls can resolve out of order, allowing an older start response to become the active scan and install a stale stream subscription.

## Issue Context
The new card-response guard relies on `activeScanIdRef` accurately representing the latest requested scan, but each `startScan` completion currently overwrites it unconditionally.

## Fix Focus Areas
- apps/web/src/hooks/useScanSession.ts[504-543]

Add a monotonically increasing invocation/generation token (or abortable start request), capture it at the beginning of each call, and ignore both success and failure from any invocation that is no longer current. Ensure stale responses cannot create subscriptions or trigger initial refreshes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced: This is a behavior-changing asynchronous scan/session fix involving aborts, stale-response guards, reducer state identity, and API propagation; it has genuine concurrency risk but remains focused enough for one careful review pass.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗