From d4473886ec824d10e2da150f017fd91c767c8a1e Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Mon, 21 Sep 2026 07:16:37 +0000 Subject: [PATCH 1/2] perf(list): resolve the GitLab project ID once per command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gitlab_project_id` shells out to `glab repo view`, a GitLab API call, and `detect_gitlab` runs once per branch — so `wt list --full` over N worktrees spent N round trips resolving one repo-level constant. Memoize it per worktree root for the process, keyed by path like the process-wide caches in `git::repository`, so in-process test runs can't read another repo's answer. The doc comment also pointed at `get_origin_owner`, which no longer exists; the GitHub side resolves its owner through `branch_owner_repo` from local git config. --- src/commands/list/ci_status/gitlab.rs | 33 +++++++++++++-- tests/integration_tests/ci_status.rs | 61 ++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/commands/list/ci_status/gitlab.rs b/src/commands/list/ci_status/gitlab.rs index 6e07e18d4c..531543a3aa 100644 --- a/src/commands/list/ci_status/gitlab.rs +++ b/src/commands/list/ci_status/gitlab.rs @@ -14,8 +14,10 @@ //! //! See: +use dashmap::DashMap; use serde::Deserialize; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::sync::LazyLock; use worktrunk::git::Repository; use super::{ @@ -23,21 +25,44 @@ use super::{ is_retriable_error, non_interactive_cmd, parse_json, }; +/// Project IDs already resolved in this process, keyed by the worktree root +/// `glab` was run from. +/// +/// Keyed by path rather than held in a bare static because tests run many +/// commands in one process, matching the path-keyed process-wide caches in +/// `git::repository`. A lookup that failed caches as `None`: retrying it once +/// per row is the cost this exists to remove. +static GITLAB_PROJECT_IDS: LazyLock>> = LazyLock::new(DashMap::new); + /// Get the GitLab project ID for a repository. /// -/// Used for client-side filtering of MRs by source project. -/// This is the GitLab equivalent of `get_origin_owner` for GitHub. +/// Used for client-side filtering of MRs by source project. The GitHub +/// equivalent is [`branch_owner_repo`](super::branch_owner_repo), which reads +/// local git config; this one has to ask the GitLab API, so it is memoized for +/// the process. [`detect_gitlab`] runs once per branch and the project ID is a +/// repo-level constant, so without that a `wt list --full` over N worktrees +/// would spend N round trips resolving the same value — and `wt list` cannot +/// finish until every row's task returns. +/// +/// The `entry` API holds the shard lock across the fetch, so concurrent rows +/// sharing the key wait on the first call rather than each spawning their own. /// /// Returns None if glab is not configured for this repo (e.g., non-GitLab /// remote, auth issues). fn gitlab_project_id(repo: &Repository) -> Option { let repo_root = repo.current_worktree().root().ok()?; + *GITLAB_PROJECT_IDS + .entry(repo_root.clone()) + .or_insert_with(|| fetch_gitlab_project_id(&repo_root)) +} +/// Ask `glab` for this repository's project ID. +fn fetch_gitlab_project_id(repo_root: &Path) -> Option { // Use glab repo view to get the project info as JSON // Disable color/pager to avoid ANSI noise in JSON output let output = non_interactive_cmd("glab") .args(["repo", "view", "--output", "json"]) - .current_dir(&repo_root) + .current_dir(repo_root) .env("PAGER", "cat") .run() .ok()?; diff --git a/tests/integration_tests/ci_status.rs b/tests/integration_tests/ci_status.rs index 99e9713ea9..142dc31847 100644 --- a/tests/integration_tests/ci_status.rs +++ b/tests/integration_tests/ci_status.rs @@ -5,7 +5,7 @@ use crate::common::{ TestRepo, make_snapshot_cmd, - mock_commands::{MockConfig, MockResponse}, + mock_commands::{MockConfig, MockResponse, mock_calls}, repo, setup_snapshot_settings, wt_command, }; use ansi_str::AnsiStr; @@ -641,6 +641,65 @@ fn test_list_full_with_gitlab_filters_by_project_id(mut repo: TestRepo) { ); } +/// `glab repo view` answers a repo-level question over the GitLab API, but MR +/// detection runs once per row — so a table of N worktrees used to spend N +/// round trips resolving one constant, on a command that cannot finish until +/// every row's task returns. Assert the spawn count, not the rendering: the +/// table looks identical either way. +#[rstest] +fn test_gitlab_project_id_resolved_once_per_command(mut repo: TestRepo) { + let head_sha = setup_gitlab_repo_with_feature(&mut repo); + + let mr_json = format!( + r#"[{{ + "iid": 1, + "sha": "{}", + "has_conflicts": false, + "detailed_merge_status": null, + "head_pipeline": {{"status": "success"}}, + "source_project_id": 12345, + "web_url": "https://gitlab.com/test-group/test-project/-/merge_requests/1" + }}]"#, + head_sha + ); + repo.setup_mock_glab_with_ci_data(&mr_json, Some(12345)); + + // Outside the repo under test, so the log can't dirty the working tree the + // command is inspecting. + let call_log = tempfile::tempdir().unwrap(); + let mut cmd = repo.wt_command(); + cmd.args(["list", "--full"]); + repo.configure_mock_commands(&mut cmd); + cmd.env("WORKTRUNK_TEST_MOCK_CALL_LOG_DIR", call_log.path()); + let output = cmd.output().unwrap(); + assert!( + output.status.success(), + "wt list --full should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let calls = mock_calls(call_log.path(), "glab"); + let repo_views: Vec<_> = calls + .iter() + .filter(|call| call.starts_with("repo view")) + .collect(); + // More than one row reached MR detection, so a per-row lookup would show up + // as more than one `repo view`. + let mr_lists = calls + .iter() + .filter(|call| call.starts_with("mr list")) + .count(); + assert!( + mr_lists > 1, + "the fixture must exercise several rows, else the count below proves nothing. calls: {calls:#?}" + ); + assert_eq!( + repo_views.len(), + 1, + "the project ID is repo-level, so it costs exactly one `glab repo view` however many rows ask for it. calls: {calls:#?}" + ); +} + // ============================================================================= // GitLab project ID edge cases (PR #846 panic prevention) // ============================================================================= From 1ded1d165db33792899a4939484a0b2fee7da747 Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Mon, 21 Sep 2026 07:29:31 +0000 Subject: [PATCH 2/2] docs(list): correct the GitLab project-ID cache rationale The keying rationale cited tests running many commands in one process, but nothing reaches gitlab_project_id in-process: integration tests spawn wt as a subprocess and this module's in-process tests cover only the parsers. Give the reason src/git/repository/mod.rs gives for its own path-keyed statics, and say what caching a failed lookup costs. --- src/commands/list/ci_status/gitlab.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/commands/list/ci_status/gitlab.rs b/src/commands/list/ci_status/gitlab.rs index 531543a3aa..8c9f6c7576 100644 --- a/src/commands/list/ci_status/gitlab.rs +++ b/src/commands/list/ci_status/gitlab.rs @@ -28,10 +28,13 @@ use super::{ /// Project IDs already resolved in this process, keyed by the worktree root /// `glab` was run from. /// -/// Keyed by path rather than held in a bare static because tests run many -/// commands in one process, matching the path-keyed process-wide caches in -/// `git::repository`. A lookup that failed caches as `None`: retrying it once -/// per row is the cost this exists to remove. +/// Keyed by path rather than held in a bare static, matching the path-keyed +/// process-wide caches in `git::repository`: two `Repository` instances +/// pointed at the same path see the same answer, and a second repository +/// resolved in the same process can't inherit the first's. A failed lookup +/// caches as `None` too — retrying it per row is the cost this exists to +/// remove — so a transient failure degrades MR detection for the rest of the +/// process. static GITLAB_PROJECT_IDS: LazyLock>> = LazyLock::new(DashMap::new); /// Get the GitLab project ID for a repository.