Repository navigation
fix(gitea): key the tea api error shape on the envelope, not on a non-empty message - #3600
Merged
Merged
Conversation
…-empty message
Gitea blanks a 500's message in production unless the token belongs to an
admin, so the body arrives as `{"message":"","url":"…"}`. 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", and `wt switch pr:<n>` told the user
"This may indicate a Gitea API change" — blaming an API change for an API
error.
The key is now the presence of the `message` key, which is the whole shape:
`tea api` never reads the HTTP status, and none of the resources read here
(a PR object, a PR array, a combined commit status) carries a `message`.
One parser, `remote_ref::gitea::api_error_message`, now serves both sites —
the CI-status backend drops its duplicate struct — and a blanked message
reaches the user as an error that says why there is no detail.
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_
The `switch pr:<n>` path had a snapshot for a blanked 500; the PR-list path, which the fix names first, had none. `tea api` returns the same `APIError` envelope there, and requiring a non-empty message let it through to `parse_json`, which warned `Failed to parse tea api pulls JSON` once per branch — blaming a Gitea API change for a Gitea API error. The warning is a `tracing` record and the stderr layer is off at the default verbosity, so the test drives `wt list --full` under `RUST_LOG=warn` and reads stderr rather than snapshotting; `-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 keeps the first case from passing merely because nothing logs, and pins the wording to unknown shapes. Verified against the pre-fix key: case 1 fails with the misleading warning, case 2 passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
max-sixty
marked this pull request as ready for review
August 2, 2026 18:31
1 task
social4hyq
pushed a commit
to social4hyq/homebrew-core
that referenced
this pull request
Sep 20, 2026
worktrunk 0.72.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## Release Notes ### Improved - **A host carrying a forge's name anywhere resolves to that forge again**: 0.71.0 required `github`, `gitlab`, or `gitea` as a whole dot-separated label, which read as an ownership check but wasn't one — an attacker controls their own DNS — while shutting out self-hosters with hyphenated names. `github-enterprise.acme.com`, `mygithub.com`, and the `github-personal` SSH alias classify again, so CI status, `wt switch --prs`, and `repo.provider` work with no config. ([#3673](max-sixty/worktrunk#3673)) - **One `[projects."…"]` entry can cover every repository on a host, and can set the forge**: A key containing `*` matches any run of characters, `/` included, so `[projects."git.company.example/*"]` covers a whole host; every matching entry applies, least- to most-specific. The table also gained `forge.platform` and `forge.hostname`, so a self-hosted host needs one entry here rather than a block in every repo. [Docs](https://worktrunk.dev/config/#user-project-specific-settings) ([#3701](max-sixty/worktrunk#3701), thanks @chrishas35 for the request and @witt-bit for the workspace-scoped case it partly serves) - **`wt merge` and `wt step push` leave the target worktree's uncommitted changes in place**: The autostash that held a dirty target's changes restored them as unstaged; both now advance the target with a compare-and-swap `update-ref` and `read-tree -m -u`, which leaves uncommitted work untouched. (Breaking: the fast-forward path no longer runs `git push`, so `pre-push` and the receive-side hooks no longer fire, and a failed sync errors with the ref rolled back.) ([#3703](max-sixty/worktrunk#3703), [#3684](max-sixty/worktrunk#3684), [#3693](max-sixty/worktrunk#3693), thanks @gubasso for reporting) - **Approval state and branch-removal outcomes are machine-readable**: `wt config approvals list --format=json` reports whether a non-interactive run would stop for approval. `wt remove` and `wt step prune` replace `branch_deleted` with `branch_outcome`: `deleted`, `deferred`, `not_attempted`, `retained_unmerged`, `retained_checked_out`, `retained_raced`, `retained_failed`. (Breaking.) ([#3710](max-sixty/worktrunk#3710), thanks @NathanaelRea for the requests) - **Every commit hash worktrunk prints follows `core.abbrev`**: The `wt list` table, `wt switch --prs`'s `log` tab, and `wt config state`'s CI cache table sliced to 8 characters while `--format=json` carried git's `%h`. All now ask git how wide it abbreviates in this repo. ([#3676](max-sixty/worktrunk#3676), [#3677](max-sixty/worktrunk#3677)) - **`wt list --format=json` schema 2 has a published JSON Schema**: [worktrunk.dev/schema/list-v2.json](https://worktrunk.dev/schema/list-v2.json) holds the contract, and `wt list --print-schema` prints the same document. Four fields that were bare strings are now enumerated vocabularies; the emitted JSON is unchanged. ([#3747](max-sixty/worktrunk#3747)) - **A detached worktree is named by its commit, not `-`**: The Branch cell hardcoded `-`, which reads as missing data rather than a state; it now carries the row's abbreviated HEAD, and the picker and statusline name the worktree the same way. ([#3675](max-sixty/worktrunk#3675)) ### Fixed - **Piped output is plain and no longer panics**: `wt list | head -3` exited 101 with `failed printing to stdout: Broken pipe`, and `wt list` wrote ANSI to a pipe unconditionally. Every stdout surface now exits cleanly, and the human-read ones are plain when piped unless `CLICOLOR_FORCE=1`. ([#3746](max-sixty/worktrunk#3746), [#3766](max-sixty/worktrunk#3766)) - **A CI check that hasn't finished no longer reads as passed**: Each forge's status parser missed documented values, so a GitHub PR parked on an approval gate showed green, and GitLab's `canceling` and Azure DevOps's `postponed` read as no CI at all. ([#3741](max-sixty/worktrunk#3741), [#3740](max-sixty/worktrunk#3740)) - **The shell wrappers survive an `rm` alias and a failing `--execute`**: Aliases bake into the wrapper at parse time, so `alias rm='rm -v'` reached its cleanup — noise on zsh and bash, and on nushell an abort that leaked three temp files, as a failing `--execute` body also did. ([#3714](max-sixty/worktrunk#3714), thanks @Ar4l), ([#3732](max-sixty/worktrunk#3732), [#3734](max-sixty/worktrunk#3734)) - **An alias or hook wrapping `wt switch` or `wt remove` keeps your subdirectory**: The user's position came from the `wt` process's cwd, which inside an alias body is the worktree root, so an aliased `wt remove` from `feature/apps/gateway` landed at the primary worktree's root. Fixes [#3723](max-sixty/worktrunk#3723). ([#3724](max-sixty/worktrunk#3724), thanks @vivienm for reporting) - **`wt merge` and `wt step push` refuse a target worktree parked mid-operation**: The two-tree sync refuses an unmerged index but not a stopped cherry-pick or rebase whose conflict was already staged, so the push range could land in a paused target and be committed by `--continue`. ([#3759](max-sixty/worktrunk#3759)) - **The Claude plugin's worktree-remove hook resolves against the worktree path**: The hook anchored at `CLAUDE_PROJECT_DIR`, which the `claude agents` view routinely leaves outside any repository, so `wt remove` died with `not a git repository` and the session became undeletable. Its guard now also requires a `.git` entry. ([#3754](max-sixty/worktrunk#3754), [#3767](max-sixty/worktrunk#3767), thanks @judewang for the fix and the report) - **A Gitea API error is reported as one, not as a parse failure**: `tea api` exits 0 whatever the HTTP status, so both call sites guessed from the body's shape and blamed an API change for an API error. `--include` surfaces the status instead. ([#3713](max-sixty/worktrunk#3713), [#3600](max-sixty/worktrunk#3600)) - **`--print-schema` and the doc-generation help flags name the right command**: All three found the subcommand by scanning `argv` for a `/wt` suffix, which never matches `wt.exe` under a backslash path, so on Windows they read the binary's own path as the command. ([#3762](max-sixty/worktrunk#3762)) - **A multibyte shell name no longer panics**: `extract_filename_from_path` sliced at `len() - 4` to test for `.exe` with no char-boundary check: `SHELL=/bin/日本語 wt config show` panicked, and on macOS every process name goes through it during shell detection. ([#3727](max-sixty/worktrunk#3727)) - **`wt` installed under a dotted name generates shell integration for that name**: `binary_name` used `file_stem`, which cuts at the last dot, so `wt config shell init bash` under `wt.old` emitted a wrapper for `wt`. It now strips only the executable suffix. ([#3719](max-sixty/worktrunk#3719)) - **Concurrent `wt step prune` removals no longer race the worktree registry**: `git worktree remove` reads every sibling under `.git/worktrees/`, so two overlapping removals could have one read a sibling mid-teardown. Registry-mutating removals now serialize behind a second lock. ([#3692](max-sixty/worktrunk#3692)) - **The `wt switch` first-run offer previews the legacy files it removes**: Accepting "Install shell integration?" could delete a deprecated worktrunk-managed wrapper the prompt never named. What gets removed is unchanged. ([#3656](max-sixty/worktrunk#3656)) - **`wt config create --project` writes a resolvable link**: The comment it writes into `.config/wt.toml` carried a raw Zola target, because the link-conversion regex stopped at the first `]` — here the one closing a nested code span. ([#3731](max-sixty/worktrunk#3731)) - **`wt list --branches` counts a local branch containing `/` as local**: The summary tally classified branch-only rows by `branch.contains('/')`, so a local `feature/login` counted under "N remote branches". ([#3687](max-sixty/worktrunk#3687)) - **`wt step relocate`'s human summary counts template-error branches as skipped**: `--format=json` already folded them into `skipped`; the human tally undercounted by the number of branches whose `worktree-path` template failed to expand. ([#3688](max-sixty/worktrunk#3688)) ### Documentation - **SignPath attribution appears with the artifacts it describes**: The code-signing notice and a route to the policy now sit in the install section's Windows block on the README and the docs landing page, as SignPath Foundation's OSS program requires. ([#3709](max-sixty/worktrunk#3709)) - **`wt step copy-ignored`'s `--require-include` example renders as a terminal block**: It was the only `console` block in the command's long help missing the `$ ` prefix. ([#3706](max-sixty/worktrunk#3706)) ### Internal - **Library API rework** (Breaking library API): `cargo-semver-checks` fails five lints — `LegacyForgeAlias` and `Repository::legacy_forge_alias` removed with the forge-classification revert, `Repository::forge_platform_override` removed for one shared resolver, `stage_worktree_removal` gained two parameters, `UserProjectOverrides` gained a `forge` field, and the `real-repo-benches` feature was removed. ([#3673](max-sixty/worktrunk#3673), [#3694](htt See merge request: Harmonybrew/homebrew-core!16275
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
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.
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 listloggedFailed 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-vorRUST_LOG.wt switch pr:<n>failed withFailed 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
messagekey, which is the whole shape.tea apinever reads the HTTP status —runApicopies the body to stdout and returns nil regardless — so the body is the only channel, and none of the resources read here carries amessage: verified against giteamainforPullRequest,CombinedStatus, andCommitStatus.urlis deliberately not part of the key. The two mistakes aren't symmetric: an error body that omitsurlwould 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 amessagefield. Gitea already ships error shapes without one (APIInvalidTopicsErrorismessageplusinvalidTopics), andCombinedStatusdoes carryurl.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:The CI cell for a blanked 500 stays blank, unchanged:
is_retriable_erroris 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_shapeflips its blank-message assertion (that assertion was the bug), a newtest_api_error_message_reads_the_response_shapepins the key against every shape both sides see, andswitch_pr_gitea_blank_error_messagesnapshots the user-visible path alongside the existing 401/403/404 cases.test_list_full_gitea_parse_warning_is_reserved_for_unknown_bodiescovers the PR-list side end to end. It driveswt list --fullunderRUST_LOG=warnand reads stderr rather than snapshotting, since the warning is atracingrecord that the stderr layer suppresses at the default verbosity (-vwould 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
apicommand has an--includeflag that writes the status line to stderr — a genuine structured channel that would beat shape-sniffing. It's in tea'smainsource but in no released changelog entry, so sending it would break every user whoseteapredates it. It would also compose with this change rather than replace it, since the body still supplies the message text.🤖 Generated with Claude Code