fix: Gateway: wizard page with workspaces has flickering workspace status icons and buttons (CRW-13522)

[vrubezhny]

fixes https://redhat.atlassian.net/browse/CRW-13522

Summary

The DevWorkspace watch in the "Connect to Dev Spaces" wizard resumed from
the resourceVersion it was originally started with instead of the latest
one it had actually consumed, and silently retried forever on a 410/Expired
response with zero logging anywhere in the failure path. In practice this
left the wizard's workspace list frozen (e.g. stuck on "Starting") until
the user noticed and clicked Refresh.

• Track the resourceVersion of the last consumed watch event and resume
  from it on reconnect, instead of the stale value start() was called with.
• Detect resourceVersion expiry in both forms the k8s client surfaces it
  (a thrown ApiException(410) and an in-stream "ERROR" event carrying a
  V1Status) and recover via a relist-and-resume path (new
  DevWorkspaceListener.onReset callback), mirroring what the manual
  Refresh button already did, instead of looping on a dead resourceVersion.
• Add a resourceVersion-ordering guard when applying updates to the table
  model, so an out-of-order/stale redelivery (e.g. a relist served from a
  lagging apiserver replica) can never regress an already-displayed state;
  a genuinely newer resourceVersion is always applied, even if its phase
  looks like a "regression" — only ordering is guarded, never phase
  semantics, so real server-side changes are never hidden.
• Log every branch of the watch's error/retry/relist handling, and every
  consumed ADDED/MODIFIED/DELETED event, so this class of issue is
  diagnosable from idea.log without a code-reading investigation.

Separately investigated and ruled out as a client-side issue: a
Running → Starting → Running status flicker observed on some
workspaces right after they finish starting. A debug-log capture confirmed
this is the devworkspace-operator itself writing a genuinely newer,
correctly ordered phase value (strictly increasing resourceVersion, no
reconnect/relist involved) — the watch is correctly relaying real server
state, so there's nothing to fix on the plugin side for that symptom.

Test plan

• ./gradlew test — 567 passed, 0 failed, 4 pre-existing skips
• New unit tests covering: resourceVersion advancing from consumed
  events, 410-via-ApiException and 410-via-in-stream-ERROR-event
  both trigger relist-and-resume, 403/404 still stop the watch
  permanently, onReset reconciliation (add/update/remove, namespace
  isolation), resourceVersion-ordering guard (stale update dropped,
  newer update applied even with a "regressed" phase, missing/
  unparseable resourceVersion fails open)
• Manually reproduced the original frozen-wizard bug against a live
  cluster and confirmed the watch now recovers automatically

Fixes: https://redhat.atlassian.net/browse/CRW-13522

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f5eddf1e-ee6d-4afe-9137-18ca1dd1efc5

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 69.72477% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.55%. Comparing base (71098f6) to head (770bdc8).
⚠️ Report is 442 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/gateway/devworkspace/DevWorkspaceWatchManager.kt	60.52%	24 Missing and 6 partials ⚠️
.../view/steps/workspaces/DevWorkspaceTableUpdater.kt	86.66%	0 Missing and 2 partials ⚠️
...at/devtools/gateway/openshift/ApiExceptionUtils.kt	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            main     #382       +/-   ##
==========================================
+ Coverage   0.00%   41.55%   +41.55%     
==========================================
  Files          4      124      +120     
  Lines         26     5432     +5406     
  Branches       0     1036     +1036     
==========================================
+ Hits           0     2257     +2257     
- Misses        26     2878     +2852     
- Partials       0      297      +297     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[adietish]

Great fix, thanks!
Found another issue that I committed on top and a minor refactoring where I split DevWorkspaceWatch.watchLoop into separate method to get better readable.

[adietish]

here's an explanatory summary using the show-me skill:

CRW-13522 — DevWorkspace watch: 410 recovery, ordering guard, logging

