mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
codex/remove-codex-cloud-specific-tests
34 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Worktrunk Bot
|
aa4e527f35 |
refactor(git): drop Branch::push_remote, unused since the @{push} fix (#3833)
Nightly sweep finding. `Branch::push_remote()` has had no callers — production or test — since [#769](https://github.com/max-sixty/worktrunk/pull/769) (Jan 2026), which replaced its only caller in the CI-status path with `push_remote_url()`. This drops it, and fixes a stale doc comment that named it. ## Why it went unnoticed It's `pub` on a public lib type, so rustc's `dead_code` lint never fires on it. `git log -S` puts the last caller's removal in `c4f1c0730` (#769): `gh pr checkout` sets `branch.<name>.pushremote` to a URL rather than a remote name, and `@{push}` — which is what `push_remote()` resolves through — fails in that case. `push_remote_url()` uses `%(push:remotename)` instead, which handles both. So the method isn't merely unused; its docstring still advertises the `@{push}` resolution chain that #769 established is wrong for the case the codebase actually hits, which makes it read as a live alternative to the function that superseded it. ## The stale comment `setup_push_tracking` in `tests/integration_tests/default_branch.rs` was documented as existing so `branch.push_remote()` and `github_push_url()` work. The first is what this PR deletes; the second has never existed anywhere in the tree (`grep` finds that comment as its only occurrence). Its four call sites all call `push_remote_url()`, so the comment now names that. ## Verification No regression test accompanies this — there's no behavior to pin, since the deleted method had no callers to change the behavior of. The proof is negative and the compiler carries it: `cargo build --all-targets` and `cargo clippy --all-targets -- -D warnings` both pass, which they could not if any call site remained. Also ran the suites covering the touched area: `cargo test --test integration default_branch` (63 passed) and `cargo test --lib git::repository` (178 passed). <details><summary>Confirming there are no callers</summary> Every mention of the bare identifier in the tree before this change: ``` src/git/repository/branch.rs:151: pub fn push_remote(&self) -> Option<String> { # the definition src/git/repository/branch.rs:186: let push_remote = self # local var in push_remote_url src/git/repository/branch.rs:197: if push_remote.contains("://") ... # same local src/git/repository/branch.rs:198: Some(push_remote) # same local src/git/repository/branch.rs:200: self.repo.effective_remote_url(...) # same local tests/integration_tests/default_branch.rs:439: /// ... `branch.push_remote()` ... # the stale comment ``` The lines in `push_remote_url` are a local binding of the same name, not calls. `switch.rs:808` writes the `branch.<name>.pushRemote` git-config key and is unrelated. </details> --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> |
||
|
Maximilian Roos
|
dd453304ed |
test: fold the mock stub into the wt binary (#3712)
`cargo test --test integration` neither built nor rebuilt `mock-stub`: a
target filter deselects the dummy test that pulled it in, so a fresh
tree panicked ("mock-stub binary not found") and a warm one could run a
stale stub. The chain that compensated — the separate helper package,
its dummy `builds.rs`, the `default-members` entry, nextest's
experimental `build-bins` setup script, `workspace_bin()` — existed only
because cargo-dist ships every `[[bin]]`, and carried a TODO to collapse
once that changed. dist 0.30.2 does support per-binary exclusion now
(`[dist.binaries]`, since 0.29.0; verified with `dist plan`), but the
TODO's plan has a hole it predates: `cargo install worktrunk` installs
every feature-satisfied `[[bin]]`, and dist config doesn't govern
crates.io installs.
So the mock commands are now the `wt` binary itself. `mock_commands`
links `wt` under the mock's name (`gh`, `glab`, …), and `main()`
dispatches to the ported playback (`testing::mock_stub`) when
`WORKTRUNK_TEST_MOCK_CONFIG_DIR` is set, argv[0] is a foreign name, and
the config dir holds `<argv0>.json` for it. The existence check keeps wt
under a foreign argv[0] *without* a config being wt — the
argv0-validation security test symlinks it as `wt;touch` and must reach
wt's own rejection. The shipped binary already compiles the whole
`testing` module unconditionally, so this adds no new category of test
code to it. Windows links `wt.exe` with `hard_link` (copy fallback for
cross-drive dirs); the debug binary is ~67 MB, so per-mock copies stay
the fallback.
Every runner is now safe by construction — cargo rebuilds a package's
own binaries whenever its integration tests build, so there is no
separate artifact to go missing or stale. Deleted: the helper package,
the setup script plus `experimental = ["setup-scripts"]`, the
`default-members` trick, `workspace_bin()`, and `wt_bin()`'s dead
compile-time branch (unit-test targets get neither the runtime variable
nor the `option_env!` value, so the runtime resolution is the one
mechanism).
Validated locally: the pre-merge gate's full suite passes (4542/4542;
its doc step also caught unescaped `argv[0]` intra-doc links in the new
comments, fixed and `cargo doc -Dwarnings` re-verified). With all stub
artifacts purged from `target/`, plain `cargo test --test integration`
on mock-dependent tests builds them and passes — the command that used
to hit the trap. Two integration tests pin the dispatch's argv[0] edges
(an empty argv[0], and a non-UTF8 one), alongside the existing
`wt;touch` carve-out test.
The first commit is the investigation that preceded the fix: it verified
the `wt` binary itself was never subject to the staleness the mock-stub
was, and documented that in `tests/CLAUDE.md`; the fix then narrows that
paragraph further, since the gap it scoped no longer exists.
A two-reviewer subagent round (one prosecuting the diff against the
failure modes documented in the repo's own mock history — the #401/#407
Windows era, #547, #654, #127, #2544, #2730, #2744 — the other
adversarial) then hardened the dispatch. The reserved-name guard is
case-insensitive, matching the config probe, which goes through a
filesystem that equates `WT.json` with `wt.json` on macOS and Windows;
`command_name()` reads `args_os` — `env::args()` panics on a non-Unicode
argument, and this runs inside `main()` on every invocation (caught by
the tend review) — and returns `None` for a degenerate argv[0] instead
of panicking; the `.exe` suffix is stripped explicitly rather than via
`file_stem`, so a dotted mock name (`python3.11`) resolves identically
on every platform; and `copy_mock_binary` is now private —
`MockConfig::write` writes `<name>.json` before linking and is the only
way to create a mock, so a link cannot exist without its config, and the
dispatch's missing-config fall-through can only mean "wt under a foreign
name" (the argv0-validation tests' `wt;touch`), never a half-configured
mock that silently runs real wt with the mocked tool's arguments. The
`Option<&str>` mock helpers whose `None` arm produced exactly such
configless links lost the arm (every caller passed `Some`), and 25
redundant standalone link calls went with it.
The review also surfaced the one remaining spawn-a-stale-binary path
outside the suite: `wt-perf timeline` resolved a sibling `wt` by path,
checked only existence, and told the user to build it manually — so
`cargo run -p wt-perf -- timeline` after a `src/` edit silently measured
stale code. It now builds `wt` first and takes the artifact path from
cargo's `--message-format=json` report rather than deriving a sibling
location, so target-dir and profile overrides can't divert the build
away from where it's resolved; a release wt-perf builds and measures a
release wt. The build runs before the timeline's wall-clock measurement
starts, cargo's progress streams on stderr, and stdout keeps the
`--chrome` JSON contract.
> _This was written by Claude Code on behalf of max-sixty_
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
305f8fcd19 |
fix(gitea): key tea api errors on the HTTP status line, not the response shape (#3713)
`tea api` copies the response body to stdout and exits 0 whatever the
HTTP status, so both Gitea call sites guessed from the body: a `message`
key meant the request had failed, anything else was the resource. `tea
api --include` writes the status line and headers to stderr and leaves
stdout the bare body, so the status is available after all.
## Availability
`--include` shipped with the `api` subcommand itself in **v0.12.0** —
`cmd/api.go` doesn't exist at v0.11.1 — and the implementation is
byte-identical through v0.15.1. A `tea` old enough to lack the flag has
no `api` to send it to, so nothing that can make this call rejects it.
tea's own `api_test.go` passes flags ahead of the endpoint (`{"-d",
"@file", "/test"}`), which settles the argument order.
## What the status buys
Two things the shape check couldn't do:
**A body Gitea didn't write is now an error.** A reverse proxy's HTML
page has no `message` key, so it used to read as the resource — and `wt
list` then logged `Failed to parse tea api pulls JSON`, blaming a Gitea
API change for a gateway that never reached Gitea. Verified by restoring
the old key and re-running: `proxy_page_is_an_error` fails against it
and passes with the status.
**Retriability stops being a text sniff.** `is_retriable_error` was
looking for `429` or `connection` somewhere in Gitea's prose, which
never carries the code. 429 and 5xx are now the server saying "later";
every other 4xx is a token or a missing resource that repeating the call
won't change. `gh` and `glab` still sniff because they bury the status
inside prose.
Visible change: a blanked 500 shows the error indicator where it used to
paint the same blank cell as a healthy branch with no CI.
`gitea_blanked_500` and `gitea_not_found` pin both halves of that
contrast.
## The body's three states
The status decides; the body only supplies text, and its three states
are three different things to say — so `api_error_message` reports all
three rather than flattening them. The switch path prints the code in
its headline:
```
✗ Gitea API error 404 for PR #9999 on owner/test-repo: pull request does not exist [index: 9999]
✗ Gitea API error 500 for PR #101 on owner/test-repo, but the response carried no message — Gitea hides 5xx messages from non-admin tokens
✗ Gitea API error 502 for PR #101 on owner/test-repo, and the response body is not one Gitea sends
```
An exit-0 `tea` that writes no status line is a backstop rather than a
fall-through: `--include` prints the moment the response does, so its
absence means nothing arrived, and reading stdout as the resource there
is the failure the status line exists to prevent.
`switch_pr_gitea_no_status_line` covers it.
With the CI path no longer reading the message, `api_error_message`
drops to module-private.
## Tests
Every mock `tea` response now carries the two streams a real one writes,
via `tea_api_include_stderr`. `setup_mock_tea` takes the status
explicitly, so each test states the HTTP response it stands for instead
of leaving it implied — which also collapses four hand-rolled inline
mocks into it.
## Cost
`--include` writes the whole header block, not just the status line, so
captured stderr now holds whatever the server returns — a `Set-Cookie`
among it, on a Gitea that issues one. That reaches disk only under
`-vv`, which logs every subprocess's output to `subprocess.log`. There's
no narrower flag, and the status isn't available any other way; it's
noted in the module docs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> _This was written by Claude Code on behalf of Maximilian_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
10e8bad2c5 |
fix(gitea): key the tea api error shape on the envelope, not on a non-empty message (#3600)
Gitea blanks a 500's message in production unless the token belongs to
an admin (`services/context/api.go`: `if setting.IsProd && !(ctx.Doer !=
nil && ctx.Doer.IsAdmin) { message = "" }`), so the body arrives as
`{"message":"","url":"…/api/swagger"}`. Both Gitea call sites required a
*non-empty* message before they would call a response an error, so that
body fell through to the data path:
- `wt list` logged `Failed to parse tea api pulls JSON`, once per branch
— blaming an API change for an API error. The CI cell was blank either
way, and the stderr layer is off at the default verbosity, so this
reached whoever was already debugging with `-v` or `RUST_LOG`.
- `wt switch pr:<n>` failed with `Failed to parse Gitea API response for
PR #N. This may indicate a Gitea API change.` — the same misattribution,
in the user's face.
## The key
The discriminator is now the presence of the `message` key, which is the
whole shape. `tea api` never reads the HTTP status — `runApi` copies the
body to stdout and returns nil regardless — so the body is the only
channel, and none of the resources read here carries a `message`:
verified against gitea `main` for `PullRequest`, `CombinedStatus`, and
`CommitStatus`.
`url` is deliberately *not* part of the key. The two mistakes aren't
symmetric: an error body that omits `url` would read as the resource,
which is the bug this key exists to prevent, while requiring it would
only guard against a resource one day growing a `message` field. Gitea
already ships error shapes without one (`APIInvalidTopicsError` is
`message` plus `invalidTopics`), and `CombinedStatus` *does* carry
`url`.
## Shape
One parser — `remote_ref::gitea::api_error_message` — now serves both
sites; the CI-status backend drops its duplicate struct, and the PR path
checks the envelope before the resource parse, so "may indicate a Gitea
API change" is reserved for a body that is neither. A blanked message
reaches the user as an error that says why there's no detail:
```
✗ Gitea API error for PR #101 on owner/test-repo, but the response carried no message — Gitea hides 500 messages from non-admin tokens
```
The CI cell for a blanked 500 stays blank, unchanged:
`is_retriable_error` is the one gate every backend uses to turn a
failure into the error indicator, and an empty message isn't retriable.
Only the misleading log line is gone.
## Tests
Extended rather than duplicated:
`test_tea_api_error_reads_the_response_shape` flips its blank-message
assertion (that assertion *was* the bug), a new
`test_api_error_message_reads_the_response_shape` pins the key against
every shape both sides see, and `switch_pr_gitea_blank_error_message`
snapshots the user-visible path alongside the existing 401/403/404
cases.
`test_list_full_gitea_parse_warning_is_reserved_for_unknown_bodies`
covers the PR-list side end to end. It drives `wt list --full` under
`RUST_LOG=warn` and reads stderr rather than snapshotting, since the
warning is a `tracing` record that the stderr layer suppresses at the
default verbosity (`-v` would turn it on but bury it under a template
expansion per worktree). A second case sends a body that is neither the
resource nor the envelope, where the warning does belong — that pins the
wording to unknown shapes and keeps the first case from passing merely
because nothing logs. Checked against the pre-fix key: the blank case
fails there with the misleading warning, the unknown case passes.
## Noted, not done
tea's `api` command has an `--include` flag that writes the status line
to stderr — a genuine structured channel that would beat shape-sniffing.
It's in tea's `main` source but in no released changelog entry, so
sending it would break every user whose `tea` predates it. It would also
compose with this change rather than replace it, since the body still
supplies the message text.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> _This was written by Claude Code on behalf of Maximilian_
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
6d09125b7b |
test: converge suite on semantic boundaries (#3663)
This follows the first test-simplification tranche by converging the remaining suite around distinct semantic and pragmatic contracts rather than raw case count. The branch removes false-confidence tests, invalid setup variants, repetitive snapshots, and expensive PTY overlap while strengthening the retained route, precondition, and interaction proofs. ## What changed - Replace obsolete CI-status integration mocks and blank snapshots with direct provider semantics, mixed-priority cases, and strict GitHub/GitLab route assertions. - Remove free-riding merge, push, remove, list, security, config, and switch cases whose setup never reached the named behavior; consolidate repetitive direct cases into labeled tables. - Reduce the switch picker from 42 PTYs to 19 distinct terminal contracts, using causal release gates for asynchronous loading and repaint behavior. - Add a cached main-only picker fixture, eliminating 138 unnecessary Git subprocesses across the retained PTYs, and integrate it with main's generated hermetic standard fixture. - Tighten test guidance around proving setup preconditions and mock invocation routes, and correct the comments-tab help text and generated mirrors. ## Reviewer map - `tests/integration_tests/ci_status.rs` and `src/commands/list/ci_status/`: provider semantics and route coverage. - `tests/integration_tests/switch_picker.rs`, `src/commands/picker/`, and `src/testing/`: retained PTY contracts, causal mocks, and fixture design. - `tests/integration_tests/config_show.rs`, `src/config/deprecation.rs`, `src/config/expansion.rs`, and worktree type/resolve tests: direct-boundary consolidation. - `tests/CLAUDE.md`: the testing rules extracted from the false-confidence cases found during the survey. The measured loop removed 98 tests, 65 snapshots, and 23 picker PTYs. Controlled warm Nextest execution improved from a 79.593-second mean to 72.574 seconds (8.8%), while comparable production-line coverage moved from 97.32% to 97.23%. The tracked PR diff is a net deletion of more than 5,700 lines. ## Validation - `cargo run -- hook pre-merge --yes` after syncing current `main`: 4,468 passed, one configured skip; docs, doctests, clippy, formatting, policy checks, and snapshots green. - `task coverage` on the completed change before the base sync: 4,465 passed, one configured skip; 97.23% comparable production-line coverage. - Three independent final audits found no remaining lost beliefs, fixture hazards, or safe PTY consolidations. > _This was written by Claude Code on behalf of max_. |
||
|
Maximilian Roos
|
a285ed308d |
refactor(tests): bring the mock-stub env vars under the WORKTRUNK_TEST_ prefix (#3621)
`MOCK_CONFIG_DIR` and `MOCK_CALL_LOG_DIR` were the only two worktrunk-invented environment variables without the `WORKTRUNK_` prefix. That is not just a naming inconsistency: `isolate_subprocess_env` scrubs the parent environment by prefix — `GIT_*` and `WORKTRUNK_*` — so an unprefixed name is the one thing a test child inherits from whoever ran the suite. Renaming them brings them under that scrub. - `MOCK_CONFIG_DIR` → `WORKTRUNK_TEST_MOCK_CONFIG_DIR` - `MOCK_CALL_LOG_DIR` → `WORKTRUNK_TEST_MOCK_CALL_LOG_DIR` `TEST` rather than a bare `WORKTRUNK_` because both are read only by `tests/helpers/mock-stub` — they are the protocol between the harness and its helper binary, and `wt` itself never reads either one. That matches the ~15 existing `WORKTRUNK_TEST_*` knobs. ## The snapshot half Mechanical but not a substitution: the `env:` block is byte-sorted by key, so the renamed entry moves position within it and a `sed` in place would leave it where the old name sorted. All 971 affected blocks were rewritten by dropping the old line, inserting the new one, and re-sorting — the aggregate diff is exactly one removed and one added line per file: ``` 971 files changed, 971 insertions(+), 971 deletions(-) - MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + WORKTRUNK_TEST_MOCK_CONFIG_DIR: "[TEST_MOCK_CONFIG]" ``` The redaction placeholder follows its neighbours' convention in `add_standard_env_redactions` (`WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` → `[TEST_NU_VENDOR_AUTOLOAD]`), so it now reads `[TEST_MOCK_CONFIG]` rather than repeating the full key. `MOCK_CALL_LOG_DIR` appears in no snapshot — its two call sites are `.output()` assertion tests — so it needs no redaction. ## Keeping it from regressing `tests/CLAUDE.md` gains the rule under "Where a new environment variable goes": name it `WORKTRUNK_TEST_*`, and the rule covers the harness↔helper-binary protocol, not just knobs `wt` itself reads. Without it the next helper-binary variable gets named `MOCK_*` again and the hermeticity hole reopens. ## Verification `cargo run -- hook pre-merge --yes` passes (exit 0) on the merged tree: 4607 tests including `--features shell-integration-tests`, `pre-commit run --all-files`, clippy, doctests. No pending snapshots. A repo-wide sweep finds no remaining unprefixed spelling. ## Merged main #3620 landed while this was in flight and regenerated several `for_each` snapshots that still carried the old key, so main is merged in here. It resolved with no conflicts, and the result is what you'd want rather than what git happened to produce: those blocks now carry #3620's new keys (`GIT_ALLOW_PROTOCOL`, `CLAUDE_CONFIG_DIR`, `WORKTRUNK_TEST_PARENT_SHELL`) *and* the renamed key, each in sorted position. The sweep and the gate above both ran after the merge. #3620 also rewrote the "Where a new environment variable goes" section this branch adds to — three layers became four. Both edits survived; the new naming paragraph follows the updated layer list. The two advisory `affected tests` checks are red for the same reason, and merging clears them: `cargo affected` errors on `git diff stdout was not valid UTF-8`, because the diff from this PR's base contains 16 binary files — the `tests/fixtures/standard/` git objects and index files that #3620 deleted. This branch's own commit contributes none. A PR based after #3620 won't see them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
58444440d7 |
fix(ci-status): classify tea failures by shape; report azure-devops extension (#3597)
Two follow-ups from #3595, both in code that talks to a forge CLI. ## Gitea CI status had the same exit-0 blind spot as the switch path `tea api` never reads the HTTP status — it copies the response body to stdout and exits 0 — so the `!output.status.success()` gate in `src/commands/list/ci_status/gitea.rs` never fired for a 4xx/5xx. #3595 fixed this for `wt switch pr:<n>`; the CI-status backend is the second instance of the same bug. The commit-status path was the quieter one: every field of `GiteaCombinedStatus` carries `#[serde(default)]`, so Gitea's `APIError` body deserialized as `{state: "", total_count: 0}` — an error indistinguishable from "this commit has no statuses". The PR-list path was noisier but less harmful: the error body failed to parse and logged `Failed to parse tea api pulls JSON`, blaming an API change for what was an API error. `tea_api_error` now reads the response shape — the `APIError` body, whose message feeds the existing retriable sniff — and a non-zero exit keeps meaning what it always meant, `tea` itself failing (transport error, no login). A proxy's HTML error page stays unclassified, as before: sniffing arbitrary bodies is the string-heuristic pile this removes. ## `wt config show --full` reports the azure-devops extension The whole `az repos` command group ships in that extension, so without it CI status stays blank however well `az` is authenticated — a persistent, user-fixable condition, which is what this surface is for. `azure_devops_extension_installed` (added in #3595) answers it in JSON: ```console ✓ az installed & authenticated ▲ azure-devops extension not installed; run az extension add --name azure-devops ``` An `az` that can't answer counts as installed, so an unanswered check never becomes an accusation. The `wt switch pr:<n>` bail now uses the same sentence — one wording for one condition, which is the one-line change to `switch_pr_azure_extension_not_installed.snap`. ## Testing The two new Gitea tests were checked against the unfixed source: reverting `gitea.rs` alone fails both, so they pin the fix rather than describe it. The two pre-existing retriable tests kept their exit-1 shape but lost their fabricated payload — `exit 1` alongside `429 Too Many Requests` is the combination `tea` can't produce — and now carry a real transport error, which is the case that does exit non-zero. `config show --full` gets a case per extension state. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jeremy Odell
|
0681a2e0d0 | feat(list): add structured repo metadata to JSON output (#3021) | ||
|
Maximilian Roos
|
bd84d7479f |
Release v0.59.0 (#3100)
Release v0.59.0 (minor). The bump is minor because two backward-incompatible behavior changes ship: in the `wt switch` picker, `1`–`5` no longer switch preview tabs (digits now filter; tabs move to `Alt-1`–`Alt-5` / `Tab`); and `wt step eval --dry-run` is removed in favor of `-v`. `cargo semver-checks` reports no library API breakage. ### Highlights - **Improved**: picker frees digit keys for filtering (preview tabs to `Alt`); squash templates use `commit_details` instead of the deprecated `commits`; `wt step eval -v` lists template variables. - **Fixed**: picker no longer freezes on the first keystroke with many worktrees (#3087, thanks @bendrucker; fixes #2926, thanks @mahume for reporting); tab-completion covers all hook types; Ctrl-C on a concurrent alias reports the right exit code. See `CHANGELOG.md` for the full set. ### Bundled test fix This PR also carries one non-release commit: `test(ci_status): cover '?' worker-thread label in snapshot filter`. The `test_list_full_with_invalid_configured_platform` snapshot filter only normalized `[a-zA-Z]` worker-thread labels, but `label_for_thread_index` emits `?` for thread IDs above 52. On high-core machines the rayon pools span enough threads to hit that, so the gate failed locally (CI's low core count never triggers it). The filter now covers the full label alphabet. Test-only; not in the changelog. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
c511d3fa2b |
feat(list): show PR/MR number in the CI column (#3041)
The CI column in `wt list --full` (and the statusline) previously showed a single colored dot. It now shows the branch's open PR/MR reference — `#3035` on GitHub/Gitea/Azure DevOps, `!3035` on GitLab — colored by CI status, dimmed when stale, and hyperlinked to the PR. When no number is available (branch workflows without a PR/MR, pre-number cache entries, or a number wider than the allocated column), the cell shows a bare `#` in the same colors. Fetch errors always render `⚠`, even when a number is known — Error and Conflicts share yellow, so a yellow `#3035` would read as a conflicted PR. The branch merges main's review-state feature (#3044): review colors (magenta/cyan) and draft dimming apply to the number cells exactly as they did to the dot, and the `--help` legend shows colored `#` samples for all seven states (the interim version had dropped the colored samples from the legend entirely). ## The width problem `wt list` renders skeleton-first: column widths are fixed before any CI data arrives, and the table never resizes mid-render. The PR number's width therefore has to be known up front. The solution is a repo-level ratchet cache (`.git/wt/cache/pr-number/max.json`) holding the largest PR number any fetch has seen — PR numbers are monotonic per repo, so the value needs no invalidation. Pre-skeleton, `collect` reads that one file and sizes the column exactly; on a cold cache the estimate is 5 chars (`#9999`). A number that outgrows the estimate renders as the bare `#` for that run and sizes correctly on the next run once the ratchet records it. The ratchet is deliberately separate from the per-branch `ci-status/` entries so the width hint isn't coupled to branch-entry retention, and `detect` re-ratchets on cache hits too, so a deleted or racily regressed `max.json` heals from locally cached numbers instead of waiting out the TTL. ## Reviewer's map - `src/commands/list/ci_status/mod.rs` — `PrRef` (number + forge sigil, `PrRef::pr`/`PrRef::mr` constructors), `PrStatus.number` (serde-default so pre-existing cache entries still deserialize, rendering `#` until their 30–60s TTL expires), `format_cell` width-aware renderer with the Error guard, ratchet in `detect` (both cache-hit and fetch paths) - `src/commands/list/ci_status/cache.rs` — `MaxPrNumber` ratchet (read/ratchet/clear) - `src/commands/list/ci_status/{github,gitlab,gitea,azure}.rs` — each fetcher populates the number (`gh --json number`, `iid`, Gitea `number`, `pullRequestId`); GitLab's mr-view-failure path carries the iid/URL/review state into the error status so the `⚠` stays clickable - `src/commands/list/layout.rs`, `collect/mod.rs` — width estimate threading - `src/commands/list/render.rs`, `model/item.rs` — table cell and statusline both go through `format_cell` - `src/commands/list/json_output.rs` — `ci.number` field - `src/commands/config/state.rs` — ratchet shown by `state get`/`cache get` (table + JSON) and swept with the CI cache category, including the deprecated `ci-status clear --all` path - `src/md_help.rs`, `src/help.rs` — legend colorization rules rewritten from `●` to `#` (terminal + website) Most of the diff is snapshot churn from the column width and glyph changes plus regenerated docs mirrors. Known trade-offs: concurrent statusline ratchet writes can transiently lose an update (monotonic, re-learns on the next render, documented at the write site); one anomalously high PR number widens the column until `wt config state cache clear`; an open Azure DevOps PR still shows gray `NoCI` instead of its pipeline status — a pre-existing gap, now marked `TODO(azure-pr-pipeline)`. Testing: unit tests for `format_cell` (including the Error-with-number and oversized-number link cases)/`pr_ref_width`/ratchet/width estimates; integration coverage for all four forges with real numbers (the Gitea mocks now exercise the number path too), review-state × number composition, the GitLab mr-view-failure `⚠` and branch-pipeline success paths, cache-TTL expiry → refetch, the statusline number view, and the `wt config state` surfaces. > _This was written by Claude Code on behalf of max_ --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
214d82aeda |
Add PR/MR review state to wt list CI status (#3044)
Adds PR/MR review state to `wt list`'s CI status, merged into the existing CI dot color. ## What - New `ReviewState` (`approved` / `changes_requested` / `pending` / `draft`) on `PrStatus`, absent when the forge reports no review signal — branches with no review activity render exactly as before. - **GitHub**: `reviewDecision,isDraft` join the existing single `gh pr list --json` call (no extra API cost). Draft wins over the decision; an empty `reviewDecision` (no required reviewers, no reviews) maps to absent rather than `pending`, so solo repos don't show a perpetual waiting state. - **GitLab**: `draft` + `detailed_merge_status == "not_approved"` → `draft`/`pending`; MR list data carries no approved/changes-requested signal. Gitea/Azure: none. - **Display**: review state merges into the CI dot in `PrStatus::color()` — conflicts (yellow) > changes-requested (**magenta**) > running (blue) > failed (red) > review-required (**cyan**) > base CI color; draft dims like staleness. Cool colors mean waiting (blue: on CI, cyan: on a human), warm mean act; changes-requested outranks running because waiting can't clear it. Magenta was the one color not already carrying a meaning in the list row; cyan is reused from the working-tree symbols column, where the differing glyph disambiguates. - **JSON**: `ci.review_state` in `wt list --format=json`, vocabulary matching Claude Code's statusline `pr.review_state` so the two surfaces agree on names. ## Notes for review - The merge lives in `PrStatus::color()`/`style()` — `CiStatus` stays pure CI (cache values and JSON `status` strings unchanged). Both the table and the statusline render through `format_indicator` → `style()`, so there's a single chokepoint. - Old CI cache files deserialize unchanged (the new field is an `Option`). - `docs/content/list.md` and `skills/worktrunk/reference/list.md` are regenerated from `src/cli/mod.rs`. The HTML and terminal help colorizers learned magenta/cyan (they previously mapped only the five existing colors). Also fixed a pre-existing missing `repo_url` row in the ci-object doc table. - Testing: unit tables for the color merge and both forge mappings; integration snapshots (mocked `gh`) for changes-requested / review-required / approved / draft verifying the exact ANSI codes; the JSON field name is pinned by an inline snapshot. If this change is bad, it's most likely because the dot now encodes two axes (CI health and review attention) in one color — a consumer who reads green as "CI passed" still gets that, but red-vs-magenta now distinguishes who rejected the branch, which is more vocabulary to learn. The `--format=json` output keeps the axes separate for anyone who wants their own logic. Ref #2950 > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Maximilian Roos
|
93812660c3 |
refactor(log): migrate from env_logger to tracing-subscriber
Replaced env_logger with a layered tracing-subscriber setup that routes log records structurally by target and verbosity level. Three layers cooperate: - stderr: off at `-vv`; otherwise `$RUST_LOG` or `-v` baseline with styled output (bold commands, red errors, thread labels) - trace.log: `-vv` only, excludes `SUBPROCESS_FULL_TARGET`, plain text format - output.log: `-vv` only, `SUBPROCESS_FULL_TARGET` only, raw passthrough The `log` crate calls throughout the codebase are bridged into `tracing` via `tracing_log::LogTracer`, so the migration is transparent to existing call sites (~100 remain using `log::debug!` etc.). Routing is now per-layer filtering rather than a post-format decision, and `RUST_LOG` merges with the verbose flag via `EnvFilter::builder().with_default_directive(...).from_env_lossy()` (env wins when set). File sinks are opened before subscriber init so their internal `Repository::current()` calls don't emit into a half-built pipeline. |
||
|
Maximilian Roos
|
6732433056 |
fix(log): honor RUST_LOG at -v and -vv, not just -v 0 (#2901)
## Summary
`wt`'s `-v 0` (no flag) honors `RUST_LOG` — `RUST_LOG=debug wt list`
shows debug records. `-v` and `-vv` silently ignored it: they hardcoded
`filter_level(Info)` / `filter_level(Debug)` and dropped any `RUST_LOG`
directives on the floor.
Now all three verbosity levels flow through the same builder shape: the
flag sets a baseline (`Off` / `Info` / `Debug`), and
`env_logger::Builder::parse_default_env()` layers `RUST_LOG` on top.
When `RUST_LOG` is set it wins (raising or lowering the level, and
supporting per-target directives like `worktrunk=trace`). When it's
unset the flag baseline stands.
Examples:
- bare `wt` → Off (silent, unchanged)
- `RUST_LOG=debug wt` → Debug (unchanged)
- `wt -v` → Info (unchanged)
- `RUST_LOG=trace wt -vv` → Trace globally (was silently capped at
Debug)
- `RUST_LOG=worktrunk=trace wt -v` → Info + Trace for `worktrunk` (was
silently ignored)
The flag still owns the file-sink side effects (`-vv+` opens `trace.log`
/ `output.log`); only the level filter is now `RUST_LOG`-aware.
## Design choice
Three patterns are common in Rust CLIs:
1. **env wins when set, flag is the default** (`env_logger` convention;
e.g. cargo via `CARGO_LOG`)
2. **flag wins when both are set** (e.g. ripgrep, which doesn't honor
`RUST_LOG` at all and uses its own `--debug` / `--trace`)
3. **max(flag, env)** with per-target additions
I picked (1). The task framing makes the case directly — `RUST_LOG`
users are debugging and know exactly what they want; the flag-only user
wants a level; the two shouldn't fight silently. (1) is also the
documented `env_logger` pattern (`builder.filter_level(...);
builder.parse_default_env();`) and gives the smallest implementation.
(3) was tempting for "the verbose user always wins", but it's not
natively supported by `env_logger` — extracting just the env's
global-wildcard level (not its per-target ceilings) has no clean API,
and any hand-rolled merge complicates an otherwise five-line function
for the small win of "`RUST_LOG=warn` doesn't downgrade `wt -vv`".
## Adjacent fix
`src/testing/mod.rs::configure_cli_command` was setting `RUST_LOG=warn`
as a blanket default for every CLI test. That was harmless under the old
behavior (the flag overrode it at `-vv`); under the new behavior it
would silently cap all `-vv` tests at Warn, starving `trace.log` of the
debug-level `[wt-trace]` records the diagnostic tests assert on. Dropped
the default; the one test that genuinely needed the warn-level
diagnostic (`test_list_full_with_invalid_configured_platform`, asserting
on the `log::warn!` for invalid CI platform) opts in via
`cmd.env("RUST_LOG", "warn")` per-invocation.
## Test plan
- New `test_rust_log_overrides_verbose_flag` in
`tests/integration_tests/diagnostic.rs` — asserts `-v` alone suppresses
`[wt-trace]` records (Info baseline) and `RUST_LOG=debug wt -v` surfaces
them (env raises).
- Existing `test_rust_log_debug_fallback_without_vv` still covers
`RUST_LOG=debug` at `-v 0`.
- All 3835 tests in `cargo run -- hook pre-merge --yes` pass.
|
||
|
Maximilian Roos
|
c0d9482335 |
refactor(ci-status): align Gitea PR detection with the GitHub backend (#2732)
Mirrors the GitHub backend's PR-resolution shape in the Gitea backend: query the primary remote's open PRs and filter by the branch's push owner against `head.repo.owner.login`. PRs opened from a fork (head owner ≠ upstream owner) now match — previously the Gitea path queried the fork's own (usually empty) `/pulls` and never found them. Adds an explicit `&limit` matching `MAX_PRS_TO_FETCH`, so the Gitea and GitHub backends fetch the same number of candidate PRs instead of relying on Gitea's default page size. With the fork-PR and page-size limitations resolved in code, the module docstring drops from ~36 lines (API spec, "two paths", "known limitations") to a short header matching `github.rs` / `azure.rs`. The remaining per-quirk note (`mergeable` is computed asynchronously) moves inline next to the conflict check. The `--remotes` test that previously asserted the PR path used the branch's own remote now documents that the PR path intentionally uses the primary remote; it still verifies the SHA-status fallback honors `branch.remote` (it passes via that fallback). A Gitea analogue of github's `test_list_full_filters_by_repo_owner` (`test_list_full_with_gitea_fork_pr`) locks in the fork-PR match: two Gitea remotes, the `feature` branch pushes to the fork, and the mock includes a decoy PR from a different head owner. The green CI indicator fails loudly if a future change reverts to querying/filtering by the branch's own remote. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Maximilian Roos
|
8425f29044 |
fix(ci-status): resolve Gitea owner/repo from the branch's own remote (#2707)
Brings the Gitea CI-status path to parity with the GitHub path when resolving owner/repo for a branch. Today `gitea_owner_repo` always reads the primary remote, so a `wt list --remotes --full` row for a remote-only branch on a non-primary remote (e.g. `gitea/feature` when `origin` points elsewhere) silently misses CI. The new `gitea_owner_repo_for_branch` reads the branch's own remote via `effective_remote_url` for remote branches and falls back to the primary remote's raw URL for local branches. The local-branch fallback keeps using `remote_url` (raw) rather than `effective_remote_url` to match `git::remote_ref::gitea`'s existing `pr:` resolver — `tea` resolves the host from its own login config, and only owner/repo are extracted from the URL. The remote-branch arm uses `effective_remote_url` for symmetry with `github_owner_repo_for_branch`, so `url.insteadOf` aliases still resolve. Test: a new `--remotes --full` integration test stands up two Gitea remotes (`origin` → `owner/test-repo`, `fork` → `forkowner/test-repo`), mirrors a `fork/feature-remote` ref, and mocks `tea` to answer only `forkowner/test-repo` paths. The green `●` in the snapshot confirms the lookup hits the branch's remote — a primary-remote lookup would silently return no CI. Out of scope: the `wt switch pr:` Gitea resolver in `src/git/remote_ref/gitea.rs` keeps its primary-remote-only behavior. Fork PRs (head owner ≠ queried owner) remain unmatched — the limitation docstring is updated to reflect the actual residual gap. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
bfb117a665 |
feat(ci-status): experimental Gitea CI-status detection (#2702)
`wt list --full` and `wt config show --full` now recognize Gitea repos,
mirroring the existing GitHub / GitLab / Azure DevOps support.
Previously Gitea was wired up only for the `wt switch pr:<N>` shortcut.
## What this does
`wt list --full` shows a CI indicator for Gitea repos:
- **PR path**: lists open PRs (`tea api
repos/{o}/{r}/pulls?state=open`), finds the one whose head branch
matches, surfaces conflicts from `mergeable` plus the real CI state from
the PR head commit's combined status, and links the indicator to the PR
(`html_url`).
- **Branch fallback** when no PR exists: queries the combined commit
status of the local HEAD (`tea api repos/{o}/{r}/commits/{sha}/status`).
Both Gitea Actions and external CI report into commit statuses, so this
is the pass/fail/pending rollup — the equivalent of GitHub's check-runs
API.
`wt config show --full` adds a `tea`/Gitea diagnostics row when the
repo's a Gitea remote; `render_ci_tool_status` now derives the
auth-setup command per CLI (\`az login\`, \`tea login add\`, else
\`<tool> auth login\`) — which also fixes a pre-existing wrong hint for
\`az\`.
\`forge.platform = \"gitea\"\` now opts a repo into Gitea CI detection
(in addition to its existing role for the \`pr:\` shortcut). The
previous \"valid for \`pr:\`, but no CI\" special-case in
\`configured_ci_platform\` is removed; \`CiPlatform::Gitea\` flows
through the same dispatch as the other forges.
## Design notes
Mirrors \`azure.rs\`'s two-path structure but improves on it: when a PR
exists, Azure's path only ever surfaces conflicts/queued (pass/fail is
unreachable once a PR is found). Gitea's path additionally queries the
PR head commit's combined status, so PRs with passing CI show green
instead of blank. Cost: two \`tea api\` calls per PR'd branch; results
cached 30–60s.
Auth detection reads \`tea\`'s config file (\`has_any_login\`) rather
than invoking \`tea\` — matches the existing \`is_authed_for\` to avoid
OAuth-refresh side effects.
## Known limitations (experimental)
- Only the first page of open PRs (Gitea's default page size,
newest-first) is inspected; in a repo with many open PRs, ours could
fall off the page.
- The PR lookup queries the primary remote's repo only; PRs from a fork
(head repo owner ≠ that repo's owner) aren't matched.
- \`mergeable\` is computed asynchronously by Gitea, so a freshly-opened
PR can briefly report a false conflict until the check completes
(self-corrects when the cache expires).
## Docs
The \`--help\` CI section (\"GitHub/GitLab pipeline status\") stays
generic, matching #1256's precedent of not enumerating Azure there
either. The \`forge.platform\` value lists, the \`config show --full\`
\"CI status requires …\" string, the \"Invalid CI platform … Expected
…\" warning, and the field docs on \`[forge]\`/\`[ci]\` all enumerate
Gitea.
## Verification
API claims verified live against \`gitea.com\` (v1.26-dev) and
\`codeberg.org\` (Forgejo, Gitea-1.22 base), plus the Gitea source and
\`tea\` v0.14.0 source — the combined-status endpoint, the PR JSON
shape, the \`tea api\` query-string passthrough, and the \`?state=open\`
requirement (Gitea's pulls list does NOT default to \`open\` — Swagger's
\`default\` is doc-only). New tests cover
passed/failed/running/conflicts/no-ci/retriable-error (×2
paths)/no-\`tea\`-installed/\`config show --full\` Gitea diagnostics —
10 new integration tests + unit tests for parsing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
00c4508671 |
refactor(ci): move CiPlatform to the lib crate, cache on RepoCache, drop the "override" framing (#2692)
Follows up on #2686 (the original "dedup the warning" fix), which parked the configured-platform state in a process-wide `OnceLock` because `CiPlatform` lived in the binary crate and `RepoCache` in the library crate. **Move `CiPlatform` to the lib crate.** It's now `worktrunk::git::CiPlatform` (a new `src/git/ci_platform.rs` module), with `Repository::ci_platform(remote_hint)` replacing the free `platform_for_repo` and a private `Repository::configured_ci_platform()` reading `forge.platform` / `ci.platform`. The configured value caches on a `OnceCell` field of `RepoCache` instead of a process global — same once-per-`wt list` warning dedup, but the repo-scoped data no longer lives in a process static. `src/commands/list/ci_status/platform.rs` is now pure forge dispatch (`gh` / `glab` / `az`) over a `CiPlatform`; the inherent `detect_*` methods became free functions so the enum could move without dragging the bin-crate backends along. **Drop the "override" framing.** `forge.platform` / `ci.platform` is now described everywhere as "the platform, falling back to URL detection when unset" rather than "overriding detection" — config docs (`## Forge platform`), the `ProjectForgeConfig` / `ProjectCiConfig` struct and accessor docs, `wt config state ci-status` help, and a few test names (`test_list_full_with_configured_platform_github`, `test_list_full_with_invalid_configured_platform`, `test_switch_pr_gitea_forge_platform`). Genuine "override" uses elsewhere (config-path/env-var overrides, `wt config state default-branch set`) are untouched. **Fix a pre-existing bug.** `forge.platform = "gitea"` is a valid value — the `wt switch pr:` shortcut uses it to pick `tea` — but worktrunk fetches CI status only from GitHub/GitLab, so `wt list` used to print `Invalid CI platform in config: 'gitea'. Expected 'github' or 'gitlab'.` for it. `configured_ci_platform()` now recognizes `gitea` as a known forge it just doesn't show CI for: no warning, blank CI column. Added `test_list_full_with_gitea_forge_platform` (asserts blank CI and empty stderr). Branch is merged with `main`, so the dispatch and detection handle the new `AzureDevOps` variant alongside GitHub and GitLab. `cargo run -- hook pre-merge --yes` passes. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
mikeyroush
|
1ca73ab52c |
feat: experimental Azure DevOps support (#1256)
## Summary
Adds **experimental Azure DevOps support** alongside the existing GitHub and GitLab integrations:
- `wt switch pr:<N>` resolves Azure DevOps PRs via the `az` CLI (auto-detected from `dev.azure.com` / `ssh.dev.azure.com` / `*.visualstudio.com` remotes, or pinned via `[forge] platform = "azure-devops"`).
- `wt list --full` surfaces Azure DevOps PR and pipeline CI status.
- `wt config show --full` reports `az` install/auth state when Azure DevOps is the detected platform.
GitHub still wins in mixed-remote setups; `forge.platform` is the override. Requires the `azure-devops` CLI extension (`az extension add --name azure-devops`).
## Context
Originally proposed by @mikeyroush; reimplemented against current `main` to pick up the `[forge]` config section, `url.insteadOf` fallback, the `handle_switch` consolidation, and the new Gitea provider that all landed after the original branch was opened. The dispatch path (`choose_pr_provider`) is shared with GitHub/Gitea — Azure is just a fourth provider in the same priority chain.
Implementation notes worth a reviewer's attention:
- Azure DevOps URLs don't fit the standard `host/owner/repo` shape (`dev.azure.com/{org}/{project}/_git/{repo}`), so `Repository::find_remote_for_azure` matches on `org` + `project` + `repo` instead of the owner-based path used for the other forges.
- Pipeline/PR web URLs are constructed from `org/project/build-id`, not the API's `url` field (which is a REST endpoint).
- `*.visualstudio.com` legacy hosts encode the org in the hostname; the URL helpers handle both shapes.
Fixes #1144
## Test plan
- `cargo run -- hook pre-merge --yes` — 3631 tests pass, clippy + fmt clean
- Unit tests cover the host-aware URL helpers, `find_remote_for_azure` (all URL shapes + the same-org/different-project collision case), and `choose_pr_provider` dispatch
- Integration tests bring Azure to parity with the other forges (see Coverage):
- 13 `test_switch_pr_azure_*` tests mirroring the Gitea suite — same-repo, fork, `*.visualstudio.com` host, create/base conflicts, not-found, az-not-installed, `forge.platform` override, invalid JSON, generic server error, auth error, missing `azure-devops` extension, undeterminable org/host
- 9 `test_list_full_with_azure_*` tests covering `detect_azure_pr` (conflicts, queued, stale, retriable error) and `detect_azure_pipeline` (passed/failed/running, stale, no runs, retriable error)
- Manual validation: `wt switch pr:<N>` and `wt list --full` against an Azure DevOps repo
## Coverage
The `az`-shelling code (`fetch_pr_info`, `detect_azure_pr`, `detect_azure_pipeline`) is now exercised by integration tests via new `setup_mock_az*` helpers (modeled on `setup_mock_gh` / `setup_mock_glab`) — covering the happy paths plus the not-found / auth / extension-missing / generic-error / retriable-error branches. The non-`az` parts (URL parsing, provider dispatch, remote matching) remain unit-tested.
|
||
|
Maximilian Roos
|
68869761da |
Fix fork CI queries, branch tracking remote check, and diverged integration target (#1812)
Three edge-case fixes found via Codex: **1. CI check-runs fallback queries the wrong repo for fork workflows** — `detect_github_commit_checks` was using `github_owner_repo()` (first GitHub remote in the repo) instead of the branch's actual push destination. For fork workflows where branches push to a different repository, this queried the upstream repo's API and found no check runs. Added `github_owner_repo_for_branch()` that resolves through `pushRemote` → `remote.pushDefault` → tracking remote, with fallback to repo-wide remote. **2. `branch_tracks_ref` only checked `.merge`, not `.remote`** — When `wt switch pr:N` found an existing branch tracking `refs/pull/N/head`, it considered it a match regardless of which remote was configured. A branch could track the right ref path but have the wrong remote, so push/pull would be wired to the wrong repository. Now also checks `branch.<name>.remote` when an expected remote is provided. **3. `effective_integration_target` missed remote merges when local main diverged** — When local `main` had extra commits (diverged from `origin/main`), the function returned `main` instead of `origin/main`, causing `wt remove` to miss branches that were squash-merged on the remote. Now prefers upstream in the diverged case, since remote merges should still count as integrated. All three have regression tests. > _This was written by Claude Code on behalf of maximilian_ --------- Co-authored-by: Maximilian Roos <maximilian@Maximilians-MacBook-Pro.local> Co-authored-by: Claude <noreply@anthropic.com> |
||
|
worktrunk-bot
|
1afa9c230b | refactor: rename get_* functions to bare nouns (#1586) | ||
|
Maximilian Roos
|
d358f2ffeb |
test: fill codecov/patch coverage gaps from recent PRs (#865)
* docs: add coverage guidance and simplify shell integration docs Add codecov/patch approval requirement to coverage guidance. Simplify skip-shell-integration-prompt documentation to reflect automatic behavior when declining first-run prompt. Add test coverage for FormattedMessage::into_inner(). * docs: simplify shell prompt and add coverage guidance Simplify skip-shell-integration-prompt documentation to clarify that declining the prompt sets the config automatically, and add guidance on codecov/patch approval requirements. Include tests for reload_projects_from error paths to improve coverage on config validation. * docs: add coverage guidance and simplify shell integration docs Add codecov/patch approval requirement to coverage section and document the zero-as-unlimited rule for concurrent command limits with tests. Simplify shell integration prompt documentation to reflect automatic behavior when declining installation. * Add coverage for codecov/patch and GitLab edge cases - Add guidance on codecov/patch approval requirement to CLAUDE.md - Simplify skip-shell-integration-prompt documentation across all config files - Add tests for GitLab MR detection without project ID (single, empty, multiple) |
||
|
Maximilian Roos
|
d51e04f408 |
fix(glab): full compatibility audit and fixes (#770)
* Refactor GitLab CI status to use two-step MR resolution
Split MR lookup into separate list and view calls to properly fetch
pipeline status. The `glab mr list` command doesn't return head_pipeline
fields, requiring a second `glab mr view <iid>` call to get complete
details.
- Create GitLabMrListEntry for basic MR info from list command
- Create GitLabMrInfo for full MR details from view command
- Add fetch_mr_details() helper for the second API call
- Update mock infrastructure to handle compound command matching
- Remove redundant --state=opened flag from mr list call
- Fix glab ci list to use --ref instead of BRANCH env var
* fix(glab): correct status mappings found via adversarial testing
Round 2 of adversarial testing found additional issues:
- Move "manual" from Failed to Running - a manual pipeline is waiting
for user action, not failed
- Remove ci_must_pass → Failed mapping - it's a policy constraint
("CI must pass to merge"), not a failure indicator
- Add is_retriable_error check to detect_gitlab_pipeline - rate limit
and network errors were being cached as "no CI"
- Return Error (not NoCI) when mr view fails after finding MR
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(mock): support iid-specific mr view matching
Extend mock-stub to support triple command matching (e.g., "mr view 1"
before "mr view"). This allows tests to verify that the code correctly
passes the iid argument to glab mr view.
- Add triple matching priority in mock-stub command resolution
- Update test helper to create separate mock responses per iid
- Reorder test data to prove filtering works (not just taking first)
Co-Authored-By: Claude <noreply@anthropic.com>
* test: add coverage for GitLab CI error paths
Add tests for two previously uncovered code paths in gitlab.rs:
- When `glab mr view` fails after finding an MR (returns error status)
- When `glab ci list` returns a rate limit error (returns error status)
Both tests verify that the CI column shows ⚠ (error indicator) rather
than silently treating these failures as "no CI".
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
c4f1c0730d |
fix: detect CI status when pushremote is a URL (#769)
`gh pr checkout` sets `branch.<name>.pushremote` to a URL directly
rather than a remote name. The `@{push}` syntax fails in this case
because it expects a remote name to resolve to a tracking ref.
Use `%(push:remotename)` via `git for-each-ref` instead, which returns
either a remote name or URL directly without failing. Then check if
the result is a URL (use directly) or remote name (look up its URL).
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
9d43844515 |
refactor: reduce reliance on primary remote for CI and fork URLs (#753)
* feat: reduce reliance on primary remote for CI status and fork URLs - CI platform detection now searches all remotes instead of just the primary remote, improving fork workflow support - MR fetch now errors with actionable hint instead of silently falling back to primary remote when target project remote is not found - PR/MR fork URL construction uses the matched base/target repo's remote URL to infer SSH vs HTTPS format preference New Repository methods: - all_remote_urls(): returns all (name, url) pairs - find_remote_url_where(): finds first URL matching a predicate - first_remote_url(): convenience for fallback URL format detection Co-Authored-By: Claude <noreply@anthropic.com> * feat: use gh/glab config for git protocol instead of inferring from remotes Previously, fork PR/MR URL construction inferred SSH vs HTTPS protocol by examining existing remote URLs, falling back to `first_remote_url()` when no specific remote was found. This was complex and could guess wrong. Now we query the CLI tools directly: - `gh config get git_protocol` for GitHub PRs - `glab config get git_protocol` for GitLab MRs This uses the user's explicit preference (configured in gh/glab) and is consistent with how these tools handle clones. Also removes the arbitrary "first PR" fallback in CI detection - if we can't determine the branch owner, we now return None instead of guessing. Co-Authored-By: Claude <noreply@anthropic.com> * Refactor git URL parsing with platform detection helpers Extract platform detection logic into `GitRemoteUrl` methods (`is_github()`, `is_gitlab()`), add support for `git://` protocol, and remove the `find_remote_url_where()` method in favor of explicit filtering. Make `GitRemoteUrl` public for use across modules. Simplify platform detection in CI status and branch logic by using the new structured URL parsing. * Simplify GitHub remote lookup in CI status detection * Refactor fork_remote_url to accept host parameter Extract host from PR info instead of deriving it from PR URL. This simplifies the function signature and moves URL parsing logic to where the PR info is fetched, reducing duplication across call sites. * refactor: validate host parsing from PR URL Replace unwrap_or with explicit validation to ensure host is not empty. Use with_context to provide error details when host parsing fails. * test: add coverage for GitRemoteUrl::host() method Co-Authored-By: Claude <noreply@anthropic.com> * test: add coverage for fork_remote_url SSH/HTTPS formats Extract format helpers and test them explicitly to ensure both SSH and HTTPS code paths are covered regardless of local gh config. Co-Authored-By: Claude <noreply@anthropic.com> * test: avoid short-circuit evaluation in URL protocol tests Use array.contains() instead of || to ensure both expected URL formats are evaluated, improving branch coverage. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
cd8ac3e060 |
perf(tests): add pre-built fixture to speed up Windows CI (#634)
* perf(tests): add pre-built fixture to speed up Windows CI Instead of creating repos from scratch for each test, copy a pre-built fixture with worktrees and remote already configured. This should significantly reduce test time on Windows where process spawning is slow. - Add tests/fixtures/standard/ with repo, 3 worktrees, and bare remote - Update TestRepo to copy fixture instead of running git init/commit - Add fixture cleanup to doc-generating tests for clean output - Remove git sample hooks and boilerplate from fixture Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): correctly clean fixture state for isolated tests - Change `remove_fixture_worktrees()` to take `&mut self` and clear the worktrees map so `add_worktree()` can recreate branches - Add remote removal to select and approval tests to prevent `origin/main` from appearing in snapshots - Update select snapshots for fixture-based commit hashes Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): use relative paths in fixture for portability The fixture gitdir files contained absolute paths to the local development machine, causing CI failures. Updated to use relative paths which work after the _git → .git rename during fixture copy. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): remove invalid remote from bare repo fixture The bare repository in the fixture had a remote configured with an absolute path to the local machine, which wouldn't exist on CI. Bare repositories don't need remotes configured. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): remove origin in approval tests for consistent project_id Tests manually create approvals using a computed project_id based on repo path. With the fixture's origin remote, worktrunk computes a different project_id from the remote URL. Removing origin makes both use the same fallback (directory path). Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): improve fixture copy robustness for CI - Add platform-specific copy: `cp -r` on Unix, `robocopy` on Windows - Add verification that essential paths exist after copy - Add verification that origin.git is a valid git repository after rename - Include stderr in error messages for better debugging Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): add .gitkeep to preserve empty git directories in fixture Git doesn't track empty directories, so objects/info, objects/pack, and refs/tags were missing on CI after checkout. These directories are required for a valid git repository. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): ensure LF line endings in fixture git files Add .gitattributes to force LF line endings for git internal files (packed-refs, HEAD, config, etc.) to prevent CRLF corruption on Windows. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): handle file copies separately from directories on Windows robocopy only works with directories, not files. Use Rust's std::fs::copy for individual files like .gitattributes, and robocopy only for directories. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): enable rerere in test git config for consistent output Git's rerere feature produces "Recorded preimage" messages during conflicts. Enable this in test config so output is consistent between local machines and CI. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): restore accidentally deleted hook_clear tests The tests test_hook_clear_no_approvals and test_hook_clear_with_approvals were accidentally removed during the fixture refactoring. This restores them with the necessary remote removal so project_identifier matches "repo" (directory name) instead of the fixture's remote URL. Also adds remote removal to test_hook_show_approval_status for consistency. Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove unreferenced ping_pong snapshots These snapshots used feature_a but the tests now use feature, leaving these orphaned. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
f0f3e18629 |
test: add GitLab MR status and shell config creation tests (#546)
Add coverage for: - GitLab MR detection via mock glab (passed, failed, running, pending, conflicts, stale, no CI, project ID filtering) - Shell config file creation when targeting a shell explicitly 288 lines of tests for 47 lines of coverage gain. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
2e2777c404 |
refactor: simplify Repository API and eliminate magic cwd lookups (#519)
* refactor: simplify Repository API and eliminate magic cwd lookups - Add WorktreeView abstraction accessed via repo.current_worktree() - Move worktree-specific methods (root, current_branch, is_dirty) to WorktreeView - Refactor CI status functions to take &Repository instead of implicitly calling Repository::current() - Remove repo_root string parameters from CI detection functions; derive working directory from &Repository internally - Add primary_remote_url() convenience method combining primary_remote() and remote_url() - Fix post-removal operations to use Repository::at(main_path) instead of Repository::current() which fails when cwd is deleted This is an intermediate step toward a cleaner Repository/Worktree separation. The WorktreeView will eventually be renamed to Worktree once the cache is moved onto Repository. Co-Authored-By: Claude <noreply@anthropic.com> * fix: update doctests to match Repository::current() signature Repository::current() now returns Repository directly, not Result<Repository>. Update doc examples to not use `?` on the return value. Co-Authored-By: Claude <noreply@anthropic.com> * test: add coverage for CI platform detection Add integration tests for: - Platform override in project config (`[ci] platform = "github"`) - GitLab remote URL detection These tests cover the changed lines in get_platform_for_repo() and exercise the GitLab remote detection path. Co-Authored-By: Claude <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * chore: remove unused WorktreeView::run_command_check This method was added but never used. All call sites use Repository::run_command_check instead. Co-Authored-By: Claude <noreply@anthropic.com> * Compute git-common-dir eagerly on Repository construction This change improves performance and simplifies the codebase by computing the git common directory once when Repository is created, rather than lazily on first access. The git-common-dir is now stored directly on the Repository struct instead of in a OnceCell, making it immediately available without runtime initialization overhead. Key changes: - Repository::current() and Repository::at() now return Result - git_common_dir() returns &Path instead of Result<PathBuf> - wt_logs_dir() returns PathBuf instead of Result<PathBuf> - All callers updated to handle the new error types - Cache lookups use the eagerly-computed path as key - Repository now implements Clone for easier passing to background tasks * chore: remove unused WorktreeView::path() and ::repo() These accessors were added but never used. Removing dead code. Co-Authored-By: Claude <noreply@anthropic.com> * refactor: remove CI tools hint code from wt list Remove the hint that suggested installing/authenticating gh/glab when CI status couldn't be fetched. This simplifies the codebase: - Remove get_gitlab_host_for_repo function - Remove CiToolsStatus::any_available method - Remove hint display logic from list command - Remove parse_remote_host import The hint was nice-to-have but not essential. Users can still see CI status when tools are available; the hint just won't appear when they're not. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Maximilian Roos
|
ecb5fe3d97 |
test: parameterize CI status tests (#516)
Consolidate 12 CI status tests into 7 parameterized test functions using rstest #[case]. Same coverage with 70% less code: - PR status tests (4 cases): passed, failed, running, conflicts - StatusContext tests (2 cases): pending, failure - Workflow run tests (2 cases): completed, running - Special cases (4 tests): stale PR, mixed types, no checks, owner filter Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
928b3017cd |
test: remove verbose assertion messages and doc comments (#458)
* Simplify assert messages by removing redundant descriptions
Remove verbose assertion messages that repeat the condition being tested.
Messages like "Should contain indicator" when asserting `contains("●")` are
redundant since the code is self-documenting. This reduces noise in tests
while maintaining clarity through meaningful assertion failures.
* test: remove verbose assertion messages and doc comments
Removed facile test verbosity across three categories:
1. **Doc comments** (~236 lines): Removed `/// Test X` comments that just
restated test function names. Kept multi-line explanations that add context.
2. **Test names** (9 functions): Shortened overly verbose names by removing
filler words while maintaining clarity:
- test_merge_when_primary_not_on_default_but_default_has_worktree →
test_merge_primary_not_on_default_with_default_worktree
- test_complete_switch_option_prefix_shows_options_not_branches →
test_complete_switch_option_prefix_shows_options
3. **Assertion messages** (19 instances): Removed messages that just restate
the check:
- assert!(path.exists(), "Path should exist") → assert!(path.exists())
- assert!(x.is_file(), "Should be a file") → assert!(x.is_file())
Kept messages that add semantic meaning (e.g., "hook should have run") or
include debugging variables.
Total: 279 lines removed across 31 files. All 723 integration + 401 unit tests
passing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* test: remove unreferenced snapshot files from renamed tests
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
|
||
|
Maximilian Roos
|
a7bb80365e |
Enable Windows CI status tests via MOCK_SCRIPT_DIR (#407)
* Enable Windows CI status tests via MOCK_SCRIPT_DIR mock-stub.exe now passes MOCK_SCRIPT_DIR environment variable to bash, allowing scripts to reliably find sibling files (like JSON test data) without relying on $0/dirname/pwd which fail in Git Bash on Windows CI. - Add script_dir computation and MOCK_SCRIPT_DIR env var to mock-stub.exe - Update mock gh script to use MOCK_SCRIPT_DIR when available - Remove all 12 Windows ignore attributes from ci_status.rs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(mock-stub): use Git Bash explicitly on Windows, not WSL bash On Windows, there are multiple 'bash' executables in PATH: - C:\Windows\System32\bash.exe - WSL wrapper (fails if WSL not installed) - C:\Program Files\Git\bin\bash.exe - Git Bash (what we need) The WSL wrapper was being found first and failing with 'Windows Subsystem for Linux has no installed distributions' because WSL isn't installed on GitHub Actions Windows runners. Fix by adding find_bash() that explicitly looks for Git Bash at known installation paths before falling back to the WSL wrapper. Co-Authored-By: Claude <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude <noreply@anthropic.com> * fix(mock-stub): collapse nested if statements for clippy Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove debug logging from Windows CI fix Remove the debug file-based logging that was added during troubleshooting. Keep the MOCK_DEBUG conditional stderr logging for future debugging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor(mock-stub): simplify find_bash to single path check GitHub Actions Windows runners have Git at C:\Program Files\Git. No need for multiple fallback paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
e8f1037719 |
Enable Windows test support with cross-platform mock infrastructure (#401)
* test: add cross-platform mock command helpers for Windows tests Add mock_commands.rs module with platform-abstracted mock creation: - create_mock_command: branching mock based on first argument - create_simple_mock: fixed output mock (e.g., for llm) - create_mock_command_2arg: two-argument matching (e.g., uv run pytest) - High-level helpers: create_mock_cargo, create_mock_llm_*, etc. On Unix: creates shell scripts with #!/bin/sh shebang On Windows: creates .cmd/.bat batch files with goto-based control flow Migrate 3 merge.rs tests from inline shell scripts to cross-platform mocks: - test_readme_example_complex - test_readme_example_hooks_post_create - test_readme_example_hooks_pre_merge Enable ci_status.rs tests on Windows (PATHEXT handling already works). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * simplify: target Git Bash for Windows test mocks Change from maintaining parallel shell/batch implementations to a unified shell script approach. Since hooks already require Git Bash on Windows (documented in FAQ), tests can assume Git Bash is available. The new approach: - Write all mock logic as shell scripts (#!/bin/sh) - On Windows, create thin .bat/.cmd shims that invoke `bash script.sh` - Single source of truth for mock behavior This reduces mock_commands.rs from 558 to 380 lines (-178 lines) while simplifying maintenance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove unused MockBranch2, clarify Git Bash requirement Code review fixes: - Delete unused MockBranch2 and create_mock_command_2arg (YAGNI) - Clarify Git Bash requirement in module documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: write mock scripts without extension for Git Bash on Windows The hooks run through Git Bash, which doesn't recognize .bat/.cmd files as executables. Git Bash needs shell scripts without extensions that it can execute directly via shebang. Now on Windows we create: - `name` (no extension) - shell script for Git Bash - `name.cmd` and `name.bat` - shims for CMD/PowerShell 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: use .cmd extension for LLM command path on Windows When worktrunk runs the LLM command directly (not via PATH lookup), Windows requires a .cmd or .bat extension for direct execution. Use llm.cmd on Windows, llm (no extension) on Unix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: add .gitattributes for consistent line endings across platforms Force LF line endings for all text files to ensure consistent commit hashes across platforms. Without this, Windows may use CRLF which changes file content hashes and thus commit hashes, causing snapshot test failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: skip CI status tests on Windows Rust's Command::new("gh") uses CreateProcessW which only searches for .exe files, not .bat/.cmd files. Our mock infrastructure creates batch files that work for Git Bash but aren't found by CreateProcessW. The CI status feature works identically on Windows — it just calls gh/glab binaries. The core functionality is verified by Unix tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: set filemode=false in test template for cross-platform consistency Git on Windows handles file modes differently than Unix. With filemode=true, Git tries to track executable bits, but Windows reports them differently, causing tree hash differences and thus different commit hashes. Setting filemode=false ensures Git records all files as 100644 (non-executable) on both platforms, producing identical commit hashes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Revert "fix: set filemode=false in test template for cross-platform consistency" This reverts commit |
||
|
Maximilian Roos
|
d13eb0c1d2 |
refactor(tests): simplify test helpers and remove unnecessary worktrees (#326)
- Add `check_git_status()` helper to verify git command success - Add thin wrappers `run_git`, `run_git_in`, `git_output` using check_git_status - Simplify `repo_with_main_worktree` fixture to no-op (primary is already on main) - Remove unnecessary `add_main_worktree()` calls from merge tests and fixtures - Fix latent test bugs where git commands were silently failing - Use `set_temp_home_env` helper in post_start_commands tests - Fix test using non-existent "develop" branch - Fix test needing `repo_with_remote` fixture Net reduction: ~208 lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
bceedcade7 |
Fix test isolation for wt commands and improve test API (#314)
Tests that spawn `wt` must be isolated from the host environment to prevent directive leakage (test commands writing to user's shell directive file), config pollution, and git interference. Changes: - Add `TestRepo::wt_command()` returning pre-configured Command - Rename `clean_cli_env` → `configure_wt_cmd` (parallel to git) - Make `git_command()` consistent (no args, chain `.args()`) - Fix approval_ui.rs and readme_sync.rs missing isolation - Add `#[must_use]` to command builders - Document pattern in tests/CLAUDE.md The public API is now 3 symmetric methods: - `repo.wt_command()` - for wt commands with TestRepo - `repo.git_command()` - for git commands - `wt_command()` - for wt without TestRepo (help tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Maximilian Roos
|
71b35e76d7 |
Enable coverage for PTY tests and add CI status tests (#222)
* Add integration tests for config and state commands Add three new tests for project config creation in config_init.rs: - test_config_create_project_creates_file: verifies project config file creation - test_config_create_project_already_exists: tests idempotent behavior Add test_state_get_logs_with_files in config_state.rs to verify log file listing displays correct table headers and file information. * Enable coverage for PTY tests and add CI status tests - Add configure_pty_command() and pass_coverage_env_to_pty_cmd() helpers for passing LLVM coverage env vars through PTY command builders - Update select.rs, approval_pty.rs, shell_wrapper.rs to use helpers - Enable --features shell-integration-tests in dev/coverage.sh - Add CI status parsing integration tests with mock gh commands Coverage for select.rs improved from 47% to 86%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Consolidate PTY setup into shared helpers Add open_pty() and open_pty_with_size() helpers to reduce boilerplate in PTY tests. Net reduction of 80 lines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add shell_command() helper for shell-specific PTY setup Consolidate the repeated shell setup pattern from shell_wrapper.rs tests into a shared helper. The helper handles: - env_clear + HOME + PATH (with optional bin_dir prefix) - Shell-specific env vars (ZDOTDIR for zsh) - Shell-specific isolation flags (--norc, --no-rcs, --no-config) - Coverage env var passthrough Net reduction: 65 lines (122 deleted, 57 added). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix Windows mock gh command not being found On Windows, PATHEXT defines extension search order (.COM;.EXE;.BAT;.CMD;...). This means Windows searches ALL directories for .exe before ANY directory for .cmd. The real gh.exe on GitHub Actions runners was being found before our mock gh.cmd, causing CI status tests to fail. Fix by modifying PATHEXT to prefer .CMD over .EXE in the test environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix Windows mock: create .bat files and adjust PATHEXT Create both .bat and .cmd mock files on Windows, and set PATHEXT to prefer .BAT before .EXE. This ensures Rust's Command::new finds our mock scripts instead of the real gh.exe installed on CI runners. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Consolidate setup_mock_gh() to delegate to setup_mock_gh_with_ci_data() Reduces ~85 lines of duplicated mock script code by having setup_mock_gh() delegate to setup_mock_gh_with_ci_data("[]", "[]"). Both mocks now use the same implementation with empty JSON arrays for PR/run data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Fix Windows mock gh: use %~dp0 for reliable relative paths The batch file now uses %~dp0 (directory of the batch script itself) instead of absolute paths. This avoids issues with path formatting differences between how Rust generates paths and how cmd.exe's type command interprets them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Skip CI status tests on Windows Rust's Command::new("gh") doesn't use PATHEXT for resolution - it looks for .exe directly. Our mock gh.bat scripts aren't found, so we can't mock the gh command reliably on Windows without creating a real .exe wrapper. TODO: Re-enable once we have a reliable Windows mocking solution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |