fix(ci-status): classify tea failures by shape; report azure-devops extension - #3597
Merged
Merged
Conversation
…xtension `tea api` copies the response body to stdout and exits 0, so the `!output.status.success()` gate in the Gitea CI-status backend never fired for an HTTP error. The error body reached `parse_json` as data — and every field of `GiteaCombinedStatus` defaults, so a 500 deserialized as "no statuses" and painted a blank cell. `tea_api_error` now reads the response shape; a non-zero exit stays what it always meant, `tea` itself failing. `wt config show --full` reports the azure-devops extension alongside az's install and auth state: the whole `az repos` command group ships in it, so without it CI status stays blank however well az is authenticated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
worktrunk-bot
approved these changes
Jul 25, 2026
max-sixty
added a commit
that referenced
this pull request
Jul 25, 2026
Follow-up to #3600, independent of it (no shared files). GitHub and GitLab rewrote forge API failures into messages of our own, discarding what the CLI said. GitLab picked its three by prose — `error_text.starts_with("404")` — the pattern #3595/#3597 removed elsewhere. ## Before / after A GitLab MR that doesn't exist: ``` - ✗ MR !9999 not found + ✗ glab api failed for MR !9999 + ▎ glab: 404 Not found (HTTP 404) ``` A GitHub PR that doesn't exist — the one message kept, now with gh's verdict under it rather than instead of it: ``` ✗ PR #999999 not found on max-sixty/worktrunk (gh default). Check that `gh repo set-default` points to the correct repository. + ▎ gh: Not Found (HTTP 404) ``` ## The rule, now written down `src/git/remote_ref/mod.rs` gets an **Error Messages** section: forward the CLI's own line, and write our own only where the CLI *structurally cannot* report the condition. Two cases qualify — GitHub's 404 (it answers about an owner/repo *we* chose, from `gh repo set-default` or a remote; gh can't know that) and Azure's missing `azure-devops` extension (a precondition `az extension list` answers, which `az` never names). Reading more nicely than the CLI doesn't qualify. `cli_api_error` and `cli_api_error_details` gained docstrings for the mechanism: our context as the headline, the CLI's words in the gutter, so a provider with something to add passes it as `message` rather than bailing. ## Evidence Measured, not assumed — both CLIs render an API failure the same way: | | stdout | stderr | |---|---|---| | `gh api …/pulls/999999` | `{"message":"Not Found",…,"status":"404"}` | `gh: Not Found (HTTP 404)` | | `glab api …/merge_requests/…` | `{"message":"401 Unauthorized"}` | `glab: 401 Unauthorized (HTTP 401)` | `cli_api_error_details` prefers stderr, so the gutter gets the human line, not the JSON. That also surfaced a test bug: **the glab mocks never set stderr**, so they'd have enshrined output real glab doesn't produce. Fixed to match the probe. `gh` puts the status in a structured field, so its 404 arm keys on `status`. `glab` puts it only inside the message text — nothing to key on, which is why prose-matching was the only way to keep those arms. ## What this gives up A bad or expired token no longer gets "run gh auth login" / "run glab auth login"; it gets `gh: Bad credentials (HTTP 401)`. The far more common case — no auth configured at all — is unaffected: gh prints its own "please run: gh auth login" and always did fall through to forwarding. Happy to restore the GitHub 401 arm (it's structurally keyable); GitLab's can't come back without prose-matching. ## Testing `cargo run -- hook pre-merge --yes` → 4572 passed. Six snapshots reviewed individually before accepting (five updated, one new). Both paths also driven through the **real** `gh` binary (a live 404 and a live 401), not only mocks. Clippy clean with `--features shell-integration-tests`. Coverage: the GitHub 404 arm and the forwarding fallthrough each have a test, on both forges. `codecov/patch` also flagged one line — re-indenting the 404 block pulled a pre-existing gap into the patch, and it turned out to be real: the `gh repo set-default` half of the hint, the fork workflow the message exists for, had never been tested. `test_switch_pr_not_found_gh_default` closes it. > _This was written by Claude Code on behalf of Maximilian Roos_
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 apinever reads the HTTP status — it copies the response body to stdout and exits 0 — so the!output.status.success()gate insrc/commands/list/ci_status/gitea.rsnever fired for a 4xx/5xx. #3595 fixed this forwt 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
GiteaCombinedStatuscarries#[serde(default)], so Gitea'sAPIErrorbody 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 loggedFailed to parse tea api pulls JSON, blaming an API change for what was an API error.tea_api_errornow reads the response shape — theAPIErrorbody, whose message feeds the existing retriable sniff — and a non-zero exit keeps meaning what it always meant,teaitself 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 --fullreports the azure-devops extensionThe whole
az reposcommand group ships in that extension, so without it CI status stays blank however wellazis authenticated — a persistent, user-fixable condition, which is what this surface is for.azure_devops_extension_installed(added in #3595) answers it in JSON:An
azthat can't answer counts as installed, so an unanswered check never becomes an accusation. Thewt switch pr:<n>bail now uses the same sentence — one wording for one condition, which is the one-line change toswitch_pr_azure_extension_not_installed.snap.Testing
The two new Gitea tests were checked against the unfixed source: reverting
gitea.rsalone 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 1alongside429 Too Many Requestsis the combinationteacan't produce — and now carry a real transport error, which is the case that does exit non-zero.config show --fullgets a case per extension state.