Three commits, one class of bug. The wizard's workspace list froze (stuck on
"Starting") because the watch resumed from a dead resourceVersion and retried
silently.

Commit	Change
b7122ff fix	Track consumed RV, detect 410/Expired in both forms, relist-and-resume via onReset, ordering guard, full logging
7c00fdf fix	Dedicated throwing LIST so a failed relist can't wipe the table
770bdc8 refactor	Split watchLoop into handleEvent() / handleApiException(), no behavior change

---

The bug: a watch that resumed from a dead address

The loop always reconnected with the RV it was started with, and every
failure path was silent — so a 410/Expired made it spin forever on a stale RV
while the wizard sat frozen:

 start(rv)
   loop:
-    watch from rv                    ← same stale RV, every reconnect
+    watch from currentRv             ← last RV actually consumed
     per event:
-      dispatch to listener
+      dispatch to listener
+      currentRv = event.rv           ← track stream progress
     on ApiException(403/404): stop
-    on ApiException(other):  retry   ← silent, forever
+    on ApiException(410):  relistAndReconcile()  ← log + recover
+    on ApiException(other): retry                ← warn
+    on stream end:        reconnect              ← debug-log

b7122ff — the fix

Both expiry shapes are caught, then one recovery path handles them:

event.type == "ERROR" (in-stream V1Status)  ──┐
thrown ApiException(410 Gone)               ──┼──> relistAndReconcile()
                                              │     1. LIST namespace  → items + fresh rv
                                              │     2. listener.onReset(ns, items)
                                              │     3. resume watch from fresh rv
                                              └──> table reconciles to ground truth

• DevWorkspace/ObjectMeta now carry resourceVersion (parsed from the JSON, never used for row identity).
• New DevWorkspaceListener.onReset — the same reconcile the manual Refresh button did.
• DevWorkspaceTableUpdater.onUpdated gets the ordering guard:

onUpdated(incoming)
  if incoming.rv <= displayed.rv     # integer compare; fail-open if non-numeric
    log "ignoring stale update"; return
  apply                             # newer rv always wins — phase semantics never guarded

• Every branch logs (410 → info, other errors → warn, reconnect → debug, each consumed event → debug) — diagnosable from idea.log without reading code.

7c00fdf — don't wipe the table on a failed relist

The first fix's relist reused listWithResult, which swallows 403/404/CRD-missing
as an empty list — a transient permission flap would have called
onReset(ns, []) and cleared every row. Fix: a dedicated throwing LIST, so the
catch skips onReset:

 relist = { ns ->
-    devWorkspaces.listWithResult(ns)     # swallows 403/404 → empty
-    DevWorkspaceRelistResult(...)
+    devWorkspaces.listForWatchResume(ns) # 403/404 propagate
 }
 relistAndReconcile()
   try { relist(ns) }
   catch { log warn; return null }        # onReset skipped, rows kept, loop retries
   dispatchToListener { onReset(ns, items) }

770bdc8 — readability split, no behavior change

 watchLoop
   for event in stream:
-    <~90 lines of inlined event + ApiException handling>
+    outcome = handleEvent(event, rv)      # ERROR→relist, ADDED/MOD/DEL→dispatch
+    rv = outcome.resourceVersion
+    if !outcome.streamUsable: break
   catch ApiException:
-    <inline 403/404/410 branches>
+    rv = handleApiException(e, rv)        # 403/404→stop, 410→relist, else→retry
+    if stopped: return

The EventOutcome(resourceVersion, streamUsable) pair keeps the loop's
"break on ERROR" explicit.

Ruled out: the Running→Starting→Running flicker

The status-icon flicker in the bug report was investigated and ruled out
client-side: a debug-log capture showed the devworkspace-operator itself writing
a genuinely newer, correctly-ordered phase (strictly increasing RV, no
reconnect/relist involved). The watch was faithfully relaying server state —
nothing to fix on the plugin side for that symptom.