Skip to content

fix(web): retain eligible providers during refresh - #3498

Open
yattdev wants to merge 2 commits into
kdlbs:mainfrom
yattdev:feature/fix-repository-provi-bdm
Open

fix(web): retain eligible providers during refresh#3498
yattdev wants to merge 2 commits into
kdlbs:mainfrom
yattdev:feature/fix-repository-provi-bdm

Conversation

@yattdev

@yattdev yattdev commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Tip

PR walkthrough: Open the visual walkthrough

Repository-provider refreshes no longer briefly hide still-eligible providers while their latest results load. The hook also now proves that a stale response from a prior workspace cannot publish.

Validation

  • pnpm --filter @kandev/web test -- --run hooks/domains/integrations/use-remote-repositories.test.tsx
  • pnpm --filter @kandev/web lint -- hooks/domains/integrations/use-remote-repositories.ts hooks/domains/integrations/use-remote-repositories.test.tsx
  • pnpm --filter @kandev/web typecheck
  • pnpm --filter @kandev/web test (1,883 files passed; 3 unrelated HTTP Git fixture failures because guarded Docker blocks the required bridge-gateway probe)

Checklist

  • If I do not have repository write access and this is a large architectural change, I discussed the direction in a linked issue before opening this PR.
  • This PR contains one logical change; unrelated work is split into separate PRs.
  • I have performed a self-review of my code.
  • I have manually tested my changes and they work as expected.
  • My changes have tests that cover the new functionality and edge cases.
  • If my change touches UI files (apps/web/), I have added or updated Playwright e2e tests in apps/web/e2e/ and verified them with make test-e2e.
  • I checked whether this affects public docs in docs/public/** and updated them or noted why no docs change is needed.

Review in cubic

Screenshots

Desktop remote picker with an unconfigured provider hidden

Mobile remote picker with an unconfigured provider hidden

Preview Environment

URL https://kandev-pr-3498-bwo7.sprites.app
Commit cb7b887
Agent Mock agent

Updates automatically on each push. Destroyed when the PR is closed.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: b7fb628d-fb78-4942-bff8-0d51d14cbd9e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: 6adec77a-3173-4c57-a800-d43490c6baaf

📥 Commits

Reviewing files that changed from the base of the PR and between e3d4352 and db2f3e2.

📒 Files selected for processing (2)
  • apps/web/hooks/domains/integrations/use-remote-repositories.test.tsx
  • apps/web/hooks/domains/integrations/use-remote-repositories.ts

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


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Available repository providers now remain visible while a manual refresh is in progress.
    • Switching workspaces no longer displays stale repository results from the previous workspace.
    • Provider availability and errors are preserved during refreshes while outdated entries are removed appropriately.

Walkthrough

The hook now preserves eligible providers during same-workspace refreshes and clears provider state on workspace changes. It also prevents stale repository results from previous workspaces. Tests cover both behaviors with deferred requests.

Changes

Remote repository refresh

Layer / File(s) Summary
Refresh state and stale result handling
apps/web/hooks/domains/integrations/use-remote-repositories.ts, apps/web/hooks/domains/integrations/use-remote-repositories.test.tsx
The hook filters providers and source errors during same-workspace refreshes, resets them on workspace changes, and keeps repository results scoped to the current workspace. Tests cover provider retention and stale request suppression.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to db2f3

Eligible repository providers remain visible during refreshes, while results from a previous workspace are not shown after switching workspaces. The covered behavior is ready to merge.

Suggested reviewers: carlosflorencio

🚥 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
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.
Title check ✅ Passed The title clearly describes the main change: retaining eligible repository providers during refresh.
Description check ✅ Passed The description explains the behavior change, documents targeted and full-suite validation, and preserves the required checklist. It is complete and directly related to the pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

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

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @yattdev's task in 4m 20s —— View job


Findings

Suggestion (recommended, doesn't block)

  1. Asymmetry between repos and availableProviders during refresh is not documented by testsuse-remote-repositories.test.tsx:442

    • setRepos([]) remains unconditional (line 316 of the hook), so during a same-workspace refresh, repos briefly go blank while availableProviders is retained. That asymmetry is intentional — stale repos would be misleading, while a flickering "no providers" message is the regression being fixed. But the new test at line 442 only asserts the providers side; it doesn't assert that repos is [] while loading is in flight.
    • Fix: add expect(result.current.repos).toEqual([]) alongside the availableProviders assertion at line 458, making the documented contract explicit for both fields.
  2. Stale-response test covers only one of two orderingsuse-remote-repositories.test.tsx:519

    • The test (lines 519–562) sequences: wait for fetch-2 to start, then resolve fetch-1 (stale). That's the "delayed-old-response-after-successor" ordering, and it passes because the old effect's cleanup (cancelled = true) ran before fetch-1 resolved.
    • The complementary ordering — resolve fetch-1 before workspace-2's effect has started — is also safe (the same cancelled closure governs both orderings), but there's no test for it. The review skill asks to cover both.
    • Fix: add a second workspace-change test that calls resolveFirst synchronously inside the same act as the rerender, before awaiting the second fetch, then assert repos stays [].

Summary

Severity Count
Blocker 0
Suggestion 2

Verdict: Ready to merge with suggestions

The core change is correct: tracking sameWorkspace and using a functional updater to filter availableProviders (and sourceErrors) instead of unconditionally clearing them is a minimal, sound fix. The cancelled closure correctly prevents stale same-workspace responses from publishing. Tests cover the two key behaviors — providers retained during refresh (line 442) and stale workspace responses discarded (line 519). The two suggestions above would close a documentation gap and a missing race-condition ordering; neither is a correctness risk given the cancelled mechanism is deterministic.

@yattdev
yattdev marked this pull request as ready for review September 8, 2026 11:59
@yattdev
yattdev temporarily deployed to opencode-review-trusted September 8, 2026 11:59 — with GitHub Actions Inactive
@yattdev

yattdev commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@carlosflorencio Ready for review at db2f3e20b46533bcbcfe9d10db02e9ff6935131a. Independent GPT-5.5 Review and distinct Sol QA passed for the provider eligibility refresh fix. The PR is confirmed non-draft, mergeable, and has no unresolved threads. Post-ready checks are terminal: the cancelled zero-step lint duplicate (run34209119205/job102005633470) was superseded by successful run34209144024/job102005713827; there is no outstanding CI failure. No layout/style visual change. Please review; merge remains the upstream maintainer's decision.

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR keeps currently eligible repository providers visible while refreshed results load and prevents requests from a previous workspace from publishing stale repositories.

  • Retains only providers still eligible during same-workspace refreshes.
  • Clears prior provider errors when a retry begins.
  • Resets provider state when switching workspaces.
  • Adds focused tests for refresh loading, error clearing, and stale workspace responses.

Confidence Score: 5/5

The PR appears safe to merge; the previous stale-error concern is fixed and no new actionable failures remain.

The refresh now clears obsolete source errors before retrying, retains only providers that remain eligible in the same workspace, and continues to discard cancelled workspace-scoped responses. Focused tests cover each changed behavior.

Important Files Changed

Filename Overview
apps/web/hooks/domains/integrations/use-remote-repositories.ts Retains eligible providers across same-workspace refreshes, clears stale errors, and preserves workspace-scoped cancellation.
apps/web/hooks/domains/integrations/use-remote-repositories.test.tsx Adds regression coverage for provider retention, retry error clearing, and stale responses after workspace changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Refresh or workspace update] --> B{Same workspace?}
  B -->|Yes| C[Retain currently eligible providers]
  B -->|No| D[Clear provider state]
  C --> E[Clear stale errors]
  D --> E
  E --> F{Eligibility still loading?}
  F -->|Yes| G[Wait for eligibility update]
  G --> F
  F -->|No| H[Load all eligible providers]
  H --> I{Request still current?}
  I -->|No| J[Discard stale result]
  I -->|Yes| K[Publish repositories and current errors]
Loading

Reviews (2): Last reviewed commit: "fix(web): clear provider errors during r..." | Re-trigger Greptile

Comment thread apps/web/hooks/domains/integrations/use-remote-repositories.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db2f3e20b4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread apps/web/hooks/domains/integrations/use-remote-repositories.ts Outdated
@yattdev
yattdev marked this pull request as draft September 8, 2026 14:16
@yattdev
yattdev marked this pull request as ready for review September 9, 2026 01:35
@yattdev
yattdev deployed to opencode-review-trusted September 9, 2026 01:35 — with GitHub Actions Active
@yattdev

yattdev commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@carlosflorencio @jcfs @zeval Ready for review at exact head cb7b887e6ad482006097a61feb4083dde81fbe36 after independent GPT-5.5 Review and fresh distinct Sol QA (session 50c5179b-4b40-46d4-b8e1-594c3476df44).

The PR is now non-draft. The post-ready refresh confirms the same head, a clean merge, zero failed checks and zero unresolved review threads. Update: the ready transition has now started Greptile review and queued walkthrough generation, so those post-ready checks are pending. The scoped validation and limitations are recorded in the PR/task evidence. No visual product change requires additional manual testing; existing screenshots are retained where supplied. This notification applies only to the head above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants