Skip to content

fix(gitea): key the tea api error shape on the envelope, not on a non-empty message - #3600

Merged
max-sixty merged 3 commits into
mainfrom
gitea-blank-error-message-shape
Aug 2, 2026
Merged

max-sixty merged 3 commits into
mainfrom
gitea-blank-error-message-shape

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 25, 2026 •

Copy link
Copy Markdown
Owner

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

This was written by Claude Code on behalf of Maximilian

…-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_
max-sixty and others added 2 commits August 2, 2026 10:42
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
max-sixty marked this pull request as ready for review August 2, 2026 18:31
@max-sixty
max-sixty merged commit 10e8bad into main Aug 2, 2026
37 checks passed
@max-sixty
max-sixty deleted the gitea-blank-error-message-shape branch August 2, 2026 18:31
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant