Skip to content

Skip proven other-forge projects in Tasks requests - #34219

Merged
brennanb2025 merged 8 commits into
mainfrom
brennanb2025/tp-forge-selection
Oct 29, 2026
Merged

brennanb2025 merged 8 commits into
mainfrom
brennanb2025/tp-forge-selection

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​463 $\color{#cf222e}{\Huge{\mathbf{−}}}$​16 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​447
Prod 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​168 $\color{#cf222e}{\Huge{\mathbf{−}}}$​26 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​142

Tasks keeps one project selection when users switch between GitHub and GitLab. A GitHub-only project could reach GitLab’s local MR fallback and turn an otherwise valid empty list into an error. GitLab-only projects could show GitHub’s unresolved-source notice. A cached GitLab project identity could also survive a remote migration and keep querying the old project.

This checks each unresolved selected repository’s current remotes at the provider request boundary. Projects proven to belong to the other forge contribute an empty result silently. Matching sources keep working, including mixed-forge remotes, and unknown identities retain the existing local/WSL MR fallback with search, state and paging. The GitLab identity cache revalidates one initial native Git config observation per repository lookup, expires external dependencies, and reads current remotes when no signature is available. List, detail and action routing retain the shipped behavior. The shared picker and settled partial-results behavior stay unchanged.

Seven regression cases failed on the original production code. Before the main merge, 147 tests passed across six named GitLab suites, including real config-reader operation counts at 3 and 30 remotes, warm native/WSL migration, edit races and late probe completion, mixed/extra remotes, unknown fallback, partial results, and cache bounds. Renderer tests cover host twins and runtime RPC → cache → notice replacement when a fresh legacy response omits the optional marker. The normal commit hook passed. Production delta: 194 changed lines; test delta: 479. Independent final source, performance and readiness reviews finished with low source merge risk and no identified negative user-facing trade-offs. All active merged-head CI checks passed. Targeted Windows/WSL QA on this head reproduced four stale GitLab list calls on pinned main after a genuine GitLab-to-GitHub origin migration; the branch made zero stale project-list or bare-MR calls and showed clean empty MR and Issues lists. Both normal Refreshes completed within five minutes of a genuine project-specific API warm attempt (main 91.518s; branch 46.856s). Existing provider-auth availability chrome remained. The warm attempt may fail authentication/rate limits; no successful live GitLab response is claimed. A real-Git/filesystem corpus passed three tests with seven measurements, including 3/30 remotes (cold config reads 3/3; warm 2/2). Provider auth/API responses in that benchmark are explicit fixtures. The branch screenshot uses the shipping Tasks list and production cache/projection: the unknown source retains one enabled Retry while the proven-other source is silent. Both copies stopped gracefully; real config hashes were unchanged.

The conflict-blocking main merge preserves the incoming partial-host recovery and pagination fixes. The partial-auth tests now explicitly supply a stable config signature for cache-hit assertions and use command-specific Git outputs; an added unsigned case verifies that late facts apply only to the current remote. All 201 tests across 12 affected named suites pass. The required merge hook passed. Independent narrow merge-interaction review agrees that both PRs’ guarantees are preserved, with LOW merge risk and no identified negative user-facing trade-offs. QA carries from ef84b3e across merge 8968760; all 26 active merged-head CI checks passed (17 skipped; none failed or incomplete).

Full path: provider failure handling, identity-cache freshness, and additive source-result metadata.

Intended differences from the common pattern: Orca retains its supported local/WSL MR inference for unresolved identities; only affirmative provider mismatch is skipped. When freshness evidence is lost and the current read fails, the old GitLab identity is not served; issues retain their existing unresolved behavior. Custom GitLab hosts without reliable affirmative evidence remain unresolved on the GitHub tab, preserving today’s notice pending the auth-inventory follow-up. GitLab issue-side not_found errors were already suppressed in the UI. The optional result marker is additive: old hosts/clients retain legacy notices when it is absent or ignored, with no new request or minimum server version.

Builds on #30151 by @yasu00000 — thank you! Reuses the provider-eligibility idea with a smaller boundary implementation; no community commit was adopted as a base.

Tracked in #13817
Tracked in STA-3902
Tracked in #15784
Tracked in STA-5041

brennanb2025 and others added 2 commits October 26, 2026 19:41
Revalidate GitLab project identity after remote configuration changes and pin resolved list calls to their host. Preserve unresolved local MR inference and settled partial results; carry additive eligibility metadata for GitHub notices.

Co-authored-by: yasuhiro nishioka <yasu00000@gmail.com>
@yasu00000

Copy link
Copy Markdown
Contributor

Thanks for the credit, and for taking this on from the main side. The request-boundary check is a much smaller surface than what #30151 carries, and I'd rather rescope mine around it than keep two implementations alive.

Two things from the #30151 work that this version leaves open, in case they help the review loop:

  1. Remote hosts. isTaskRepositoryIneligible returns false whenever connectionId is set, so a mixed GitHub/GitLab selection on an SSH host keeps today's behavior: the GitLab-only repo still lands in the GitHub tab's unresolved-source notice, and the GitHub-only repo still comes back through the MR path's structured not_found. fix(tasks): skip projects hosted on the other forge instead of failing them #30151 keys its known-hosts lookup by host + connectionId and runs the same check for remote repos. That part could be carried over here if you want remote parity in this PR, or I can follow up with it on top of yours.

  2. Visibility of the skip. A repo proven to belong to the other forge now contributes an empty result silently. That's the right default for the error banner, but it is also the other half of the report on [Bug]: Shared project filter in Tasks generates errors #15784: the repo is selected, nothing shows, and there's no way to tell why. fix(tasks): skip projects hosted on the other forge instead of failing them #30151 collapses those repos into one "Skipped — hosted on GitHub/GitLab (n)" row without Retry (SkippedReposNotice), and the ineligible marker you're adding is exactly the hook that row needs. If you'd prefer to keep this PR to the boundary change, I'll trim fix(tasks): skip projects hosted on the other forge instead of failing them #30151 down to that notice (plus the remote-host piece) and rebase it on top once this lands.

Either way, tell me which pieces you'd like over here vs. in a follow-up and I'll shape #30151 accordingly.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 27, 2026 03:51
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ebc54b7a-20b6-4a1e-9b50-a7b421be2db8
Commits

Reviewing files that changed from the base of the PR and between 8968760 and f720df4.

Files selected for processing: 1
  • src/main/gitlab/issues.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The changes add remote-based task-repository eligibility checks for GitHub and GitLab. GitHub work-item results can carry an ineligible marker, which task-page selectors use when identifying unresolved sources. GitLab issue and merge-request listings return empty results for repositories identified as belonging to another forge. GitLab project-reference caching now uses local Git configuration signatures and expires cached entries.

Priority: Low

Merge Risk: Minimal · up to f720d

No concrete merge-blocking risk was found in the reviewed changes.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: skipping requests for repositories proven to belong to another forge.
Description check Passed The description is detailed and covers the change, motivation, testing, linked tracking items, compatibility behavior, and review results. It does not follow the template headings or checklist format,…
Docstring Coverage Passed Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
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.
Full details: Description check

Explanation

The description is detailed and covers the change, motivation, testing, linked tracking items, compatibility behavior, and review results. It does not follow the template headings or checklist format, and it uses “Tracked in” instead of the required “Fixes #” issue link.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@brennanb2025
brennanb2025 marked this pull request as draft October 27, 2026 03:55
@brennanb2025
brennanb2025 marked this pull request as ready for review October 27, 2026 03:55
@brennanb2025

brennanb2025 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Tasks shares one project selection between GitHub and GitLab. A project on the other forge could produce a list error or unresolved-source notice; GitLab could also keep querying an old project after its remote changed.

This keeps matching and mixed-forge projects working, silently skips only projects proven to belong to the other forge, and revalidates GitLab identity after remote changes. Unknown/offline projects keep the existing behavior, including local/WSL MR inference. Older paired hosts and clients keep their existing notices when the optional result marker is absent or ignored.

The review fixed host-twin notice identity, restored shipped list/detail routing, and removed repeated config reads per remote. Two source-review rounds and independent performance/readiness checks found low merge risk. Conflict repair preserves incoming partial-host auth recovery and pagination alongside signature-based identity revalidation. The three failing partial-auth cases were unsigned-fixture assumptions: their cache-hit tests now explicitly supply a stable signature and realistic command-specific Git responses, while an added unsigned case guards current-remote recovery. Original miss/TTL/eviction and 100/1000 bounds remain. Independent narrow merge review found no lost fixes or negative user-facing trade-offs. A second main merge, f720df4aea7, incorporates main's coded GitHub/GitLab failure copy (#35689); its only conflict was an import line in src/main/gitlab/issues.ts (both kept), 219 tests across 8 affected named suites pass, and merged-head CI is green (33 passed, 17 skipped).

Merge 896876040daa47858cb42c2602e6bcc0bb7f5edb incorporates main db138baac0bbdbd4c056f2d8aacf52c038239c27. All 201 tests across 12 affected named suites pass through tc-gate, and the required merge hook passed. All 26 active merged-head CI checks passed (17 skipped, no failures or incomplete checks), including the aggregate PR gate, all five unit shards, both packages, Mobile and all three WSL distros. Renderer host-twin and runtime RPC → cache → notice contracts remain covered. QA carries from ef84b3eb7042f6df01cd647e3d5ed90fdf2f7b43 across this merge: the independent review confirms the exercised WSL and notice flows remain intact. Targeted Windows/WSL QA used genuine Git/glab commands: after a real GitLab-to-GitHub origin edit, main made four stale GitLab list calls; the branch made zero project-list or bare-MR calls and both lists settled cleanly empty. Both normal Refreshes finished within five minutes of a genuine project-specific warm API attempt (main 91.518s, branch 46.856s). The warm attempt may fail auth/rate limits; no successful live GitLab response is claimed. Existing auth availability chrome and the unrelated updater toast remained visible. Both copies stopped gracefully and all real config hashes were unchanged.

The real-Git/filesystem benchmark passed three tests and seven measurement records. At 3/30 remotes it made 3/3 cold config reads and 2/2 warm reads; measured warm times were 10.985/11.061ms. Auth/API replies in this benchmark are labelled fixtures. The branch-only notice capture uses the shipping Tasks list and production cache/projection: the unknown source keeps one enabled Retry and the proven-other host twin is silent. This is presentation evidence; no pairing or live-provider success is claimed.

Existing limitations remain: custom GitLab hosts need affirmative identity evidence; broader host propagation and the other providers’ shared positive-cache freshness are outside this change. Conditional includes selected by HEAD can retain an identity until the five-minute expiry, improving on main’s unbounded cache without claiming complete dependency tracking.

For Brennan’s review

The main and branch screenshots both show empty lists; the stale-call difference is established by the command trace. The branch’s existing auth banner and unrelated updater toast are retained.

Flow Pinned main Branch
GitLab MRs after origin migration main-wsl-migration-mrs.png branch-wsl-migration-mrs.png
GitLab Issues after origin migration main-wsl-migration-issues.png branch-wsl-migration-issues.png

Branch-only shipping-list notice capture (presentation fixture):

github-source-notice-branch.png

Preserve partial GitLab host recovery and pagination alongside signature-based project identity revalidation.
@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

Tasks now silently skips selected repositories proven to belong to the other forge.

  • Tasks skips list requests for projects proven to belong to the other forge.
  • GitLab list requests use a project identity that still matches the repository remotes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Selected repository] --> B[Resolve requested forge source]
  B --> C{Source found?}
  C -->|Yes| D[Use existing list request]
  C -->|No| E[Read current remotes]
  E --> F{All remotes prove another forge?}
  F -->|Yes| G[Return silent empty result]
  F -->|No or read fails| H[Keep existing unresolved behavior]
Loading

Reviews (1) · Last reviewed commit: "Merge origin/main into brennanb2025/tp-f..." · Reviewed by Greptile

@brennanb2025
brennanb2025 merged commit d2baeff into main Oct 29, 2026
55 of 56 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/tp-forge-selection branch October 29, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants