feat(switch): experimental Gitea PR support via pr: shortcut - #1320
Conversation
worktrunk-bot
left a comment
There was a problem hiding this comment.
Two significant issues need attention before this can be approved.
1. tea api likely does not support {owner}/{repo} template expansion (critical)
The code passes repos/{owner}/{repo}/pulls/{number} to tea api, mirroring what gh api does. However, {owner} and {repo} are template variables that gh expands automatically based on the current repo context — this is a gh-specific feature. The tea api command is a raw HTTP client and (to my knowledge) does not perform this expansion. If so, the literal string {owner}/{repo} would be sent as the URL path, resulting in a 404 from the Gitea API.
This needs verification against the actual tea CLI. If tea doesn't support templates, the owner/repo would need to be resolved from git remotes before constructing the API path.
2. Happy-path integration tests are broken — they capture error output as expected
test_switch_gpr_same_repo and test_switch_gpr_fork are intended to test successful PR checkout, but their snapshots show tea api failed errors with "No gitea login configured" messages. Compare with the equivalent switch_pr_same_repo.snap which shows the full successful flow (fetching, PR info display, branch creation). The mock tea binary isn't intercepting despite identical setup to the working gh mock tests. The snapshots pass CI because they were committed with the error output — the tests aren't actually verifying the happy path.
Minor issues:
RefType::syntax()atsrc/git/error.rs:64is now dead code — both call sites were replaced with the explicitsyntaxparameter. Should be removed.configure_mock_tea_envis a line-for-line copy ofconfigure_mock_gh_env. Consider extracting a shared helper.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Two significant issues need attention before this can be approved.
1. tea api likely does not support {owner}/{repo} template expansion (critical)
The code at src/git/remote_ref/gitea.rs:82 passes repos/{owner}/{repo}/pulls/{number} to tea api, mirroring what gh api does. However, {owner} and {repo} are template variables that gh expands automatically based on the current repo context — this is a gh-specific feature. The tea api command is a raw HTTP client and (to my knowledge) does not perform this expansion. If so, the literal string {owner}/{repo} would be sent as the URL path, resulting in a 404 from the Gitea API.
This needs verification against the actual tea CLI. If tea doesn't support templates, the owner/repo would need to be resolved from git remotes before constructing the API path.
2. Happy-path integration tests are broken — they capture error output as expected
test_switch_gpr_same_repo and test_switch_gpr_fork are intended to test successful PR checkout, but their snapshots show tea api failed errors with "No gitea login configured" messages. Compare with the equivalent switch_pr_same_repo.snap which shows the full successful flow (fetching, PR info display, branch creation). The mock tea binary isn't intercepting despite identical setup to the working gh mock tests. The snapshots pass CI because they were committed with the error output — the tests aren't actually verifying the happy path.
Minor issues:
RefType::syntax()atsrc/git/error.rs:64is now dead code — both call sites were replaced with the explicitsyntaxparameter. Should be removed.configure_mock_tea_envis a line-for-line copy ofconfigure_mock_gh_env. Consider extracting a shared helper.
There was a problem hiding this comment.
Pull request overview
This PR adds Gitea pull request support to the wt switch command via the gpr:<number> syntax, following the existing patterns established for GitHub (pr:) and GitLab (mr:).
Changes:
- Introduces
GiteaProviderinsrc/git/remote_ref/gitea.rsimplementing theRemoteRefProvidertrait, using theteaCLI to fetch PR metadata - Adds
PlatformData::Giteavariant and wires Gitea handling into switch command parsing, remote discovery, and branch creation logic — including adding asyntaxfield toRefCreateConflict/RefBaseConflicterror variants to disambiguatepr:vsgpr: - Adds documentation, CLI help text, integration tests, and snapshots for the new
gpr:syntax
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/git/remote_ref/gitea.rs |
New Gitea PR provider with tea CLI integration, response parsing, and unit tests |
src/git/remote_ref/mod.rs |
Exposes GiteaProvider, adds Gitea module doc and test |
src/git/remote_ref/info.rs |
Adds PlatformData::Gitea variant, wires source_ref() and prefixed_local_branch_name(), adds unit tests |
src/git/error.rs |
Adds syntax: &'static str field to RefCreateConflict and RefBaseConflict variants |
src/commands/worktree/switch.rs |
Parses gpr: syntax, refactors find_github_remote → find_pr_remote, adds Gitea handling in resolve_same_repo_ref |
src/cli/mod.rs |
Adds gpr: to help text and shortcut documentation |
docs/content/switch.md |
Documents Gitea PR section in user-facing docs |
skills/worktrunk/reference/switch.md |
Adds gpr: reference to skills documentation |
tests/integration_tests/switch.rs |
Adds integration tests for gpr: create conflict, base conflict, same-repo, fork, not-found, and tea-not-installed scenarios |
tests/snapshots/*.snap (6 files) |
Snapshot files for the new integration tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| use worktrunk::config::UserConfig; | ||
| use worktrunk::git::remote_ref::{ | ||
| self, GitHubProvider, GitLabProvider, RemoteRefInfo, RemoteRefProvider, | ||
| self, GitHubProvider, GitLabProvider, GiteaProvider, RemoteRefInfo, RemoteRefProvider, |
There was a problem hiding this comment.
The import ordering is not alphabetical: GiteaProvider should come before GitHubProvider and GitLabProvider. Since the project uses edition 2024 and rustfmt (via pre-commit hooks), this should be self, GiteaProvider, GitHubProvider, GitLabProvider, RemoteRefInfo, RemoteRefProvider.
| self, GitHubProvider, GitLabProvider, GiteaProvider, RemoteRefInfo, RemoteRefProvider, | |
| self, GiteaProvider, GitHubProvider, GitLabProvider, RemoteRefInfo, RemoteRefProvider, |
| @@ -30,10 +30,16 @@ | |||
| //! Uses `glab api projects/:id/merge_requests/<number>`. Fork MRs require additional | |||
| //! API calls to fetch source/target project URLs. | |||
|
|
|||
There was a problem hiding this comment.
The Gitea doc section is separated from the preceding doc block by a blank line without //! prefix (line 32). This creates a separate doc comment block, unlike the GitHub and GitLab sections which are part of one continuous //! block. The blank line before the ## Gitea section should use //! to maintain a single continuous doc comment block, matching the convention used for the GitHub and GitLab sections above.
| //! |
| fn configure_mock_tea_env(cmd: &mut std::process::Command, mock_bin: &Path) { | ||
| cmd.env("MOCK_CONFIG_DIR", mock_bin); | ||
|
|
||
| let (path_var_name, current_path) = std::env::vars_os() | ||
| .find(|(k, _)| k.eq_ignore_ascii_case("PATH")) | ||
| .map(|(k, v)| (k.to_string_lossy().into_owned(), Some(v))) | ||
| .unwrap_or(("PATH".to_string(), None)); | ||
|
|
||
| let mut paths: Vec<std::path::PathBuf> = current_path | ||
| .as_deref() | ||
| .map(|p| std::env::split_paths(p).collect()) | ||
| .unwrap_or_default(); | ||
| paths.insert(0, mock_bin.to_path_buf()); | ||
| let new_path = std::env::join_paths(&paths) | ||
| .unwrap() | ||
| .to_string_lossy() | ||
| .into_owned(); | ||
|
|
||
| cmd.env(path_var_name, new_path); | ||
| } |
There was a problem hiding this comment.
configure_mock_tea_env is an exact duplicate of configure_mock_gh_env (defined at line 1513). Consider extracting a shared helper function (e.g., configure_mock_cli_env) to avoid the code duplication, following the pattern of the already shared configure_cli_not_installed_env.
| [31m✗[39m [31mtea api failed for PR #101[39m | ||
| [107m [0m No gitea login configured. To start using tea, first run | ||
| [107m [0m tea login add | ||
| [107m [0m and then run your command again. |
There was a problem hiding this comment.
This snapshot shows "tea api failed for PR #101" instead of the expected create conflict error ("Cannot create branch for gpr:101 — PR already has branch feature-auth"). Compare with the equivalent GitHub test snapshot (switch_pr_create_conflict.snap) which correctly shows the create conflict error after successfully fetching PR info.
The mock tea doesn't appear to be returning the configured response, so this test is exercising the API failure path rather than the --create conflict path it's intended to test. The same issue affects test_switch_gpr_same_repo and test_switch_gpr_fork — all three tests fall back to the API error path instead of testing their intended scenarios.
| �[31m✗�[39m �[31mtea api failed for PR #101�[39m | |
| �[107m �[0m No gitea login configured. To start using tea, first run | |
| �[107m �[0m tea login add | |
| �[107m �[0m and then run your command again. | |
| �[31m✗�[39m �[31mCannot create branch for gpr:101 — PR already has branch feature-auth�[39m |
| syntax, | ||
| number, | ||
| branch, | ||
| } => { | ||
| let name = ref_type.name(); |
There was a problem hiding this comment.
With the syntax field now passed explicitly to both RefCreateConflict and RefBaseConflict, the RefType::syntax() method (defined at line 63) is no longer called anywhere in the codebase. It should be removed to avoid dead code and a Clippy warning, or marked with #[allow(dead_code)] if it's intended for future use.
| syntax, | |
| number, | |
| branch, | |
| } => { | |
| let name = ref_type.name(); | |
| syntax: _, | |
| number, | |
| branch, | |
| } => { | |
| let name = ref_type.name(); | |
| let syntax = ref_type.syntax(); |
| /// Fetch PR information from Gitea using the `tea` CLI. | ||
| fn fetch_pr_info(pr_number: u32, repo_root: &Path) -> anyhow::Result<RemoteRefInfo> { | ||
| let api_path = format!("repos/{{owner}}/{{repo}}/pulls/{}", pr_number); |
There was a problem hiding this comment.
The API path uses {owner}/{repo} template variables (literal curly braces in the string), mirroring the gh api pattern. While gh api supports template variable substitution for {owner} and {repo} (resolving them from the current repo context), tea api may not support this same template mechanism. If tea api doesn't resolve {owner} and {repo}, the literal curly braces would be sent as part of the URL path, causing the API request to fail.
Please verify that tea api supports {owner} and {repo} template substitution. If it doesn't, you'd need to resolve the owner and repo from the git remote URL before constructing the API path.
| /// Fetch PR information from Gitea using the `tea` CLI. | |
| fn fetch_pr_info(pr_number: u32, repo_root: &Path) -> anyhow::Result<RemoteRefInfo> { | |
| let api_path = format!("repos/{{owner}}/{{repo}}/pulls/{}", pr_number); | |
| /// Resolve Gitea repository owner and name from the git remote URL. | |
| /// | |
| /// This inspects `git remote get-url origin` from the given `repo_root` and attempts to | |
| /// extract the last two path components as `<owner>/<repo>`, handling common SSH and | |
| /// HTTPS URL formats used with Gitea. | |
| fn resolve_owner_repo(repo_root: &Path) -> anyhow::Result<(String, String)> { | |
| let output = Cmd::new("git") | |
| .args(["remote", "get-url", "origin"]) | |
| .current_dir(repo_root) | |
| .run() | |
| .context("Failed to run git remote get-url origin")?; | |
| if !output.status.success() { | |
| let stderr = String::from_utf8_lossy(&output.stderr); | |
| let details = if stderr.trim().is_empty() { | |
| String::from_utf8_lossy(&output.stdout).trim().to_string() | |
| } else { | |
| stderr.trim().to_string() | |
| }; | |
| bail!("git remote get-url origin failed: {}", details); | |
| } | |
| let url = String::from_utf8(output.stdout).context("Remote URL is not valid UTF-8")?; | |
| let url = url.trim().trim_end_matches(".git"); | |
| // Strip scheme (e.g., https://, ssh://) if present. | |
| let without_scheme = if let Some(idx) = url.find("://") { | |
| &url[idx + 3..] | |
| } else { | |
| url | |
| }; | |
| // Strip leading user/host part (e.g., git@host:, host/). | |
| let path_part = if let Some(idx) = without_scheme.find(':') { | |
| &without_scheme[idx + 1..] | |
| } else if let Some(idx) = without_scheme.find('/') { | |
| &without_scheme[idx + 1..] | |
| } else { | |
| without_scheme | |
| }; | |
| let segments: Vec<&str> = path_part.split('/').filter(|s| !s.is_empty()).collect(); | |
| if segments.len() < 2 { | |
| bail!("Unable to extract owner and repo from git remote URL: {}", url); | |
| } | |
| let owner = segments[segments.len() - 2].to_string(); | |
| let repo = segments[segments.len() - 1].to_string(); | |
| Ok((owner, repo)) | |
| } | |
| /// Fetch PR information from Gitea using the `tea` CLI. | |
| fn fetch_pr_info(pr_number: u32, repo_root: &Path) -> anyhow::Result<RemoteRefInfo> { | |
| let (owner, repo) = resolve_owner_repo(repo_root)?; | |
| let api_path = format!("repos/{owner}/{repo}/pulls/{pr_number}"); |
…1322) ## Summary - Adds explicit guidance in the `review-pr` skill to verify whether a review was already posted before retrying a failed/stuck API call - Prevents intra-run duplicate reviews (distinct from cross-run dedup in #1139) Fixes #1321 ## Context Run 22784882686 posted the same review body twice on PR #1320 (4 minutes apart) because the bot thought the first API call was "stuck" and retried with `gh pr review --comment` without checking if the review had already been posted. ## Test plan - [ ] Next external PR review should not produce duplicate reviews even if the API call is slow 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
tbc, I would be up for merging something like this if we can get it into good shape. I think we can use |
worktrunk-bot
left a comment
There was a problem hiding this comment.
Thanks for iterating — pr: auto-detection with explicit config override is a cleaner direction than a separate gpr: prefix. Three blockers, though:
1. All six new test_switch_gpr_* tests fail locally (cargo test --test integration test_switch_gpr). The test bodies were updated to pass "pr:101" / "pr:42", but the committed snapshots still record "gpr:101" / "gpr:42" in the info.args block and still capture the "No gitea login configured" error text. cargo insta test --accept wasn't re-run after the gpr: → pr: rename, and the happy-path snapshots (switch_gpr_same_repo, switch_gpr_fork) still capture the same failure the previous review flagged — mock tea is never invoked because the ambiguous fallback tries gh first and git.example.com doesn't match is_gitea()'s contains("gitea") check.
2. ambiguous_pr_error produces a multiline error that trips debug_assert! at src/main.rs:1189 (Multiline error without context: ...). The {gh_err:#} / {tea_err:#} alternate-format includes each inner error's chain separated by newlines, which get embedded verbatim into the outer message. In debug builds this panics; in release it renders as a generic "Command failed" with a garbled gutter. This is visible in the --create test failure output. Inline suggestion below.
3. tea api repos/{owner}/{repo}/pulls/N was flagged in the previous review and is unchanged (src/git/remote_ref/gitea.rs:82). {owner}/{repo} is a gh-specific template; tea api is a raw HTTP client and sends the literal braces, which Gitea returns 404 for. The mock tests don't exercise this path because the mock binary accepts any argv. github.rs resolves owner and repo_name from the primary remote URL before formatting the path — the same pattern is needed here.
Aside: configure_mock_tea_env is still line-for-line configure_mock_gh_env (flagged last round). Not a blocker but worth extracting before this lands.
|
@worktrunk-bot can you try to get this into a good shape? lots has changed and lots might need rewriting. Consider how we can get confident that it'll work with Gitea. (as part of those changes, use |
|
Pushed 1. 2. Multiline 3. Mock With that fix, switching the test fixtures' remote URLs to Other cleanup:
Confidence note: the integration tests now mock the full happy path (same-repo, fork, base/create conflicts, not-found, missing CLI, forge.platform override). The remaining gap is end-to-end against a real Gitea instance — not something we can run in CI without standing up a server, but the API path/JSON shape are now grounded against the Gitea SDK and |
worktrunk-bot
left a comment
There was a problem hiding this comment.
The previous review's three blockers look resolved on this commit (owner/repo expansion, multi-line ambiguous error, mock tea actually exercised). However, CI is red on every required job because the branch is behind main and conflicts with two recently-merged changes:
Blocker — branch is out of date with main.
CI builds the pull_request merge ref, so it includes commits that landed since this branch was last updated. Two collisions:
-
#2498 changed
resolve_remote_ref_as_base's return type toanyhow::Result<(String, Option<(String, String)>)>(it now also returns the upstream(remote, branch)to set when paired with--create). This branch still usesanyhow::Result<String>, so all the new GitHub/Gitea/GitLab call sites inresolve_pr_base/resolve_base_ref(and theresolve_remote_ref_as_basedefinition itself) hitE0308: mismatched types. After mergingmain, the Gitea path will need to plumb the sameSome((remote, source_branch))through for--base pr:N --createto set up upstream correctly. -
tests/integration_tests/switch.rsonmaincallsconfigure_mock_gh_envat multiple sites (#2596 and adjacent). Renaming it toconfigure_mock_cli_envhere without rebasing leaves those sites unresolved, producingE0425: cannot find function configure_mock_gh_env. After merging, the new sites need to be renamed too (they're mechanical s/configure_mock_gh_env/configure_mock_cli_env/g).
Failing logs for reference: test (linux).
Minor — hand-rolled ANSI stripping.
strip_ansi in src/commands/worktree/switch.rs:141-158 reimplements what the project already does via ansi_str::AnsiStr::ansi_strip (used in diagnostic.rs, help.rs, styling/mod.rs, md_help.rs, etc., and ansi-str = "0.9" is already in Cargo.toml). The CLAUDE.md "Use Existing Dependencies" rule covers this case directly. Inline suggestion below.
Minor — choose_pr_provider URL parse can collapse to one helper.
primary_remote().ok().and_then(remote_url).and_then(GitRemoteUrl::parse) is the body of Repository::primary_remote_parsed_url(), which also documents that it intentionally uses the raw URL (not effective) for owner/namespace extraction — same rationale you wrote inline. Worth swapping in as a separate one-line cleanup, not a blocker.
I haven't verified end-to-end against a real Gitea instance — the mocked path is grounded against the SDK's JSON shape, and the JSON-decoding edge cases (extract_source_branch, missing head.repo, non-200 with parseable error message) are covered by unit/integration tests. Happy to push a fix commit for the merge once you've confirmed the direction.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Dismissing my approval — codecov/patch is failing at 79.35% (269/339 changed lines covered) and CLAUDE.md requires the patch check to pass before merging.
Looking at the cobertura report from the code-coverage job, three concrete gaps are reachable from new tests rather than dead error paths:
1. The ambiguous pr: fallback in src/commands/worktree/switch.rs:191-200 is never executed. When neither [forge] platform nor URL detection identifies the host (e.g. a self-hosted Gitea on code.example.com), resolve_pr_target falls through to the Ambiguous arm that calls GitHub then Gitea in sequence and wraps the dual error via ambiguous_pr_error. Every existing test_switch_pr_* either sets gitea.example.com (matched by is_gitea()) or github.com (matched by is_github()), so this arm — and the flatten_error/ambiguous_pr_error helpers it depends on (switch.rs:145-171) — is dark. A test pointing origin at e.g. https://code.example.com/owner/repo.git with mocks for both gh and tea failing would cover all three.
2. The Gitea path in resolve_pr_base (switch.rs:214) is never executed. test_switch_base_pr_sets_upstream exercises the same-repo upstream-tracking codepath against GitHub, but no test runs wt switch --create <branch> --base pr:<n> against a Gitea remote. Cloning that test with setup_mock_tea and a Gitea URL would cover this and the corresponding Ambiguous-base fallback at lines 215-222.
3. Several Gitea API error paths in src/git/remote_ref/gitea.rs are uncovered. Specifically the malformed-host fallback (line 184), the head.repo == null "fork was deleted" branch (line 163-170), the empty-label / empty-ref returns in extract_source_branch (lines 235, 246), and the remote_url / parse error guards (lines 95, 97). Most of these are 1-3 line bails; the deleted-fork case is the most worth exercising since a real Gitea instance can return that shape. Compare against github.rs which has corresponding unit tests for each error response shape.
The 401/403/non-success branches at lines 127-140 are also uncovered but mirror what github.rs does and could be exercised by setting MOCK_RESPONSE_EXIT=1 with a JSON error body in the mock fixture.
Happy to push a commit adding the ambiguous-fallback test plus a Gitea variant of test_switch_base_pr_sets_upstream if you'd like — those two together should land patch coverage above the threshold without much new code.
worktrunk-bot
left a comment
There was a problem hiding this comment.
Dismissing my approval — codecov/patch is failing at 79.35% (269/339 changed lines covered) and CLAUDE.md requires the patch check to pass before merging.
Looking at the cobertura report from the code-coverage job, three concrete gaps are reachable from new tests rather than dead error paths:
1. The ambiguous pr: fallback in src/commands/worktree/switch.rs:191-200 is never executed. When neither [forge] platform nor URL detection identifies the host (e.g. a self-hosted Gitea on code.example.com), resolve_pr_target falls through to the Ambiguous arm that calls GitHub then Gitea in sequence and wraps the dual error via ambiguous_pr_error. Every existing test_switch_pr_* either sets gitea.example.com (matched by is_gitea()) or github.com (matched by is_github()), so this arm — and the flatten_error/ambiguous_pr_error helpers it depends on (switch.rs:145-171) — is dark. A test pointing origin at e.g. https://code.example.com/owner/repo.git with mocks for both gh and tea failing would cover all three.
2. The Gitea path in resolve_pr_base (switch.rs:214) is never executed. test_switch_base_pr_sets_upstream exercises the same-repo upstream-tracking codepath against GitHub, but no test runs wt switch --create <branch> --base pr:<n> against a Gitea remote. Cloning that test with setup_mock_tea and a Gitea URL would cover this and the corresponding Ambiguous-base fallback at lines 215-222.
3. Several Gitea API error paths in src/git/remote_ref/gitea.rs are uncovered. Specifically the malformed-host fallback (line 184), the head.repo == null "fork was deleted" branch (line 163-170), the empty-label / empty-ref returns in extract_source_branch (lines 235, 246), and the remote_url / parse error guards (lines 95, 97). Most of these are 1-3 line bails; the deleted-fork case is the most worth exercising since a real Gitea instance can return that shape. Compare against github.rs which has corresponding unit tests for each error response shape.
The 401/403/non-success branches at lines 127-140 are also uncovered but mirror what github.rs does and could be exercised by setting MOCK_RESPONSE_EXIT=1 with a JSON error body in the mock fixture.
Happy to push a commit adding the ambiguous-fallback test plus a Gitea variant of test_switch_base_pr_sets_upstream if you'd like — those two together should land patch coverage above the threshold without much new code.
codecov/patch failed (79.35%) — see follow-up review for analysis
worktrunk-bot
left a comment
There was a problem hiding this comment.
codecov/patch is up from 79.35% → 92.6% on this push, but still failing. Two of the three specific gaps from my last review remain:
-
resolve_pr_baseGitea path is still uncovered (src/commands/worktree/switch.rsline 214 and the Ambiguous-base fallback at 215-222). The previous review suggested cloningtest_switch_base_pr_sets_upstreamwithsetup_mock_teaand agitea.example.comremote — that wasn't added. This is the largest remaining patch-coverage gap. -
The
Okarms of the Ambiguous fallback inresolve_pr_target(lines 192, 195, 198) are still dark — both new ambiguous tests (test_switch_pr_no_remote_falls_back_to_ambiguous,test_switch_pr_ambiguous_both_providers_fail) only exercise the Err-then-Err path. A test whereghreturns a valid PR JSON for an Ambiguous-detected remote would cover line 192; one whereteareturns valid JSON afterghfails would cover 195; one where the second (tea) error is itself aGitErrorwould cover 198.
Separately, while reviewing the new snapshots, the snapshot for switch_pr_forge_platform_invalid reveals a behavior/message mismatch in choose_pr_provider that the new test happens to encode as expected:
The warning emitted at src/commands/worktree/switch.rs:107 says "Falling back to remote detection", but the _ arm at line 110 returns Ok(PrProviderChoice::Ambiguous) — which skips URL detection entirely and goes straight to trying both providers in sequence. In the switch_pr_forge_platform_invalid snapshot you can see this concretely: with forge.platform = "bitbucket" and a github.com remote, wt switch --create pr:101 runs two "Fetching PR #101..." attempts (gh then tea) and emits the wrapped "with both providers" error, instead of detecting GitHub from the URL, fetching once, and surfacing GitHub's specific RefCreateConflict ("Cannot create branch for pr:101 — PR already has branch feature-auth"). The test docstring at tests/integration_tests/switch.rs:3703-3704 also asserts the URL-detection behavior in prose, which contradicts what the snapshot captures.
Either the warning text + test docstring should change to match the actual behavior ("Falling back to trying both providers"), or the _ arm in choose_pr_provider should fall through to the URL-detection block below instead of early-returning Ambiguous. The latter matches what users with a typo'd forge.platform likely expect, and would surface a clearer single-provider error instead of the ambiguous-wrapper message.
worktrunk-bot
left a comment
There was a problem hiding this comment.
eb540d3f covers most of the previously-flagged gaps. Three small items still remain from the previous review — codecov/patch will likely still be short:
-
resolve_pr_targetAmbiguous fallback line 195 (the secondOk(target) => Ok(target), gh fails then tea succeeds) is still uncovered._gh_succeedscovers line 192 and_tea_returns_cli_errorcovers line 198, but no test exercises the path where gh returns an error and tea returns valid PR JSON. -
resolve_pr_baseGitea-detected path (switch.rs:214) is still uncovered.test_switch_create_base_pr_ambiguouscovers the Ambiguous arm at 215-222; the direct Gitea arm needs a test withgitea.example.com(orforge.platform = "gitea") plussetup_mock_teareturning a valid same-repo PR — same shape astest_switch_base_pr_sets_upstreambut pointed at Gitea. -
gitea::fork_remote_urlinfind_remote(src/git/remote_ref/mod.rs:168-172) is uncovered. Fires when a Gitea PR is fetched but no local remote points to the base repo. A small unit test inmod.rsconstructing aRemoteRefInfowithPlatformData::Giteaand asserting onfind_remoteagainst a repo without a matching remote would cover it.
Behavior note — forge.platform invalid arm. The warning at src/commands/worktree/switch.rs:107 says "Falling back to remote detection", but the _ arm at line 110 returns Ok(PrProviderChoice::Ambiguous) which skips the URL-detection block below entirely. The switch_pr_forge_platform_invalid snapshot confirms this: with forge.platform = "bitbucket" and a github.com remote, the output shows two "Fetching PR #101..." attempts (gh then tea) — not the single GitHub-only fetch the warning text and the test docstring at tests/integration_tests/switch.rs:3703-3704 describe. Either the warning + docstring should change to "trying both providers", or the _ arm should fall through to URL detection (matching what users with a typo'd forge.platform likely expect, and producing a clearer single-provider error). This was item 3 in the previous review and isn't addressed here.
I'm happy to push a small commit (Gitea base test + the missing-remote unit test + an Ambiguous-tea-wins integration test, plus your preferred fix for the warning/behavior mismatch) — let me know which direction you want for the forge.platform invalid case.
…licates (#395) ## Outcome evidence `tend-review` run [25499066953](https://github.com/max-sixty/worktrunk/actions/runs/25499066953) on worktrunk PR [#2641](max-sixty/worktrunk#2641) posted **two `APPROVED` reviews 4 seconds apart** from a single completed session — review [4244431335](max-sixty/worktrunk#2641 (review)) at 13:35:57Z and [4244431873](max-sixty/worktrunk#2641 (review)) at 13:36:01Z, both empty-body, both on the same head SHA `55ef4fba`. Only one `tend-review` run touched this SHA, so this is not the documented concurrency-cancel `Non-issue` (no SIGTERM'd siblings here). Decision chain in session log `bcfcce99-ae59-4201-a411-fe82f2b074bc.jsonl`: 1. **13:35:55Z** — bot ran a guarded combined script: `read CURRENT_HEAD/PR_STATE`, HEAD-match check, OPEN-state check, `ALREADY_POSTED` check, then `gh pr review 2641 --approve -b ""` as the script's last command. The first `APPROVED` posted at 13:35:57Z. Tool-result showed only the prelude's `echo` lines (`CURRENT_HEAD=… PR_STATE=OPEN ALREADY_POSTED=`); the approve itself produces no stdout on success. 2. **13:36:00Z** (5s later) — bot issued a bare standalone `gh pr review 2641 --approve -b ""` with no preceding text. The second `APPROVED` posted at 13:36:01Z. 3. **13:36:04Z** — bot queried `reviews[-1]`, saw the latest entry was `APPROVED`, and wrote "Approval posted on HEAD" (singular) to the conversation log, never noticing the duplicate. The prelude's silence was indistinguishable from "approve didn't fire", so the model retried "to be sure". ## Root cause The `### 5. Submit` recipe in `plugins/tend-ci-runner/skills/review/SKILL.md` shows `gh pr review --approve -b ""` with no positive confirmation. `gh pr review --approve` is silent on success, so combining it into a guarded script (where the only stdout is the prelude's `echo`) leaves the model with no visible success signal. The retry follows. ## Fix Append `&& echo "✓ approved"` to the recipe so the tool-result shows explicit confirmation, and add one sentence explaining why future maintainers should not strip the echo as cosmetic noise. One-line code change, one sentence of explanation. ## Gate assessment Evidence: https://gist.github.com/9675d1510da32c68a68c1d45137b21e0 (current month) plus prior month's gist `44ab1483b29406255dd36141650c894d`. Three cumulative occurrences of single-session duplicate-review-POST across the past two months, all from `tend-review` sessions, all on `worktrunk`: | Date | Run | PR | Review type | Notes | |---|---|---|---|---| | 2026-04-29 | [25089599666](https://github.com/max-sixty/worktrunk/actions/runs/25089599666) | [#2454](max-sixty/worktrunk#2454) (Dependabot) | 2 × `APPROVED`, empty-body | Same combined-script-then-bare-retry shape; explicitly proposed `&& echo` as the fix | | 2026-05-06 | [25454510442](https://github.com/max-sixty/worktrunk/actions/runs/25454510442) | [#1320](max-sixty/worktrunk#1320) | 2 × `COMMENTED`, byte-identical 2476-byte bodies | Different verb path (`gh api .../reviews POST`) but same single-session-duplicate-POST signature | | 2026-05-07 | [25499066953](https://github.com/max-sixty/worktrunk/actions/runs/25499066953) | [#2641](max-sixty/worktrunk#2641) | 2 × `APPROVED`, empty-body | This run; recipe-level `--approve` shape, identical to the April 29 case | - **Classification**: stochastic (model has decision points; not every replay would retry). Restricted to the recipe-level `--approve` shape: 2 cumulative occurrences (April 29, today). Across the broader single-session-duplicate-POST family: 3 occurrences. - **Gate 1 (Confidence)**: stochastic with 3 cumulative occurrences ≥ High threshold (2-3). For the narrower `--approve`-only shape, 2 occurrences also meets the High threshold. - **Gate 2 (Magnitude)**: targeted recipe fix (one line of code, one sentence of explanation) — Normal evidence bar, met by Gate 1 outcome. - **Both gates pass**, with the April 29 entry's own forward statement: "if a second occurrence appears … warrant a PR even at the stochastic tier (and 2 would also satisfy the 'High' 2-3 threshold)." ## Why now The April 29 entry recorded the exact same root cause and the exact same fix this PR applies, and put the bot on a watch-list pending a second occurrence. Today's run on PR #2641 is that second `--approve` occurrence (third cumulative across all single-session duplicate review shapes). The watch-list has tripped. ## Why not broader Scope is constrained to the `--approve` recipe because that is where the evidence concentrates and where the call site is unambiguous. The COMMENTED API path (`gh api .../reviews POST`) returns JSON on success, so the silent-success-retry mechanism does not apply directly there; if its single occurrence recurs, a follow-up PR can address that path. Same for `gh api .../dismissals -X PUT` — referenced in the new explanatory sentence as guidance, not modified directly. Smallest change that addresses the observed pattern. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Add Gitea support for `pr:<N>` using the `tea` CLI and wire it into the unified remote-ref resolution flow. - Introduce `GiteaProvider` implementing `RemoteRefProvider` (`src/git/remote_ref/gitea.rs`) that calls `tea api` to fetch PR info, extract source branch, and compute fork push URLs. - Add `PlatformData::Gitea` and update `RemoteRefInfo` helpers (source_ref, prefixed local branch) so forked Gitea PRs behave like GitHub forks. - Add `GitRemoteUrl::is_gitea()` and export `GiteaProvider` from `remote_ref` module. - Implement provider selection logic: - honor `[forge] platform` config override, - detect from primary remote URL, - fall back to ambiguous mode that tries GitHub (`gh`) then Gitea (`tea`) before erroring. - surface helpful error messages when detection indicates GitLab or when both providers fail. - Update switch command logic to use the new selection and resolution functions. - Update CLI docs and help text to mention Gitea and explain provider detection/fallback. - Add integration tests and snapshot fixtures for Gitea PR flows and error cases. No breaking changes to public behavior; `pr:` now supports both GitHub and Gitea repositories.
Address the three blockers from review:
- **Owner/repo resolution**: `tea api repos/{owner}/{repo}/pulls/N` was passing the literal placeholder string. While `tea api` does support template expansion, it relies on `tea`'s own context resolver (login + remote config), not on the running directory. Mirror what `github.rs` does: parse the primary remote URL and pass an already-expanded path so the call works without depending on `tea`'s context.
- **Multiline ambiguous error**: `ambiguous_pr_error` used `{err:#}` which inlined the styled multi-line `GitError::CliApiError` Display, tripping `main.rs`'s "multiline error without context" `debug_assert\!`. Add a `flatten_error` helper that walks the cause chain, strips ANSI, and joins on `: ` so the outer error stays single-line.
- **Provider detection through insteadOf**: `choose_pr_provider` was using `effective_remote_url`, which applies `insteadOf` rewrites. Tests rewrite `https://gitea.example.com/...` to a local bare path, so detection saw the local path (no host) and fell through to the ambiguous flow. Switch to the raw `remote_url` — `insteadOf` is for transport, not forge identity. (This is the same rationale already documented in `github.rs`.)
Tests: rename `test_switch_gpr_*` → `test_switch_pr_gitea_*`, switch fixture URLs to `gitea.example.com` so `is_gitea()` matches and the ambiguous fallback isn't taken (mock `gh` doesn't need to be set up). Add `test_switch_pr_gitea_forge_platform_override` to exercise the explicit `[forge] platform = "gitea"` config path with a non-Gitea remote URL.
Also extract `configure_mock_cli_env` (was duplicated as `configure_mock_gh_env` and `configure_mock_tea_env`), align `extract_source_branch` with Gitea's actual `head.ref` shape (bare branch name, not `refs/heads/...`), and update fixtures accordingly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rebasing onto current main pulls in `resolve_remote_ref_as_base` returning `(String, Option<(String, String)>)` (introduced for `--base pr:N`/`mr:N` upstream tracking config). Update `resolve_pr_base` to the same signature so the ambiguous fallback compiles. Also rename two stragglers from `configure_mock_gh_env` → `configure_mock_cli_env` that landed on main since the fix commit was authored, and switch the new `flatten_error` helper to use the project's existing `ansi-str` crate (`AnsiStr::ansi_strip`) rather than hand-rolling an ANSI parser — `no-manual-ansi` enforces this. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Six new integration tests for paths that codecov flagged uncovered: - `forge.platform = "gitlab"` + `pr:` bails with "use mr:". - Invalid `forge.platform` warns and falls back to remote-URL detection. - A GitLab remote URL with `pr:` bails with "use mr:". - Ambiguous fallback (non-GitHub, non-Gitea host) where both providers return 404 — exercises `flatten_error` and `ambiguous_pr_error`. Snapshot confirms the wrapped error stays on a single line. - Gitea 401 / 403 hit their dedicated bail messages instead of falling through to the generic CLI error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three more unit tests in `gitea.rs` for `extract_source_branch` edge cases: - Label with empty branch part (e.g. "owner:") falls through to the ref-name branch. - Bare "refs/heads/" strips to empty → returns None. - Empty ref returns None. One integration test for `wt switch pr:N` in a repo with no parseable remote URL — exercises the `Ok(PrProviderChoice::Ambiguous)` early return when remote-URL detection fails, and the ambiguous fallback path with non-GitError errors (covers `flatten_error` for that shape). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Seven more integration tests for paths codecov flagged uncovered: - Ambiguous fallback where gh succeeds first (tea is never called). - Ambiguous fallback where tea returns a `CliApiError` (`GitError`) — short-circuits the wrapper and propagates the structured error. - `--base pr:N` in the ambiguous fallback (exercises `resolve_pr_base`'s ambiguous branch). - Gitea returns malformed JSON (parse-error path). - Gitea returns a non-401/404/403 error (generic `cli_api_error` fallback). - Gitea PR has empty `head.label` and `head.ref` — `extract_source_branch` returns None, `fetch_pr_info` bails. - Gitea PR with `head.repo: null` (deleted fork). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Both `github.rs` and `gitea.rs` parsed the host segment out of the API's
`html_url` with the same 7-line chain. Lift it into
`extract_host_from_html_url` in `mod.rs` next to the other shared helpers
(`cli_api_error`, `cli_config_value`).
Also collapse the `let fork_push_url = if is_cross_repo { Some(...) } else
{ None }` block in both providers into `is_cross_repo.then(|| ...)`.
Net: -15 lines, no behavior change. GitLab is left alone — its host comes
from `GitRemoteUrl::parse` on a URL slice, not the html_url shape.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ection Previously when the remote URL didn't match `is_github`/`is_gitea`/`is_gitlab` the dispatcher fell into an Ambiguous arm that ran both `gh api` and `tea api`, then wrapped both errors into a single multi-provider message. The wrapped error was hard to read and over-engineered for what's basically a one-time config decision. Drop the Ambiguous variant and instead inspect each tool's local auth state: - `github::is_authed_for(host)` runs `gh auth token --hostname <host>` and checks the exit code. Local-only (reads `~/.config/gh/hosts.yml`). - `gitea::is_authed_for(host)` reads `~/.config/tea/config.yml` directly (honoring `$XDG_CONFIG_HOME` and the legacy `~/.tea/tea.yml` fallback) and checks for any `logins[].url` whose host matches. Avoids `tea login helper get`, which can trigger an OAuth refresh on success. Dispatch order is now: `forge.platform` config override → URL substring match → tea-authed-and-not-gh-authed → default GitHub. The GH default keeps error messages clean for the common case; self-hosted Gitea users with `tea login add` still auto-route correctly without setting `forge.platform`. Removes `flatten_error`, `ambiguous_pr_error`, and the four codecov-driven ambiguous-fallback integration tests (`_both_providers_fail`, `_tea_returns_cli_error`, `_create_base_pr_ambiguous`, `_ambiguous_gh_succeeds`). Replaces them with one self-hosted-defaults-to-GH test, one tea-authed-routes-to-Gitea test, and a pure unit test for the config parser. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A typo like `forge.platform = "bitbuckt"` would previously emit a warning and then silently re-route through URL detection / CLI auth / GH default — so the command might "work" by accident, but the user wouldn't see they had a configuration error to fix. Bail with a clear single-line error listing the accepted values (`github`, `gitea`, `gitlab`) so the user is told to fix the typo. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Default user-facing copy ("pr:{N}", help table, examples) now says "GitHub PR"
rather than "GitHub/Gitea PR" — Gitea is mentioned once, in a single
paragraph at the bottom of the Pull-requests section, tagged experimental.
This keeps the help concise for the common GitHub case and signals that
Gitea support is still settling, without burying that it exists.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex review caught: when a Gitea PR's source repo/branch is deleted, the API returns `head.label = "unknown repository"` and `head.ref = "refs/pull/<n>/head"` (or `pull/<n>/head`) rather than null. `extract_source_branch` was accepting those as branch names, so `wt switch pr:N` would proceed to fetch a non-existent branch instead of bailing with the deleted-source error. Tighten the parser with `is_real_branch_name()`: a candidate must be non-empty, have no whitespace, and not start with `refs/`/`pulls/`/`pull/`. Placeholders fail all three; legitimate branch names pass. Also update `forge.platform` docs/help to list `"gitea"` as a valid value (previously only mentioned `"github"`/`"gitlab"` — contradicted the switch docs that recommend the gitea setting). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
One observation on the new placeholder-rejection logic in gitea.rs — the change is correct but, for the head.repo == null shape, the user now sees a less-specific error than before. Details inline.
Otherwise this looks good: the forge.platform bail is a clear UX win over the previous silent fall-through, the docs reposition is appropriate for experimental scope, and the is_real_branch_name helper is well-covered by the new unit tests.
…c error Auto-reviewer caught: my prior placeholder-rejection change made `extract_source_branch` return None for the `head.repo == null` shape (Gitea emits "unknown repository" / "refs/pull/<n>/head" placeholders there). `fetch_pr_info` then bailed with the generic "has no source branch" message instead of the specific "source repository was deleted" message users got before. Reorder `fetch_pr_info` so `head.repo`'s null check runs before `extract_source_branch`. Now: - `head.repo == null` → "source repository was deleted" (specific, helpful). - `head.repo == Some + placeholder label/ref` → "no usable source branch — head.label/head.ref carry placeholders" (rarer, generic). Refactor `extract_source_branch(&TeaPrRef)` into `extract_source_branch_from_parts(label, ref_name)` so we can destructure `response.head` and move `repo` out without a partial-move conflict. Tests updated for the new signature. Also add a Gitea section to `remote_ref::mod.rs`'s module docstring for consistency with GitHub/GitLab. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Release v0.50.0. Highlights: - Experimental Azure DevOps support (#1256, thanks @mikeyroush; fixes #1144 from @dlecan) — `wt switch pr:<N>`, `wt list --full`, and `wt config show --full` recognize Azure DevOps via the `az` CLI. - Experimental Gitea CI-status detection (#2702) on top of the Gitea `pr:` shortcut (#1320, thanks @SjB). - Hooks now resolve `.config/wt.toml` from the worktree they act on — the primary-worktree fallback is gone, and `post-remove` reads the removed worktree's config (snapshotted before removal). Approval prompts collect hook commands from the same worktree. Breaking change for setups that relied on the primary-worktree fallback; the changelog entry has the recovery action. (#2690, #2703, #2714, #2717, #2701, #2708, #2727, #2736, #2748) - The `wt switch` picker's `alt-r` removal no longer runs unapproved project hooks (#2746) — the picker's removal path is now routed through `handle_remove_output` and consults the existing approval state read-only. - `wt config alias show` with no name lists every alias's full definition (#2684, #2691); `wt --help` switches to a compact aliases pointer (#2688). - `wt list --branches` warm-run perf: SHA-keyed cache for `main↕` and `Remote⇅` ahead/behind counts; shared push-remote URL and local-branch scan (#2704, #2718, #2673). - Claude Code plugin ships the `wt-switch-create` skill (#2737, thanks @onetom for #2631). See `CHANGELOG.md` for the full list (8 Improved, 5 Fixed, 5 Internal). semver-checks reports breaking library-API changes (new enum variants without `#[non_exhaustive]`, removed `Branch::github_push_url`, new trait method on `RemoteRefProvider`), which mandates at minimum a minor bump pre-1.0.
Add experimental Gitea PR support to
wt switch pr:<number>.The
pr:syntax already resolved GitHub PRs; this teaches it to also resolve Gitea PRs via theteaCLI. GitLab continues to usemr:.Dispatch
pr:Nnow goes throughchoose_pr_provider:[forge] platformin.config/wt.tomlif set (github/gitea/gitlab)github/gitea/gitlab)teais configured for this host (per~/.config/tea/config.yml) butghis not (pergh auth token --hostname <host>), pick GiteaThere is no longer an "ambiguous" fallback that tries both providers and wraps both errors — users on self-hosted Gitea instances either run
tea login add <host>(auto-detected) or set[forge] platform = "gitea".New code
src/git/remote_ref/gitea.rs—GiteaProviderimplementingRemoteRefProviderviatea api repos/<owner>/<repo>/pulls/<n>; reuses the sharedcli_api_error/run_cli_apihelpers.src/git/remote_ref/info.rs—PlatformData::Gitea { host, head_owner, head_repo, base_owner, base_repo }, wired intosource_ref(),prefixed_local_branch_name(), andfind_remote().src/git/url.rs—GitRemoteUrl::is_gitea().Shared helpers introduced in this PR:
mod.rs::extract_host_from_html_url()(used by github + gitea; identical 7-line chains collapsed).github::is_authed_for()(wrapsgh auth token --hostname).gitea::is_authed_for()(reads tea's config.yml; never invokesteato avoid OAuth refresh on lookup).Docs
User-facing copy says "GitHub PR" by default; one paragraph in
wt switch --helpmentions Gitea support, marked experimental.Tests
teanot installed,forge.platformoverrides, GitLab-remote-with-pr:bail, self-hosted defaults-to-GitHub, and self-hosted-with-tea-login routes-to-Gitea.extract_source_branchedge cases and the tea config parser.Compatibility
No CLI flag or config file changes. The
teaCLI is only required for Gitea PRs; GitHub-only users see no change.