Skip to content

Tui Clean Up#314

Merged
arul28 merged 2 commits into
mainfrom
ade/tui-clean-up-68fafd96
May 16, 2026
Merged

Tui Clean Up#314
arul28 merged 2 commits into
mainfrom
ade/tui-clean-up-68fafd96

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 16, 2026

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Greptile Summary

This PR addresses two previously flagged issues — per-branch fault isolation in fetchMissingSameRepoLanePulls and the terminal-GitHub-PR preference in shouldPreferGithubPrTag — and adds supporting infrastructure: fork-PR discrimination in branch matching, residual-file cleanup after git worktree remove, and a forced GitHub snapshot refresh triggered by lane branch-signature changes.

  • laneService.ts: Extracts removeResidualDirectory so both the success and failure paths after git worktree remove share the same fs.promises.rm + git worktree prune cleanup; adds a post-success existence check to catch git reporting exit 0 while leaving files behind.
  • prService.ts: Adds fetchMissingSameRepoLanePulls, capped at 12 branches, with per-branch try/catch so a single flaky API call doesn't abort the remaining lookups; populates headRepoOwner/headRepoName on mapped GitHubPrListItem rows.
  • lanePageModel.ts: Adds a same-repo owner/name guard to githubPrMatchesCurrentBranch (blocking fork PRs with matching branch names) and introduces shouldPreferGithubPrTag which restricts the branch-reuse preference to non-terminal GitHub PRs.

Confidence Score: 5/5

Safe to merge — all changed paths are well-covered by new tests and directly address previously identified gaps.

The three substantive logic changes (worktree residual cleanup, per-branch fault isolation, and the GitHub-vs-ADE PR tag preference) each have dedicated unit tests that exercise the exact scenarios they fix. The type additions are backward-compatible optional fields. No regressions are apparent in the shared selector or snapshot-fetch paths.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/main/services/lanes/laneService.ts Extracted removeResidualDirectory helper so both the success-path and failure-path worktree cleanup share the same fs.promises.rm + git worktree prune logic; now also checks for residual files after a zero-exit git worktree remove.
apps/desktop/src/main/services/prs/prService.ts Adds fetchMissingSameRepoLanePulls (capped at 12 branches, per-branch fault-isolated with try/catch+continue) and helper extractors for head-repo fields; also populates headRepoOwner/headRepoName on mapped GitHubPrListItem rows.
apps/desktop/src/renderer/components/lanes/lanePageModel.ts Adds fork-PR guard to githubPrMatchesCurrentBranch and introduces shouldPreferGithubPrTag which correctly restricts branch-reuse preference to non-terminal GitHub PRs.
apps/desktop/src/renderer/components/lanes/LanesPage.tsx Adds lanePrBranchSignature memo to trigger a forced GitHub snapshot refresh whenever the set of lane branches/types changes.
apps/desktop/src/shared/types/prs.ts Adds optional headRepoOwner and headRepoName fields to GitHubPrListItem to support fork-PR discrimination in the renderer.
apps/desktop/src/main/services/lanes/laneService.test.ts Adds a test verifying that residual worktree files left after a successful git worktree remove are cleaned up and that git worktree prune is called before the lane row is deleted.
apps/desktop/src/main/services/prs/prService.test.ts Adds two tests: one verifying targeted per-branch PR fetching for lane branches absent from the repo snapshot window, and one verifying that a per-branch lookup failure does not abort remaining branches.
apps/desktop/src/renderer/components/lanes/LanesPage.test.ts Adds/updates tests for the ADE-vs-GitHub PR tag preference logic, including the fork-PR guard and the terminal-vs-live branch reuse scenario.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getGithubSnapshot] --> B[fetchAllPages repo PRs]
    B --> C[fetchMissingSameRepoLanePulls]
    C --> D{Active lane branches\nwithout same-repo PR?}
    D -- No --> G[backfillLanePrRowsFromGithubPulls]
    D -- Yes, up to 12 --> E[Per-branch targeted fetch\nhead=owner:branch]
    E --> F{API error?}
    F -- Yes --> E2[log warn, continue next branch]
    F -- No --> E3[Deduplicate by PR number\nadd to rawPulls]
    E2 --> G
    E3 --> G
    G --> H[selectLaneTabPrTag in renderer]
    H --> I{mappedPr exists?}
    I -- Yes --> J[selectTerminalGithubUpdateForPr]
    J -- terminal match --> K[return GitHub terminal tag]
    J -- no match --> L{shouldPreferGithubPrTag?}
    L -- same PR state differs OR ADE terminal + GitHub live --> M[return githubPr tag]
    L -- No --> N[return ADE mapped PR tag]
    I -- No --> O{githubPr branch match?}
    O -- Yes, same-repo owner+name check passes --> P[return GitHub PR tag]
    O -- No / fork PR blocked --> Q[return null]
Loading

Reviews (2): Last reviewed commit: "ship: iteration 1 - address Greptile PR ..." | Re-trigger Greptile

@vercel
Copy link
Copy Markdown

vercel Bot commented May 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 16, 2026 11:26pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

Warning

Rate limit exceeded

@arul28 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 27 minutes and 41 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e0457e7f-783e-418a-8cb0-07662a3c781c

📥 Commits

Reviewing files that changed from the base of the PR and between 70e5bb3 and d67d5cc.

⛔ Files ignored due to path filters (3)
  • docs/features/lanes/README.md is excluded by !docs/**
  • docs/features/lanes/worktree-isolation.md is excluded by !docs/**
  • docs/features/pull-requests/README.md is excluded by !docs/**
📒 Files selected for processing (8)
  • apps/desktop/src/main/services/lanes/laneService.test.ts
  • apps/desktop/src/main/services/lanes/laneService.ts
  • apps/desktop/src/main/services/prs/prService.test.ts
  • apps/desktop/src/main/services/prs/prService.ts
  • apps/desktop/src/renderer/components/lanes/LanesPage.test.ts
  • apps/desktop/src/renderer/components/lanes/LanesPage.tsx
  • apps/desktop/src/renderer/components/lanes/lanePageModel.ts
  • apps/desktop/src/shared/types/prs.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/tui-clean-up-68fafd96

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 and usage tips.

@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 16, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 16, 2026

@copilot review but do not make fixes

@arul28 arul28 force-pushed the ade/tui-clean-up-68fafd96 branch from e5c9332 to 3172a3f Compare May 16, 2026 22:54
Copy link
Copy Markdown
Contributor

Copilot AI commented May 16, 2026

@copilot review but do not make fixes

Reviewed commit 3172a3f without making fixes. I don’t see any blocking issues in the lane deletion recovery, targeted same-repo PR backfill, or lane PR badge matching changes. I attempted targeted validation, but npm --prefix apps/desktop run test -- ... failed in this environment because vitest is not installed.

Copilot finished work on behalf of arul28 May 16, 2026 22:55
Comment thread apps/desktop/src/main/services/prs/prService.ts
Comment thread apps/desktop/src/renderer/components/lanes/lanePageModel.ts
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 16, 2026

@copilot review but do not make fixes

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