Skip to content

fix(gui): keep file clients unknown on a failed-cold client read - #5088

Merged
lidge-jun merged 2 commits into
lidge-jun:devfrom
luvs01:fix/gui-failed-cold-settled
Sep 19, 2026
Merged

lidge-jun merged 2 commits into
lidge-jun:devfrom
luvs01:fix/gui-failed-cold-settled

Conversation

@luvs01

@luvs01 luvs01 commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

A failed initial read of /api/client-integrations (failed-cold, no data) was counted as a settled answer, so the Integrations Overview dropped the file-client rows as if the server had omitted them and rendered the "No installed clients were detected" panel next to the load error — a conclusion that can only be drawn from a settled response.

Description

  • clientsSettled now reads the resource's actual data presence (state.data !== undefined): undefined covers both pre-response and cold failures, while failed-with-stale keeps its real rows.
  • The file clients stay as "unknown" rows until a successful settlement, matching the keyPhase three-phase idiom already used for the API-keys row.

Tests

  • cd gui && bun test tests/integrations-surfaces.test.tsx — 38 pass; the new regression drives an HTTP 500 on the client list and asserts the load error stands alone, the false empty-state panel is absent, and the file-client row stays mounted.
  • cd gui && bun run lint — clean.

Screenshots

[Integrations overview during a failed-cold client-state read: the load error stands alone and file clients stay as unknown rows] (screenshot removed; was hosted on the contributor fork)

Summary by CodeRabbit

  • Bug Fixes
    • Improved integration status handling when client data is available alongside an error.
    • Prevented the interface from incorrectly stating that no clients are installed after an initial loading failure.
    • Existing integration entries remain visible while an error notice is displayed.

clientsSettled counted a failed-cold read as an answer, so the overview dropped the file-client rows as a server-side omission and showed the 'No installed clients were detected' panel next to the load error. Settle on the payload itself: undefined covers pre-response and cold failures while failed-with-stale keeps its real rows.
@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d6e0f046-d0c3-4d7d-9251-a3bbce59e159

📥 Commits

Reviewing files that changed from the base of the PR and between 3d5efc7 and b242957.

⛔ Files ignored due to path filters (1)
  • .github/pr-assets/5088-integrations-failed-cold.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • gui/src/pages/integrations/IntegrationsOverview.tsx
  • gui/tests/integrations-surfaces.test.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The integration overview now treats any available state data as settled. A regression test covers failed initial reads and verifies that the error notice and existing Hermes client row remain visible without showing an empty-installation message.

Changes

Integration state handling

Layer / File(s) Summary
Settled state and regression coverage
gui/src/pages/integrations/IntegrationsOverview.tsx, gui/tests/integrations-surfaces.test.tsx
clientsSettled now checks whether state data exists instead of excluding cold and retrying-cold kinds. The test covers a 500 response and verifies the error notice, the absence of the “No installed clients were detected” panel, and the Hermes client row.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to b2429

The change correctly avoids presenting an empty installation state after a failed read while preserving known or unknown client rows.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving file-client uncertainty when a client read fails in the cold state. It matches the changes in IntegrationsOverview.tsx and the reg…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • UI screenshot required.

What to do

  • Add a screenshot of the UI change to the PR description.

Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required enforce-target check will keep failing until every issue above is resolved.

@github-actions
github-actions Bot marked this pull request as ready for review September 18, 2026 22:22
@lidge-jun
lidge-jun merged commit c289926 into lidge-jun:dev Sep 19, 2026
35 of 36 checks passed
@luvs01
luvs01 deleted the fix/gui-failed-cold-settled branch September 20, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants