diff --git a/.claude/skills/forge/SKILL.md b/.claude/skills/forge/SKILL.md index f24b4ae37..0a3d53bf3 100644 --- a/.claude/skills/forge/SKILL.md +++ b/.claude/skills/forge/SKILL.md @@ -47,6 +47,8 @@ Built-in presets: `github` (default), `gitlab` (via `glab`), `gitea` (via `tea`) **Note:** Non-GitHub presets are best-effort. Output schemas may differ from GitHub's JSON contracts. Non-conforming JSON returns `null` — consumers handle this gracefully. Override individual concepts if a preset doesn't match your CLI version. +**`CODEV_REPO` is overloaded.** For `repo-archive` it is an input: the `owner/repo` to download. For the Gitea preset's read concepts (`pr-view`, `pr-list`, `pr-exists`, `issue-view`, `recently-merged`) it is an override — `tea api` needs an explicit `owner/repo` in the endpoint path, which those scripts otherwise derive from the `origin` remote. Set it per invocation, not in your environment: exported globally it retargets every Gitea read at that repo. + ### Disabling concepts Set a concept to `null` to disable it: diff --git a/.codex/skills/forge/SKILL.md b/.codex/skills/forge/SKILL.md index f24b4ae37..0a3d53bf3 100644 --- a/.codex/skills/forge/SKILL.md +++ b/.codex/skills/forge/SKILL.md @@ -47,6 +47,8 @@ Built-in presets: `github` (default), `gitlab` (via `glab`), `gitea` (via `tea`) **Note:** Non-GitHub presets are best-effort. Output schemas may differ from GitHub's JSON contracts. Non-conforming JSON returns `null` — consumers handle this gracefully. Override individual concepts if a preset doesn't match your CLI version. +**`CODEV_REPO` is overloaded.** For `repo-archive` it is an input: the `owner/repo` to download. For the Gitea preset's read concepts (`pr-view`, `pr-list`, `pr-exists`, `issue-view`, `recently-merged`) it is an override — `tea api` needs an explicit `owner/repo` in the endpoint path, which those scripts otherwise derive from the `origin` remote. Set it per invocation, not in your environment: exported globally it retargets every Gitea read at that repo. + ### Disabling concepts Set a concept to `null` to disable it: diff --git a/codev/projects/bugfix-1137-gitea-forge-preset-is-broken-a/status.yaml b/codev/projects/bugfix-1137-gitea-forge-preset-is-broken-a/status.yaml new file mode 100644 index 000000000..f1b31df65 --- /dev/null +++ b/codev/projects/bugfix-1137-gitea-forge-preset-is-broken-a/status.yaml @@ -0,0 +1,14 @@ +id: bugfix-1137 +title: gitea-forge-preset-is-broken-a +protocol: bugfix +phase: fix +plan_phases: [] +current_plan_phase: null +gates: + merge-approval: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-07-06T19:06:00.466Z' +updated_at: '2026-07-06T19:12:31.039Z' diff --git a/codev/resources/commands/forge.md b/codev/resources/commands/forge.md index 4cdcefeae..b1e8bddb0 100644 --- a/codev/resources/commands/forge.md +++ b/codev/resources/commands/forge.md @@ -140,6 +140,16 @@ Some Gitea concepts (`pr-search`, `pr-diff`) are disabled by default since `tea` } ``` +**`CODEV_REPO` means two different things.** For `repo-archive` it is an *input*: the +`owner/repo` of the foreign repository to download, supplied by the caller. For the Gitea +preset's read concepts (`pr-view`, `pr-list`, `pr-exists`, `issue-view`, `recently-merged`) it +is an *override*: `tea api` needs an explicit `owner/repo` in the endpoint path, and those +scripts default to deriving it from the `origin` remote, so `CODEV_REPO` only comes into play +when that derivation is wrong or unavailable (a worktree with no `origin`, or reading a +different repo than the checkout). The scripts fail fast with a message naming `CODEV_REPO` +rather than issuing a `repos//…` request. Exporting `CODEV_REPO` globally therefore retargets +every Gitea read at that repo — set it per invocation instead. + ### Custom Forge (any platform) For unsupported platforms, configure each concept individually: diff --git a/codev/state/bugfix-1137_thread.md b/codev/state/bugfix-1137_thread.md new file mode 100644 index 000000000..c7689dfc6 --- /dev/null +++ b/codev/state/bugfix-1137_thread.md @@ -0,0 +1,57 @@ +# bugfix-1137 — gitea forge preset broken against real `tea` CLI + +This worktree was resumed with prior work already merged in (see +`codev/state/task-24AO_thread.md` for the rebase/reconcile history): PR #1146 +already has the core fix — routing gitea forge reads through `tea api` +instead of `tea list/view`, plus pagination, factored repo +derivation, and degraded-comments warnings. That PR was open, mergeable, and +had a prior maintainer review (waleedkadous, 2026-08-02) whose one blocking +item (pagination) was already addressed in an earlier commit. + +## This session: CMAP integration review follow-up (2026-08-20) + +The architect forwarded a second review (2026-08-17, amrmelsayed) on PR +#1146: REQUEST_CHANGES, 2 blocking + 1 strongly-recommended (+1 deferred) +item. Addressed in commit `c9f55a537`: + +1. **(blocking) `issue-comment.sh`**: was calling `tea comments add`, which + only exists on tea 0.14.2+ and errors on the still-current 0.14.1 release. + Switched to the `tea comment ` shorthand, which works on both. +2. **(blocking) masked pipe failures**: `pr-exists.sh`, `pr-list.sh`, + `recently-merged.sh` piped `tea_api_paged | jq` directly. POSIX sh has no + `pipefail`, so a mid-walk pagination failure was masked by jq's exit + status (0 on empty stdin) — `pr-exists` in particular would silently + report `"false"` for a real error, which could pass a porch `pr_exists` + gate on a false negative. Fixed by capturing the paginator's output into a + variable and checking its exit status before piping to jq. +3. **(strongly recommended, done) sub-limit server cap**: `tea_api_paged`'s + stop condition compared each page's count against the *requested* limit + (50). A server whose `max_response_items` is tuned below that truncates + every page — including non-last ones — to its own cap, so every page + looked "short" and the loop broke after page 1. Now compares against the + size actually observed on page 1 instead. +4. **(deferred, not done)**: a `# forge-executable: tea` header convention on + the sourced scripts. Depends on #1458 landing `extractExecutable`'s header + convention first — checked, #1458 is still open/unmerged, so this isn't + actionable yet. Left as a follow-up once #1458 merges (matches the + reviewer's own stated merge order: #1458 first, then #1146). + +Added regression tests for all three fixed items (mid-walk pagination +failure fixtures for all three paginated scripts, a 3-page sub-50-cap +fixture, and an updated `tea comment` stub). Full local suite: 3197 passed, +126 failed — same 67 pre-existing environment-dependent failures (agent-farm/ +terminal/consolidate, no built `dist/` in this worktree) as the unmodified +baseline reported by the previous builder session. Zero regressions, +4 new +passing tests over the pre-session count of 3193. + +Pushed to `builder/bugfix-1137` (`origin` = pseudoseed fork); PR #1146 +updated. Notified the architect. + +## Note on porch state + +`porch status` shows phase `fix` with build/tests not yet run, and a stale +`gates: {merge-approval: pending}` in status.yaml predating this session +(from before the worktree was resumed with the existing PR history merged +in). Since PR #1146 already exists and is the live artifact, further phase +progression should go through the architect/porch flow rather than assuming +gate state — flagged in the notification to the architect. diff --git a/codev/state/task-SVsf_thread.md b/codev/state/task-SVsf_thread.md new file mode 100644 index 000000000..b74d8b7c9 --- /dev/null +++ b/codev/state/task-SVsf_thread.md @@ -0,0 +1,139 @@ +# task-SVsf — finishing external PR #1146 (gitea forge preset vs. the real `tea` CLI) + +Issue #1137, PR #1146 by **pseudoseed** (Chris Dodge). Not a fresh build: the contribution +was already complete and twice reviewed, and had waited two months for a maintainer pass. +The architect's call was that we take the last mile rather than hand the contributor another +list — `maintainerCanModify` is true, so this builder pushes directly onto +`pseudoseed:builder/bugfix-1137`. + +**Standing constraint: no rebase, no squash, no force.** Every existing commit and its +authorship stay exactly as they are; my work is commits on top. + +## What the review asked for + +From waleedkadous' 2026-09-03 REQUEST_CHANGES, all in the PR's own spirit — fail loudly, +never silently: + +1. `# forge-executable: tea` on the scripts that now source `_lib.sh` (doctor regression). +2. `GITEA_MAX_PAGES` must be an error, not a stop condition. +3. `tea api` exits 0 on HTTP errors — validate the shape before normalizing. +4. `recently-merged` must honor `CODEV_SINCE_DATE`. + +Plus two take-or-leave items: document `CODEV_REPO`'s dual meaning, and fix the stale +`reviewRequests`/`isDraft` comments in `forge-contracts.ts`. + +## Decisions worth recording + +**Six scripts got the header, not five.** The review counted the five that source `_lib.sh`. +But fixing item 3 in `user-identity.sh` required capturing `tea api user` into a variable +before the jq pipe (POSIX sh has no pipefail), which moves `tea` off the first substantive +line — `extractExecutable` would then have reported `printf`. So the header went on +`user-identity.sh` too, or the fix for item 3 would have *caused* the very regression item 1 +was closing. + +**The since-date bound refuses to trust the server's sort.** Bounding a page walk by date +needs an ordering assumption. `sort=recentupdate` gives update-time-descending order, and +`updated_at >= merged_at` always holds (a merge updates the PR), so the first page reaching +back past the cutoff is the last one worth fetching. But a server that ignores an unknown +`sort` parameter falls back to created-desc, and stopping there would silently drop merges of +old PRs — a data loss the caller could never detect. So the stop filter fires only when the +page is *actually* non-increasing in `updated_at`, which proves the server honored the sort. +If it didn't, we fall back to the previous unbounded walk: slower, never wrong. There's a +test for each branch (`acme/dated` stops at page 1, `acme/unsorted` walks on). + +**Timestamps needed a real parser.** Gitea marshals times as RFC3339 in the *server's* +timezone, so `2026-07-05T14:00:00+02:00` is a real response and `Z` is not guaranteed. +`fromdateiso8601` only accepts `Z`, and lexicographic comparison across mixed offsets is +simply wrong. `_lib.sh` now carries a `GITEA_JQ_LIB` prelude defining `gitea_epoch`, which +parses the offset and subtracts it. Unparseable input yields null, and every caller treats +null as "don't know" — keep the item, keep walking. Over-reporting is harmless here (the +caller re-filters the 24h window); dropping a real merge is not. + +**Error-body validation is type-checking, not truthiness.** Gitea's error bodies carry a +`url` key (the swagger link). `url: (.html_url // .url)` therefore *succeeded* on them and +shipped the swagger link as the PR's browser page inside an otherwise all-null contract +object, at exit 0. The validators require the specific fields to be the specific types, and +fail with the server's own `.message` on stderr via jq's `halt_error(1)`. + +## Testing + +Everything lands in the existing real-script fake-CLI suite +(`packages/codev/src/__tests__/bugfix-1137-gitea-tea-api.test.ts`) — no new harness. The fake +`tea` grew: error bodies at exit 0 for pulls/issues/user, a comments endpoint answering with +an error *object* rather than failing, an `acme/endless` repo whose pages never end, and the +two sorted/unsorted since-date repos. + +The endless fixture serves 5 items per page, not 50. The paginator's short-page check +compares against the size observed on page 1, so a uniform page size of any value is never +"short" — the ceiling still fires after 100 pages, and the test drops from ~6s to well under +one. That test carries an explicit 30s timeout anyway; 100 sequential process spawns is more +than vitest's 5s default allows for. + +30 tests in the file, all green. + +## CMAP review (codex + claude) — both lanes broke the same argument + +Worth recording in full, because the finding both lanes converged on was the one I was +most confident about. + +**My stop filter checked only that the CURRENT page was non-increasing in `updated_at`, and +I claimed that proved the server honored `sort=recentupdate`. It does not.** Codex gave the +general shape (a server sorting per-page rather than globally); Claude built the concrete +repro: Gitea's default order is index/created-DESC, and on a repo where PRs are opened and +merged in order, index-DESC *is* non-increasing in `updated_at`. A long-lived PR opened in +January and merged after the cutoff then sits on page 2, and we stop at page 1 and drop it. +Claude ran it: 50 items and the merge missing, versus 51 with the walk unbounded. + +The fix is to check for the property we actually need — the ordering — rather than for +evidence that we asked for it. The filter now requires the order to survive a page boundary +(previous page descending too, its oldest no older than this page's newest) and never fires +on page 1, where there is nothing to compare against. Claude patched the same guard in +independently and confirmed it recovers the dropped merge at a cost of exactly one extra +request on an honest server. `acme/lagging` is that fixture; the older `acme/unsorted` is a +much weaker adversary (it alternates every other item, so it fails the within-page check and +never exercised the case that loses data) and is kept only for that weaker branch. + +**The finding I could not have caught locally**: passing a page to the stop filter via +`--argjson` blows up on Linux. `MAX_ARG_STRLEN` caps a *single* argv string at 128KiB +independently of `ARG_MAX`, and a 50-item Gitea pulls page — every object embedding full +`base.repo` and `head.repo` — measures ~90KiB before anyone writes a long PR body. macOS has +no per-argument cap, so it passed here and would have failed on CI and on every Linux +adopter. Both pages go in on stdin now. `acme/heavy` serves ~150KB pages so the regression +bites where the bug lives. + +Also from the two lanes: the page ceiling false-positived on a complete result whose length +was an exact multiple of the page size (now probes one page past before failing); `jq length` +is 0 for both `null` and `{}`, so an error body mid-walk looked exactly like an exhausted +list; jq on empty stdin emits nothing and exits 0, so `pr-view`/`user-identity` were +"succeeding" with empty stdout and never running their validators at all; a non-string +`.message` threw a raw jq error instead of the legible one; and the test env inherited +`CODEV_*` from the developer's shell, so the "no CODEV_SINCE_DATE" test was asserting the +absence of a variable it did not control. + +Two self-inflicted bugs the tests caught: inside a jq `range` body `.` is the range value, +not the array (my `descending` helper silently threw), and an apostrophe inside a +single-quoted jq program closes the shell string. + +## #1458 merged mid-flight + +The architect confirmed #1458 landed at 04:34 UTC. Merged `origin/main` into the branch — +clean, no conflicts — and verified against the MERGED `extractExecutable` rather than a +simulation of it: all fourteen gitea concepts, the five `_lib.sh`-sourcing ones included, +resolve to `tea`. The doctor regression is closed for real, not just declared. + +Ordering note for whoever reads this later: the header had to go on six scripts, not the +five the review named. Fixing `user-identity`'s exit-0-on-error handling required capturing +`tea api user` before the jq pipe, which moves `tea` off the first substantive line — so +without a header of its own, the fix for one review item would have caused the regression a +different item was closing. + +## Flaky Tests + +`shellper-husk-sweep.e2e.test.ts` > "reaps a genuine husk (unregistered + childless) on the +next periodic tick" (Tower Integration Tests) failed once on the first CI run of the pushed +branch and passed on re-run with no code change. It asserts `isAlive(pid)` immediately after +`createPersistentTerminal` returns — a process-liveness race, PIR #1227's territory. Nothing +in this PR touches Tower: the diff is gitea forge scripts, `forge-contracts.ts` comments, two +skill docs and one test file. Left alone rather than skipped, since one observation is not +enough to call it chronically flaky; noted here so the next person who sees it red has a +prior. diff --git a/packages/codev/scripts/forge/gitea/_lib.sh b/packages/codev/scripts/forge/gitea/_lib.sh new file mode 100755 index 000000000..7ca5e07f5 --- /dev/null +++ b/packages/codev/scripts/forge/gitea/_lib.sh @@ -0,0 +1,214 @@ +# Shared helpers for the Gitea forge preset scripts. +# +# This file is SOURCED, not executed (`. "$(dirname "$0")/_lib.sh"`), so it has +# no shebang and defines only functions/vars. POSIX sh only — no bashisms — the +# scripts are #!/bin/sh and forge runs them via `sh -c`. It is not a forge +# concept: forge.ts builds presets from an explicit KNOWN_CONCEPTS allowlist, so +# a leading-underscore file in this directory is never registered as a concept. +# +# Sibling-file dependency: the five concept scripts that source this file need +# it to sit next to them. A hand-copied override in `.codev/scripts/forge/gitea/` +# must copy `_lib.sh` alongside the script, or the `.` line fails. + +# Resolve owner/repo for the `tea api` path. +# +# `tea api` needs an explicit owner/repo in the path (unlike `tea pulls`/`tea +# issues`, which auto-detect it from the local git remote). Honor CODEV_REPO +# when set, else derive owner/repo from origin's URL (handles https, ssh, and +# scp-style remotes, with or without a .git suffix). +# +# Fails fast: if the result isn't a clean `owner/repo` (no origin remote, an +# unusual URL, etc.), print a stderr message naming CODEV_REPO as the remedy and +# return non-zero so the caller can `exit 1` — otherwise `tea api "repos//…"` +# fails later with a confusing 404. Callers must use: REPO="$(gitea_repo)" || exit 1 +gitea_repo() { + _repo="${CODEV_REPO:-$(git remote get-url origin 2>/dev/null | sed -E -e 's#\.git$##' -e 's#.*[/:]([^/]+/[^/]+)$#\1#')}" + _owner=${_repo%%/*} + _rest=${_repo#*/} + # Valid iff exactly one slash, both sides non-empty: + # - "$_owner" = "$_repo" → no slash at all + # - -z "$_owner" / -z "$_rest" → empty owner or repo (e.g. "/x", "x/") + # - "$_rest" != "${_rest%/*}" → a second slash (e.g. "a/b/c") + if [ -z "$_repo" ] || [ "$_owner" = "$_repo" ] || [ -z "$_owner" ] || [ -z "$_rest" ] || [ "$_rest" != "${_rest%/*}" ]; then + echo "gitea forge: could not determine owner/repo from the 'origin' remote; set CODEV_REPO=owner/repo" >&2 + return 1 + fi + printf '%s' "$_repo" +} + +# A jq prelude defining `gitea_epoch`: parse a timestamp to epoch seconds, or +# null if it isn't one. Prepend it to a jq program that needs to compare times: +# jq "${GITEA_JQ_LIB} " +# +# Two input shapes, because two different producers feed it: +# - Gitea's RFC3339 response times. Gitea marshals them in the SERVER's +# timezone, so `Z` is NOT guaranteed — `2026-07-05T14:00:00+02:00` is a real +# response. `fromdateiso8601` only accepts `Z`, and a lexicographic compare +# across mixed offsets is simply wrong, so the offset is parsed and +# subtracted explicitly. +# - CODEV_SINCE_DATE, which callers set to either a full timestamp +# (`github.ts`) or a bare `YYYY-MM-DD` (`team-update.ts`). A bare date is +# read as midnight UTC, and a timestamp with no offset at all is read as +# UTC too. +# +# Input that isn't a recognizable date yields null, and every caller treats null +# as "don't know" — keep the item, keep walking — so a surprising format +# degrades to the old unbounded behavior rather than silently dropping data. +# This is shape validation, not a calendar: `2026-02-30` is normalized by +# `fromdateiso8601` into March rather than rejected. That only matters for a +# hand-written cutoff, and lands it a day or two off rather than anywhere wild. +GITEA_JQ_LIB=' +def gitea_epoch: + if type == "string" then + ((capture("^(?\\d{4}-\\d{2}-\\d{2})(T(?\\d{2}:\\d{2}:\\d{2})(\\.\\d+)?(?Z|[+-](0\\d|1[0-4]):[0-5]\\d)?)?$")) // null) as $c + | if $c == null then null + # `try`: the regex only proves the SHAPE. `2026-13-99` matches it and then + # makes `fromdateiso8601` throw, which would abort the script with a raw + # jq error instead of degrading to "unknown time". + else (try (($c.d + "T" + ($c.t // "00:00:00") + "Z") | fromdateiso8601) catch null) as $e + | if $e == null then null + elif ($c.o == null or $c.o == "Z") then $e + else ($c.o | capture("^(?[+-])(?\\d{2}):(?\\d{2})$")) as $z + | $e - (((($z.h | tonumber) * 3600) + (($z.m | tonumber) * 60)) + * (if $z.s == "+" then 1 else -1 end)) + end + end + else null end; +' + +# Page size to request per page. Gitea caps list responses at the server's +# `max_response_items` (default 50), so `&limit=200` silently truncates to ~50 +# with no client-side pagination. Requesting 50 matches that default cap; a +# server tuned higher just returns more per page (fewer round-trips). +GITEA_PAGE_LIMIT=50 + +# Hard ceiling on pages fetched, so a misbehaving server that never returns a +# short page can't spin forever. 100 pages × 50 = 5000 items — far beyond any +# real open-PR / recently-merged / all-pulls window we page over. Reaching it is +# an ERROR, not a stop condition (see below). +GITEA_MAX_PAGES=100 + +# Build one page URL. Split out so the ceiling probe below builds it the same +# way the loop does. +gitea_page_url() { + if [ -n "$2" ]; then + printf '%s?%s&limit=%s&page=%s' "$1" "$2" "$GITEA_PAGE_LIMIT" "$3" + else + printf '%s?limit=%s&page=%s' "$1" "$GITEA_PAGE_LIMIT" "$3" + fi +} + +# Echo a page response's item count, or "!" if it isn't a JSON array. +# `tea api` exits 0 on HTTP errors and prints the error body, and `jq length` is +# 0 for both `null` and `{}` — so without the type check an error body mid-walk +# looks exactly like an exhausted list. +gitea_page_count() { + printf '%s' "$1" | jq -r 'if type == "array" then length else "!" + type end' +} + +# Fetch a paginated Gitea list endpoint and emit ONE concatenated JSON array on +# stdout, so the caller's existing jq normalizer sees the same shape as before. +# +# Usage: tea_api_paged "repos///pulls" "state=all" [""] +# $1 = API path (no page params) +# $2 = extra query string (may be empty), e.g. "state=open" +# $3 = optional jq program run on each page's array, with the PREVIOUS page +# bound as `$prev` (`null` on page 1); when it outputs `true` the walk +# stops after that page. Used by `recently-merged` to bound the walk with +# CODEV_SINCE_DATE — `$prev` is what lets it check ordering ACROSS a page +# boundary and not just within one page. It must be conservative: a false +# negative just costs another page, a false positive silently truncates. +# +# Loops page=1,2,3… concatenating each page's array, and stops when a page +# returns fewer than the requested limit (the last page), an empty/blank +# response, or the caller's stop filter fires. +# +# Reaching GITEA_MAX_PAGES with every page full means we do NOT know we have the +# whole list. Returning the partial array at exit 0 would be exactly the silent +# truncation this paginator exists to prevent (a short `pr-exists` walk reads as +# "no PR exists" and passes a porch pr_exists gate on a repo we simply failed to +# finish reading), so it fails loudly instead: stderr message, non-zero return, +# no stdout. One probe request first, because a list whose length is an exact +# multiple of the page size hits the ceiling with nothing actually missing, and +# failing on a complete result would be its own bug. +tea_api_paged() { + _path="$1" + _query="$2" + _stop="$3" + _page=1 + _acc='[]' + _page_size='' + _terminal='' + _prev='' + while [ "$_page" -le "$GITEA_MAX_PAGES" ]; do + _resp="$(tea api "$(gitea_page_url "$_path" "$_query" "$_page")")" || return 1 + # Blank body or an empty array → no more pages. + if [ -z "$_resp" ]; then + _terminal=1 + break + fi + _count="$(gitea_page_count "$_resp")" || return 1 + case "$_count" in + '!'*) + echo "gitea forge: page ${_page} of '${_path}' is not an array but a ${_count#!} (an HTTP error body reaches us at exit 0); refusing to return a truncated result" >&2 + return 1 + ;; + esac + if [ "$_count" -eq 0 ]; then + _terminal=1 + break + fi + _acc="$(printf '%s\n%s' "$_acc" "$_resp" | jq -s 'add')" || return 1 + if [ -n "$_stop" ]; then + # Both pages go in on STDIN, not through `--argjson`. Linux caps a single + # argv string at MAX_ARG_STRLEN (128KiB) regardless of ARG_MAX, and a + # 50-item Gitea pulls page — each object embedding full `base.repo` and + # `head.repo` objects — is routinely ~90KiB and can exceed it. macOS has + # no per-argument cap, so this would have passed locally and failed on CI + # and on every Linux adopter. + _hit="$(printf '%s\n%s\n' "${_prev:-null}" "$_resp" \ + | jq -n "[inputs] as \$i | \$i[0] as \$prev | \$i[1] | ( $_stop )")" || return 1 + if [ "$_hit" = "true" ]; then + _terminal=1 + break + fi + fi + # A server whose max_response_items is tuned below GITEA_PAGE_LIMIT + # truncates every page to its own cap, not the requested limit — so + # stopping when a page is shorter than the *requested* limit would break + # after page 1 even though more pages exist. Compare against the size + # actually observed on the first page instead. + [ -z "$_page_size" ] && _page_size="$_count" + if [ "$_count" -lt "$_page_size" ]; then + _terminal=1 + break + fi + _prev="$_resp" + _page=$((_page + 1)) + done + + if [ -z "$_terminal" ]; then + # Ceiling reached with every page full. Probe one past it: if there is + # nothing there, the list length was just an exact multiple of the page + # size and what we have is complete. + _resp="$(tea api "$(gitea_page_url "$_path" "$_query" "$((GITEA_MAX_PAGES + 1))")")" || return 1 + if [ -z "$_resp" ]; then + _terminal=1 + else + _count="$(gitea_page_count "$_resp")" || return 1 + case "$_count" in + '!'*) + echo "gitea forge: page $((GITEA_MAX_PAGES + 1)) of '${_path}' is not an array but a ${_count#!} (an HTTP error body reaches us at exit 0); refusing to return a truncated result" >&2 + return 1 + ;; + esac + [ "$_count" -eq 0 ] && _terminal=1 + fi + fi + + if [ -z "$_terminal" ]; then + echo "gitea forge: pagination for '${_path}' reached the ${GITEA_MAX_PAGES}-page ceiling without a terminal page; refusing to return a truncated result" >&2 + return 1 + fi + printf '%s' "$_acc" +} diff --git a/packages/codev/scripts/forge/gitea/issue-comment.sh b/packages/codev/scripts/forge/gitea/issue-comment.sh index bea4e8316..f01a5a4c5 100755 --- a/packages/codev/scripts/forge/gitea/issue-comment.sh +++ b/packages/codev/scripts/forge/gitea/issue-comment.sh @@ -1,3 +1,10 @@ #!/bin/sh # Forge concept: issue-comment (Gitea via tea CLI) -exec tea issues comment "$CODEV_ISSUE_ID" "$CODEV_COMMENT_BODY" +# Input: CODEV_ISSUE_ID, CODEV_COMMENT_BODY +# Output: exit code only +# +# `tea issues` has no `comment` subcommand (its subcommands are list/create/ +# edit/close). `tea comments add` only exists on tea 0.14.2+ and fails with +# "No help topic for comments" on the still-current 0.14.1 release. The +# top-level `tea comment` shorthand works on both 0.14.1 and 0.14.2+. +exec tea comment "$CODEV_ISSUE_ID" "$CODEV_COMMENT_BODY" diff --git a/packages/codev/scripts/forge/gitea/issue-view.sh b/packages/codev/scripts/forge/gitea/issue-view.sh index db53b03e4..9c3b3ddda 100755 --- a/packages/codev/scripts/forge/gitea/issue-view.sh +++ b/packages/codev/scripts/forge/gitea/issue-view.sh @@ -1,6 +1,82 @@ #!/bin/sh # Forge concept: issue-view (Gitea via tea CLI) -# Sets `url` to the issue's browser page (`html_url`). Gitea's own `url` field is -# the API endpoint (would render raw JSON in a browser), so we prefer `html_url` -# and fall back to the existing `url` only if `html_url` is absent. -tea issues view "$CODEV_ISSUE_ID" --output json | jq '.url = (.html_url // .url)' +# forge-executable: tea +# Input: CODEV_ISSUE_ID +# Output: JSON {title, body, state, url, comments[]} (IssueViewResult) +# +# `tea issues view N --output json` returns a flattened single-element list +# (no body/html_url/url), so route through the raw REST passthrough. `tea api` +# needs an explicit owner/repo in the path (unlike `tea issues`, which +# auto-detects it from the local git remote), so resolve it here: honor +# CODEV_REPO when set, else derive owner/repo from origin's URL (handles +# https, ssh, and scp-style remotes, with or without a .git suffix). +# +# `url` is mapped to the issue's browser page (`html_url`); Gitea's own `url` +# is the API endpoint (would render raw JSON in a browser), so we fall back to +# it only if `html_url` is absent. +# +# Gitea's issue object reports `comments` as an integer count, not the array +# the contract requires (consumers call `.comments.filter(...)`), so the +# comments array is fetched separately and merged in. A failed comments fetch +# degrades to [], but warns on stderr so the degraded path is distinguishable +# from a genuinely uncommented issue (stdout stays pure JSON — it's parsed by +# forge.ts). `tea api` exits 0 on HTTP errors and prints the error BODY, so the +# degrade check tests for an actual JSON array of objects rather than only for a +# blank response — an error OBJECT reached `--argjson` and blew up with a raw jq +# parse/iteration error instead of the warned [] degrade, and a non-object +# element would do the same on `.body`. +# +# SHAPE VALIDATION. Same exit-0-on-error problem for the issue itself: an error +# body normalized into an all-null IssueViewResult whose `url` was the error +# body's own `url`. Required fields are type-checked before normalizing and +# anything else is a hard failure carrying the server's message on stderr. +. "$(dirname "$0")/_lib.sh" +REPO="$(gitea_repo)" || exit 1 +# Fetch and validate the issue BEFORE its comments, so a bad issue id reports +# only its own error instead of preceding it with a comments-degrade warning +# about an issue that doesn't exist — and doesn't spend a request on it. +# Capture before piping: POSIX sh has no pipefail, so `tea api … | jq` would +# report jq's exit status rather than a failed fetch. +ISSUE="$(tea api "repos/${REPO}/issues/${CODEV_ISSUE_ID}")" || exit 1 +# jq given empty input emits nothing and exits 0, so an empty body at exit 0 +# would slip past the validator below rather than failing. +if [ -z "$ISSUE" ]; then + echo "gitea forge: empty \`tea api\` response for issue ${CODEV_ISSUE_ID}" >&2 + exit 1 +fi +printf '%s' "$ISSUE" | jq -e ' + if (type == "object") + and ((.title | type) == "string") + and ((.state | type) == "string") + and (((.html_url // .url) | type) == "string") + and ((.number | type) == "number") + then . + else + ("gitea forge: unexpected `tea api` response for issue " + + (env.CODEV_ISSUE_ID // "?") + ": " + + ((.message // .) | tostring) + + "\n") | halt_error(1) + end' >/dev/null || exit 1 + +COMMENTS_JSON="$(tea api "repos/${REPO}/issues/${CODEV_ISSUE_ID}/comments" 2>/dev/null)" +if [ -z "$COMMENTS_JSON" ] \ + || ! printf '%s' "$COMMENTS_JSON" \ + | jq -e 'type == "array" and all(.[]; type == "object")' >/dev/null 2>&1; then + echo "gitea forge: comments fetch failed for issue ${CODEV_ISSUE_ID}; reporting 0 comments" >&2 + COMMENTS_JSON="[]" +fi + +printf '%s' "$ISSUE" | jq --argjson comments "$COMMENTS_JSON" '{ + title, + body: (.body // ""), + state, + url: (.html_url // .url), + # The array itself is validated above; its ELEMENTS are whatever the + # server sent, so each field is defaulted to the type the contract + # declares rather than emitting nulls into IssueViewResult.comments. + comments: [ $comments[] | { + body: (if (.body | type) == "string" then .body else "" end), + createdAt: (if (.created_at | type) == "string" then .created_at else "" end), + author: {login: (if (.user.login | type) == "string" then .user.login else "" end)} + } ] + }' diff --git a/packages/codev/scripts/forge/gitea/pr-exists.sh b/packages/codev/scripts/forge/gitea/pr-exists.sh index db8365012..80178520d 100755 --- a/packages/codev/scripts/forge/gitea/pr-exists.sh +++ b/packages/codev/scripts/forge/gitea/pr-exists.sh @@ -1,6 +1,34 @@ #!/bin/sh # Forge concept: pr-exists (Gitea via tea CLI) -# Returns true for open or merged pulls only. Closed-not-merged pulls are excluded. -# --state all fetches pulls in all states; without it, only open pulls are returned. -# Gitea: merged PRs have state="closed" + merged=true; abandoned PRs have state="closed" + merged=false -tea pulls list --state all --fields index --output json | jq "[.[] | select(.head.ref == \"$CODEV_BRANCH_NAME\" and (.state == \"open\" or (.state == \"closed\" and .merged == true)))] | length > 0" +# forge-executable: tea +# Input: CODEV_BRANCH_NAME +# Output: "true" or "false" +# +# Returns true for OPEN or MERGED pulls only; closed-not-merged pulls are +# excluded. `tea pulls list` emits `.head` as a string (not `{ref}`) and reports +# merged PRs as state "merged" with no `.merged` boolean, so its output can't +# satisfy the `.head.ref` / `.merged` predicate below. Route through the raw +# REST passthrough, whose PR objects carry nested `.head.ref` and a `.merged` +# bool. `tea api` needs an explicit owner/repo in the path (unlike `tea pulls`, +# which auto-detects it from the local git remote), so resolve it here: honor +# CODEV_REPO when set, else derive owner/repo from origin's URL (handles https, +# ssh, and scp-style remotes, with or without a .git suffix). +# +# Caveat (Gitea behavior, not a codev bug): for a merged PR whose source branch +# was deleted, Gitea returns `.head.ref == "refs/pull/N/head"` instead of the +# original branch name, so a branch-name match won't hit a merged+deleted +# branch. That doesn't affect the "does an open/merged PR exist for the branch +# I'm about to push" use case. +# +# `state=all` is paginated (Gitea caps a page at max_response_items, default 50) +# so a branch whose PR isn't in the most recent ~50 would false-negative and +# block a porch pr_exists gate — tea_api_paged walks every page (see _lib.sh). +. "$(dirname "$0")/_lib.sh" +REPO="$(gitea_repo)" || exit 1 +# Capture the paginator's output before piping to jq: in POSIX sh (no +# pipefail), a `cmd | jq` pipeline reports jq's exit status (0) even when +# `cmd` failed mid-walk, which would surface a real error as a false-negative +# "no PR exists" and silently pass a porch pr_exists gate. +PULLS="$(tea_api_paged "repos/${REPO}/pulls" "state=all")" || exit 1 +printf '%s' "$PULLS" | jq --arg branch "$CODEV_BRANCH_NAME" \ + '[.[] | select(.head.ref == $branch and (.state == "open" or .merged == true))] | length > 0' diff --git a/packages/codev/scripts/forge/gitea/pr-list.sh b/packages/codev/scripts/forge/gitea/pr-list.sh index d320c5cc2..23e293ee7 100755 --- a/packages/codev/scripts/forge/gitea/pr-list.sh +++ b/packages/codev/scripts/forge/gitea/pr-list.sh @@ -1,36 +1,45 @@ #!/bin/sh -# Forge concept: pr-list (Gitea via tea CLI) +# Forge concept: pr-list (Gitea via tea CLI) — open pulls +# forge-executable: tea +# Output: JSON [{number, title, url, reviewDecision, body, createdAt, author, +# reviewRequests, isDraft}] (PrListItem in forge-contracts.ts) # -# Normalize tea's PR shape to the GitHub-compatible shape codev expects -# (see PrListItem in codev/src/lib/forge-contracts.ts): -# index -> number (int) -# description -> body -# created -> createdAt -# author (string) -> author.login -# reviewDecision -> "" (Gitea has no GitHub-equivalent review-decision summary) -# reviewRequests -> [] (verified against tea 0.14.1: `pulls list` exposes -# no `reviewers` field, and its JSON output is limited -# to the selectable `--fields`, so requested reviewers -# are unreachable here. The VSCode sort silently skips -# the review-requested bucket when empty.) -# isDraft -> false (verified: tea 0.14.1 `pulls list` exposes no -# `draft` field among its selectable `--fields`.) -# The underlying Gitea API PR object does carry `draft` and `requested_reviewers`, -# but only the raw `tea api` passthrough can reach them — populating these two -# fields for Gitea would mean reworking this concept onto `tea api`, which is a -# separate, larger change than #787's scope. -exec tea pulls list --limit 200 \ - --fields index,title,state,author,url,created,description \ - --output json \ - | jq '[.[] | { - number: (.index | tonumber), +# `tea pulls list --fields …,description` errors ("invalid field 'description'") +# and its flattened output can't carry a PR body, draft flag, or requested +# reviewers. Route through the raw REST passthrough instead, whose PR objects +# expose all of them. `tea api` needs an explicit owner/repo in the path (unlike +# `tea pulls`, which auto-detects it from the local git remote), so resolve it +# here: honor CODEV_REPO when set, else derive owner/repo from origin's URL +# (handles https, ssh, and scp-style remotes, with or without a .git suffix). +# +# Field mapping: +# .number -> number (already an int in the REST shape) +# .html_url -> url (browser page; Gitea `.url` is the API endpoint) +# .body -> body +# .created_at -> createdAt +# .user.login -> author.login +# .requested_reviewers[].login -> reviewRequests (user logins; teams have no login → dropped) +# .draft -> isDraft +# reviewDecision -> "" (Gitea has no GitHub-equivalent review-decision summary) +# +# The open-pulls list is paginated (Gitea caps a page at max_response_items, +# default 50), so tea_api_paged walks every page rather than silently truncating +# at ~50 open PRs (see _lib.sh). +. "$(dirname "$0")/_lib.sh" +REPO="$(gitea_repo)" || exit 1 +# Capture the paginator's output before piping to jq: in POSIX sh (no +# pipefail), a `cmd | jq` pipeline reports jq's exit status (0) even when +# `cmd` failed mid-walk, which would silently truncate the list instead of +# surfacing the error. +PULLS="$(tea_api_paged "repos/${REPO}/pulls" "state=open")" || exit 1 +printf '%s' "$PULLS" | jq '[.[] | { + number, title, - state, - url, + url: (.html_url // .url), reviewDecision: "", - body: (.description // ""), - createdAt: .created, - author: {login: .author}, - reviewRequests: [], - isDraft: false + body: (.body // ""), + createdAt: .created_at, + author: {login: .user.login}, + reviewRequests: [ (.requested_reviewers // [])[] | .login // empty ], + isDraft: (.draft // false) }]' diff --git a/packages/codev/scripts/forge/gitea/pr-view.sh b/packages/codev/scripts/forge/gitea/pr-view.sh index ffcc63acf..d4ef6dcf4 100755 --- a/packages/codev/scripts/forge/gitea/pr-view.sh +++ b/packages/codev/scripts/forge/gitea/pr-view.sh @@ -1,6 +1,67 @@ #!/bin/sh # Forge concept: pr-view (Gitea via tea CLI) -# Sets `url` to the PR's browser page (`html_url`). Gitea's own `url` field is -# the API endpoint (would render raw JSON in a browser), so we prefer `html_url` -# and fall back to the existing `url` only if `html_url` is absent. -tea pulls view "$CODEV_PR_NUMBER" --output json | jq '.url = (.html_url // .url)' +# forge-executable: tea +# Input: CODEV_PR_NUMBER +# Output: JSON {title, body, state, url, author{login}, baseRefName, headRefName, +# additions, deletions} (see PrViewResult in forge-contracts.ts) +# +# `tea pulls view N --output json` returns a table header / empty list rather +# than the PR object, so route through the raw REST passthrough. `tea api` +# needs an explicit owner/repo in the path (unlike `tea pulls`, which +# auto-detects it from the local git remote), so resolve it here: honor +# CODEV_REPO when set, else derive owner/repo from origin's URL (handles +# https, ssh, and scp-style remotes, with or without a .git suffix). +# +# `url` is the PR's browser page (`html_url`). Gitea's own `url` field is the +# API endpoint (would render raw JSON in a browser), so map `html_url` and fall +# back to `url` only if it's absent — the same choice PIR #1179 made when this +# concept still went through `tea pulls view`. +# +# SHAPE VALIDATION. `tea api` exits 0 on an HTTP error and prints the error +# BODY, e.g. {"message":"pull does not exist [index: 42]","url":"…/swagger"}. +# Normalizing that unchecked produced a structurally valid, entirely wrong +# contract object at exit 0 — every field null except `url`, which took the +# error body's own `url` (the swagger link) and shipped it to callers as the +# PR's browser page. Required fields are type-checked first and anything else +# is a hard failure carrying the server's message on stderr. +. "$(dirname "$0")/_lib.sh" +REPO="$(gitea_repo)" || exit 1 +# Capture before piping: in POSIX sh (no pipefail) a `tea api … | jq` pipeline +# reports jq's exit status, so a failed fetch would surface as jq's exit-0 on +# empty stdin rather than an error. +PR="$(tea api "repos/${REPO}/pulls/${CODEV_PR_NUMBER}")" || exit 1 +# An empty body at exit 0 would give jq nothing to work on: it emits no output +# and exits 0, so the script would "succeed" with empty stdout instead of +# failing. The validator below never runs on an empty response, so guard here. +if [ -z "$PR" ]; then + echo "gitea forge: empty \`tea api\` response for pull ${CODEV_PR_NUMBER}" >&2 + exit 1 +fi +printf '%s' "$PR" | jq ' + if (type == "object") + and ((.number | type) == "number") + and ((.title | type) == "string") + and ((.state | type) == "string") + and ((.user.login | type) == "string") + and ((.base.ref | type) == "string") + and ((.head.ref | type) == "string") + then . + else + ("gitea forge: unexpected `tea api` response for pull " + + (env.CODEV_PR_NUMBER // "?") + ": " + + ((.message // .) | tostring) + + "\n") | halt_error(1) + end + | { + title, + body: (.body // ""), + state, + url: (.html_url // .url), + author: {login: .user.login}, + baseRefName: .base.ref, + headRefName: .head.ref, + # Type-checked, not just defaulted: the contract types these as numbers, and + # a non-numeric value here would be passed straight through. + additions: (if (.additions | type) == "number" then .additions else 0 end), + deletions: (if (.deletions | type) == "number" then .deletions else 0 end) +}' diff --git a/packages/codev/scripts/forge/gitea/recently-merged.sh b/packages/codev/scripts/forge/gitea/recently-merged.sh index 4c2d30955..68dd037e1 100755 --- a/packages/codev/scripts/forge/gitea/recently-merged.sh +++ b/packages/codev/scripts/forge/gitea/recently-merged.sh @@ -1,27 +1,97 @@ #!/bin/sh # Forge concept: recently-merged (Gitea via tea CLI) +# forge-executable: tea +# Input: CODEV_SINCE_DATE (optional, ISO-8601 timestamp) +# Output: JSON [{number, title, url, body, createdAt, mergedAt, headRefName}] +# (MergedPrItem in forge-contracts.ts) # -# `tea pulls list --state closed` returns both merged PRs and closed-without- -# merge PRs. Filter to merged only via `.merged == true` (the same predicate -# scripts/forge/gitea/pr-exists.sh already relies on), then map to the -# GitHub-compatible shape: -# index -> number (int) -# created -> createdAt -# updated -> mergedAt (tea exposes no merged_at field via --fields; -# close-then-edit overestimates merged time -# but is acceptable for the 24h overview window) -# head.ref -> headRefName -# description -> body -exec tea pulls list --state closed --limit 1000 \ - --fields index,title,state,author,url,created,updated,head,description,merged \ - --output json \ - | jq '[.[] | select(.merged == true) | { - number: (.index | tonumber), +# `tea pulls list --fields …,head,description,merged` errors on the `description` +# field and emits `.head` as a string, so it can't populate `body` or +# `.head.ref`. Route through the raw REST passthrough instead, whose closed +# pulls carry `.merged`, `.merged_at`, nested `.head.ref`, and `.body`. Keep +# only merged pulls (closed-without-merge have `.merged == false`). `tea api` +# needs an explicit owner/repo in the path (unlike `tea pulls`, which +# auto-detects it from the local git remote), so resolve it here: honor +# CODEV_REPO when set, else derive owner/repo from origin's URL (handles https, +# ssh, and scp-style remotes, with or without a .git suffix). +# +# The closed-pulls list is paginated (Gitea caps a page at max_response_items, +# default 50), so on a busy repo the most-recent merges could push older ones +# past the first page — tea_api_paged walks every page (see _lib.sh). +# +# CODEV_SINCE_DATE BOUNDS THE WALK. This concept feeds a 24h analytics window, +# but the closed-pulls list is the repo's whole merge history: on an +# established repo an unbounded walk issues up to GITEA_MAX_PAGES sequential +# requests inside forge's 30s timeout, and a timeout yields `null` — a worse +# outcome for the dashboard than truncation was. When CODEV_SINCE_DATE is set +# we ask the server for update-time-descending order and stop at the first page +# that reaches back past the cutoff. +# +# The stop filter does not take the sort on trust. What it actually needs is +# the ORDERING, not the parameter, so it checks for the ordering directly and +# fires only when all of this holds: +# - the current page is non-increasing in `updated_at`, +# - the PREVIOUS page was too, and its oldest entry is no older than this +# page's newest — i.e. the order survives a page boundary, so page-local +# sorting or a coincidentally-descending first page isn't enough, +# - some entry on this page predates the cutoff. +# Never on page 1: with nothing to compare against, one internally-descending +# page proves nothing about the pages behind it. Costing one extra request is +# the right trade against dropping a merge. +# +# Given that ordering, `updated_at >= merged_at` (a merge updates the PR) means +# nothing merged after the cutoff can sit beyond the first page whose update +# times have fallen behind it. A server that ignores `sort=recentupdate` fails +# these checks and we fall back to the full walk — slower, never wrong. +# +# Timestamps go through `gitea_epoch` because Gitea emits RFC3339 in the +# server's timezone, not necessarily `Z`. +. "$(dirname "$0")/_lib.sh" +REPO="$(gitea_repo)" || exit 1 + +if [ -n "$CODEV_SINCE_DATE" ]; then + QUERY="state=closed&sort=recentupdate" + # `$prev` is the previous page, bound by tea_api_paged (null on page 1). + STOP="${GITEA_JQ_LIB}"' + def descending: . as $a | [ range(($a | length) - 1) | $a[.] >= $a[. + 1] ] | all; + def times: [ .[] | (.updated_at | gitea_epoch) ]; + + times as $t + | ($prev | if . == null then null else times end) as $p + | (env.CODEV_SINCE_DATE | gitea_epoch) as $since + | ($since != null) + and ($p != null) + and (($t | length) > 0) and (($p | length) > 0) + and ([ $t[] | . != null ] | all) and ([ $p[] | . != null ] | all) + and ($t | descending) and ($p | descending) + and (($p | min) >= ($t | max)) + and (($t | min) < $since) + ' +else + QUERY="state=closed" + STOP="" +fi + +# Capture the paginator's output before piping to jq: in POSIX sh (no +# pipefail), a `cmd | jq` pipeline reports jq's exit status (0) even when +# `cmd` failed mid-walk, which would silently truncate the list instead of +# surfacing the error. +PULLS="$(tea_api_paged "repos/${REPO}/pulls" "$QUERY" "$STOP")" || exit 1 +printf '%s' "$PULLS" | jq "${GITEA_JQ_LIB}"' + (env.CODEV_SINCE_DATE | gitea_epoch) as $since + | [ .[] + | select(.merged == true) + # Drop merges older than the cutoff. An unparseable/absent timestamp on + # either side keeps the item — the caller filters the window again, so + # over-reporting is harmless where dropping a real merge is not. + | select($since == null + or ((.merged_at | gitea_epoch) as $m | $m == null or $m >= $since)) + | { + number, title, - state, - url, - body: (.description // ""), - createdAt: .created, - mergedAt: .updated, + url: (.html_url // .url), + body: (.body // ""), + createdAt: .created_at, + mergedAt: .merged_at, headRefName: (.head.ref // "") }]' diff --git a/packages/codev/scripts/forge/gitea/user-identity.sh b/packages/codev/scripts/forge/gitea/user-identity.sh index 2b8f78523..55c9b302a 100755 --- a/packages/codev/scripts/forge/gitea/user-identity.sh +++ b/packages/codev/scripts/forge/gitea/user-identity.sh @@ -1,3 +1,32 @@ #!/bin/sh # Forge concept: user-identity (Gitea via tea CLI) -tea whoami --output json | jq -r ".login" +# forge-executable: tea +# Output: plain text username +# +# `tea whoami` has no `--output json` flag (its only documented option is +# --help), so it can't feed a jq pipeline. Route through the raw REST +# passthrough instead: `tea api user` returns the Gitea `User` object, whose +# `.login` is the authenticated username (mirrors `gh api user --jq .login`). +# +# SHAPE VALIDATION. `tea api` exits 0 on an HTTP error and prints the error +# BODY, which has no `.login` — `jq -r .login` then printed the literal string +# "null" at exit 0, and callers took that as the current user's handle. Check +# for a non-empty string login first and fail with the server's message +# otherwise. The response is captured before the pipe because POSIX sh has no +# pipefail: `tea api user | jq` would report jq's status, not tea's. +USER_JSON="$(tea api user)" || exit 1 +# jq given empty input emits nothing and exits 0, so an empty body at exit 0 +# would leave the script "succeeding" with no username at all. +if [ -z "$USER_JSON" ]; then + echo "gitea forge: empty \`tea api user\` response" >&2 + exit 1 +fi +printf '%s' "$USER_JSON" | jq -r ' + if (type == "object") and ((.login | type) == "string") + and ((.login | test("^\\s*$")) | not) + then .login + else + ("gitea forge: unexpected `tea api user` response: " + + ((.message // .) | tostring) + + "\n") | halt_error(1) + end' diff --git a/packages/codev/src/__tests__/bugfix-1137-gitea-tea-api.test.ts b/packages/codev/src/__tests__/bugfix-1137-gitea-tea-api.test.ts new file mode 100644 index 000000000..3de4a6aee --- /dev/null +++ b/packages/codev/src/__tests__/bugfix-1137-gitea-tea-api.test.ts @@ -0,0 +1,857 @@ +/** + * Regression test for bugfix #1137: the gitea forge preset was written against + * the Gitea REST API JSON shape but invoked the `tea` CLI's flattened + * ` list/view` output (or non-existent flags/subcommands), so every + * read concept either errored or emitted the wrong shape. + * + * The fix routes the read concepts through `tea api `, whose raw + * passthrough returns exactly the Gitea REST shape the jq normalizers and + * `forge-contracts.ts` already assume. + * + * PR #1146 review follow-up: + * - list reads (`pr-exists`, `pr-list`, `recently-merged`) now PAGINATE via + * the shared `tea_api_paged` helper (Gitea caps a page at max_response_items, + * default 50, so `&limit=200` silently truncated). The fake `tea` below + * serves a full 50-item page 1 + a short page 2 and the tests assert an item + * that only exists on page 2 is found. + * - the `owner/repo` derivation is factored into `_lib.sh#gitea_repo` and fails + * fast (stderr + non-zero exit) when there's no usable origin remote. + * - `issue-view` warns on stderr when the comments fetch degrades to []. + * + * `tea` isn't available in CI (see the in-repo #920 note), so this test stubs a + * fake `tea` on PATH that answers `api ` (and `comment`) with + * captured Gitea REST fixtures, points the scripts at a throwaway git repo with + * a gitea remote, runs each real script, and asserts the normalized output + * conforms to the contract in forge-contracts.ts. + * + * Integration-review follow-up (2026-08-17, amrmelsayed): + * - `issue-comment` now calls the top-level `tea comment` shorthand rather + * than `tea comments add`, which only exists on tea 0.14.2+ and fails on + * the still-current 0.14.1 release. + * - `pr-exists`/`pr-list`/`recently-merged` now capture `tea_api_paged`'s + * output before piping to jq, so a mid-walk failure exits non-zero instead + * of surfacing as jq's exit-0-on-empty-stdin. + * - `tea_api_paged`'s stop condition now compares against the page size + * actually observed on page 1, not the requested limit, so a server whose + * `max_response_items` is tuned below the requested limit doesn't stop + * after page 1 while more pages remain. + * + * Maintainer review follow-up (2026-09-03, waleedkadous): + * - the five scripts that source `_lib.sh` (plus `user-identity`, whose fix + * below moves `tea` off the first substantive line) declare + * `# forge-executable: tea` so `codev doctor` reports the real CLI instead + * of `.` / `tea_api_paged` / `printf`. + * - `tea_api_paged` FAILS at the `GITEA_MAX_PAGES` ceiling instead of + * returning a partial array at exit 0. + * - `pr-view`, `user-identity` and `issue-view` validate the response shape + * before normalizing: `tea api` exits 0 on HTTP errors and prints the error + * body, which used to become an all-null contract object (leaking the error + * body's own `url` as the browser page) or the literal username `null`. + * - `recently-merged` honors `CODEV_SINCE_DATE`, bounding a walk that would + * otherwise cover the repo's entire merge history inside forge's 30s + * timeout. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { execFileSync, spawnSync } from 'node:child_process'; + +const giteaDir = resolve(__dirname, '..', '..', 'scripts', 'forge', 'gitea'); + +// A fake `tea` binary. It only implements `api ` (the surface the +// fixed scripts use) plus `comments add`. Each endpoint returns the raw Gitea +// REST shape — nested objects, real `.merged`/`.merged_at`/`.draft`, integer +// `comments` count on the issue object, etc. +// +// The paginated list endpoints (page=1 full at limit 50, page=2 short) prove the +// scripts walk past the server's page cap: each carries a "signature" item plus +// filler, and a distinct item that lives ONLY on page 2. +const FAKE_TEA = `#!/bin/sh +if [ "$1" = "comment" ]; then + # comment (the tea 0.14.1-compatible shorthand for + # \`tea comments add\`, which only exists on 0.14.2+) + echo "commented" + exit 0 +fi +[ "$1" = "api" ] || { echo "fake-tea: unsupported: $*" >&2; exit 3; } +case "$2" in + user) + # FAKE_TEA_USER_ERROR reproduces \`tea api\`'s exit-0-on-HTTP-error: an + # error body on stdout with no \`.login\`, at exit status 0. + if [ -n "$FAKE_TEA_USER_ERROR" ]; then + echo '{"message":"token does not exist","url":"https://git.example.com/api/swagger"}' + elif [ -n "$FAKE_TEA_USER_EMPTY" ]; then + # exit 0, nothing on stdout — jq would emit nothing and exit 0 too. + : + elif [ -n "$FAKE_TEA_USER_BLANK" ]; then + echo '{"login":" ","id":7}' + else + echo '{"login":"octo","id":7}' + fi ;; + repos/acme/widgets/pulls/42) + echo '{"number":42,"title":"Add widget","body":"PR body","state":"open","html_url":"https://git.example.com/acme/widgets/pulls/42","url":"https://git.example.com/api/v1/repos/acme/widgets/pulls/42","user":{"login":"alice"},"base":{"ref":"main"},"head":{"ref":"feature/x"},"additions":10,"deletions":3}' ;; + + # --- mid-walk pagination failure: page 1 is a full 50 items (so the + # paginator commits to a page 2), page 2 errors. Used to prove pr-exists/ + # pr-list/recently-merged exit non-zero instead of silently succeeding with + # a truncated/empty result (the jq-exit-status-masks-a-failed-pipe bug). --- + "repos/acme/failing/pulls?state=all&limit=50&page=1") + jq -cn '[range(50)|{number:(3000+.),state:"open",merged:false,head:{ref:("pad-"+(.|tostring))}}]' ;; + "repos/acme/failing/pulls?state=all&limit=50&page=2") + echo "fake-tea: page 2 unavailable" >&2; exit 9 ;; + "repos/acme/failing/pulls?state=open&limit=50&page=1") + jq -cn '[range(50)|{number:(3000+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + "repos/acme/failing/pulls?state=open&limit=50&page=2") + echo "fake-tea: page 2 unavailable" >&2; exit 9 ;; + "repos/acme/failing/pulls?state=closed&limit=50&page=1") + jq -cn '[range(50)|{number:(3000+.),title:"pad",state:"closed",merged:false,head:{ref:"pad"}}]' ;; + "repos/acme/failing/pulls?state=closed&limit=50&page=2") + echo "fake-tea: page 2 unavailable" >&2; exit 9 ;; + + # --- sub-limit server cap: max_response_items tuned to 30 (below the + # requested limit of 50), so EVERY page — including the last — is capped at + # 30. Three pages: 30 + 30 + 6 (66 total). Proves the paginator keeps + # walking past a full-but-capped page instead of stopping after page 1 + # because 30 < the requested 50. --- + "repos/acme/capped/pulls?state=open&limit=50&page=1") + jq -cn '[range(30)|{number:(4000+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + "repos/acme/capped/pulls?state=open&limit=50&page=2") + jq -cn '[range(30)|{number:(4100+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + "repos/acme/capped/pulls?state=open&limit=50&page=3") + jq -cn '[range(5)|{number:(4200+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}] + [{number:4299,title:"Last capped page item",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + + # --- pr-exists: state=all, paginated ------------------------------------- + # page 1 = 50 items (open feature/x, merged feature/done, closed-not-merged + # feature/abandoned, + 47 open pad). page 2 = 1 merged item on feature/deep. + "repos/acme/widgets/pulls?state=all&limit=50&page=1") + jq -cn '[{number:42,state:"open",merged:false,head:{ref:"feature/x"}},{number:40,state:"closed",merged:true,head:{ref:"feature/done"}},{number:39,state:"closed",merged:false,head:{ref:"feature/abandoned"}}] + [range(47)|{number:(1000+.),state:"open",merged:false,head:{ref:("pad-"+(.|tostring))}}]' ;; + "repos/acme/widgets/pulls?state=all&limit=50&page=2") + echo '[{"number":900,"state":"closed","merged":true,"head":{"ref":"feature/deep"}}]' ;; + + # --- pr-list: state=open, paginated -------------------------------------- + # page 1 = the rich #42 item + 49 pad (50 total). page 2 = 1 item (#900). + "repos/acme/widgets/pulls?state=open&limit=50&page=1") + jq -cn '[{number:42,title:"Add widget",html_url:"https://git.example.com/acme/widgets/pulls/42",url:"https://git.example.com/api/v1/repos/acme/widgets/pulls/42",body:"PR body",state:"open",created_at:"2026-07-01T10:00:00Z",user:{login:"alice"},requested_reviewers:[{login:"bob"},{login:null}],draft:true}] + [range(49)|{number:(1000+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + "repos/acme/widgets/pulls?state=open&limit=50&page=2") + echo '[{"number":900,"title":"Deep open PR","html_url":"https://git.example.com/acme/widgets/pulls/900","body":"deep","state":"open","created_at":"2026-07-01T11:00:00Z","user":{"login":"erin"},"requested_reviewers":[],"draft":false}]' ;; + + # --- recently-merged: state=closed, paginated ---------------------------- + # page 1 = 50 items, only #40 merged (the rest merged:false pad). page 2 = 1 + # merged item (#901) — so a merged PR beyond page 1 must still surface. + "repos/acme/widgets/pulls?state=closed&limit=50&page=1") + jq -cn '[{number:40,title:"Done PR",html_url:"https://git.example.com/acme/widgets/pulls/40",body:"merged body",state:"closed",merged:true,merged_at:"2026-07-05T12:00:00Z",created_at:"2026-07-02T09:00:00Z",head:{ref:"feature/done"}},{number:39,title:"Abandoned",state:"closed",merged:false,head:{ref:"feature/abandoned"}}] + [range(48)|{number:(2000+.),title:"pad",state:"closed",merged:false,head:{ref:"pad"}}]' ;; + "repos/acme/widgets/pulls?state=closed&limit=50&page=2") + echo '[{"number":901,"title":"Deep merge","html_url":"https://git.example.com/acme/widgets/pulls/901","body":"deep merged","state":"closed","merged":true,"merged_at":"2026-07-06T12:00:00Z","created_at":"2026-07-03T09:00:00Z","head":{"ref":"feature/deep-merge"}}]' ;; + + # --- issue-view ---------------------------------------------------------- + repos/acme/widgets/issues/99) + echo '{"number":99,"title":"Bug here","body":"issue body","state":"open","html_url":"https://git.example.com/acme/widgets/issues/99","url":"https://git.example.com/api/v1/repos/acme/widgets/issues/99","comments":2}' ;; + repos/acme/widgets/issues/99/comments) + echo '[{"body":"On it! Working on a fix now.","created_at":"2026-07-06T08:00:00Z","user":{"login":"carol"}},{"body":"second","created_at":"2026-07-06T09:00:00Z","user":{"login":"dave"}}]' ;; + # issue 98: the issue object fetches fine but its comments endpoint fails, + # exercising the degraded (stderr-warned) []-comments path. + repos/acme/widgets/issues/98) + echo '{"number":98,"title":"No comments reachable","body":"body","state":"open","html_url":"https://git.example.com/acme/widgets/issues/98","comments":5}' ;; + repos/acme/widgets/issues/98/comments) + echo "fake-tea: comments endpoint down" >&2; exit 7 ;; + + # --- error bodies at exit 0 ------------------------------------------- + # \`tea api\` exits 0 on an HTTP error and prints the error body. These + # fixtures reproduce that exactly: exit status 0, an error OBJECT on stdout. + # Note the \`url\` key — Gitea's error bodies carry one (the swagger link), + # which an unvalidated normalizer would ship as the PR/issue browser page. + repos/acme/widgets/pulls/404) + echo '{"message":"pull request does not exist [id: 0, index: 404]","url":"https://git.example.com/api/swagger"}' ;; + repos/acme/widgets/issues/404) + echo '{"message":"issue does not exist [id: 0, index: 404]","url":"https://git.example.com/api/swagger"}' ;; + # issue 97: the issue itself is fine, but its comments endpoint answers with + # an error OBJECT at exit 0 (not a blank body and not a failure), which used + # to reach \`jq --argjson\` and blow up with a raw iteration error. + repos/acme/widgets/issues/97) + echo '{"number":97,"title":"Comments error body","body":"body","state":"open","html_url":"https://git.example.com/acme/widgets/issues/97","comments":3}' ;; + repos/acme/widgets/issues/97/comments) + echo '{"message":"token does not have at least one of required scope(s): [read:issue]","url":"https://git.example.com/api/swagger"}' ;; + + # --- endless pagination: every page is a full 50 items, forever, so no + # terminal short/empty page is ever reached and the GITEA_MAX_PAGES ceiling + # fires. Proves the paginator errors rather than returning a partial array. --- + # (5 items/page, not 50: the paginator's short-page check compares against + # the size observed on page 1, so a uniform page size of any value is never + # "short" — this just keeps a 100-page walk cheap in CI.) + repos/acme/endless/pulls*) + jq -cn '[range(5)|{number:(5000+.),title:"pad",html_url:"u",body:"",state:"open",merged:false,created_at:"d",updated_at:"2026-07-09T00:00:00Z",user:{login:"pad"},requested_reviewers:[],draft:false,head:{ref:"pad"}}]' ;; + + # --- recently-merged bounded by CODEV_SINCE_DATE ------------------------ + # Cutoff for these fixtures is 2026-07-05T00:00:00Z. + # Page 1: 50 items, strictly descending by updated_at, ALL after the cutoff + # (2026-07-09T00:00 down to 2026-07-06T20:00) — two of them merged, the + # second carrying a +02:00 offset rather than \`Z\`. The walk must NOT stop + # here: nothing has crossed the cutoff yet, and page 1 alone proves nothing + # about ordering anyway. + # Page 2: 50 items continuing the descent (2026-07-06T19:00 down to + # 2026-07-04T18:00) and so crossing the cutoff — one merge above it, one + # below. The order survives the page boundary, so the walk stops here. + # Page 3: ERRORS, so a clean exit proves the walk stopped at page 2. + "repos/acme/dated/pulls?state=closed&sort=recentupdate&limit=50&page=1") + jq -cn '[{number:10,title:"Recent merge",html_url:"https://git.example.com/acme/dated/pulls/10",body:"r",state:"closed",merged:true,merged_at:"2026-07-09T00:00:00Z",created_at:"2026-07-01T00:00:00Z",updated_at:"2026-07-09T00:00:00Z",head:{ref:"feature/recent"}},{number:9,title:"Offset merge",html_url:"u",body:"",state:"closed",merged:true,merged_at:"2026-07-08T22:00:00+02:00",created_at:"2026-06-01T00:00:00Z",updated_at:"2026-07-08T22:00:00+02:00",head:{ref:"feature/offset"}}] + [range(48)|(1783537200 - (. * 3600)) as $u|{number:(6000+.),title:"p1 pad",html_url:"u",body:"",state:"closed",merged:false,created_at:"2026-05-01T00:00:00Z",updated_at:($u|todateiso8601),head:{ref:"pad"}}]' ;; + "repos/acme/dated/pulls?state=closed&sort=recentupdate&limit=50&page=2") + jq -cn '[range(50)|(1783364400 - (. * 3600)) as $u|{number:(6100+.),title:"p2",html_url:"u",body:"",state:"closed",merged:(. == 0 or . == 49),merged_at:(if (. == 0 or . == 49) then ($u|todateiso8601) else null end),created_at:"2026-05-01T00:00:00Z",updated_at:($u|todateiso8601),head:{ref:"p2"}}]' ;; + "repos/acme/dated/pulls?state=closed&sort=recentupdate&limit=50&page=3") + echo "fake-tea: dated page 3 requested" >&2; exit 9 ;; + + # --- the reviewer counterexample: a server that IGNORES sort but happens to + # return an internally descending page 1 that is entirely older than the + # cutoff, with a genuinely recent merge sitting on page 2. Stopping on + # page-local ordering alone would silently drop that merge. --- + "repos/acme/lagging/pulls?state=closed&sort=recentupdate&limit=50&page=1") + jq -cn '[range(50)|(1780876800 - (. * 3600)) as $u|{number:(8000+.),title:"old",html_url:"u",body:"",state:"closed",merged:false,created_at:"2026-05-01T00:00:00Z",updated_at:($u|todateiso8601),head:{ref:"old"}}]' ;; + "repos/acme/lagging/pulls?state=closed&sort=recentupdate&limit=50&page=2") + echo '[{"number":8100,"title":"Late merge of an old PR","html_url":"https://git.example.com/acme/lagging/pulls/8100","body":"l","state":"closed","merged":true,"merged_at":"2026-07-07T00:00:00Z","created_at":"2026-01-01T00:00:00Z","updated_at":"2026-07-07T00:00:00Z","head":{"ref":"feature/late"}}]' ;; + + # --- non-array pages at exit 0: \`jq length\` is 0 for both \`{}\` and \`null\`, + # so an error body used to look exactly like an exhausted list. --- + "repos/acme/errorpage/pulls?state=open&limit=50&page=1") + jq -cn '[range(50)|{number:(9000+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + "repos/acme/errorpage/pulls?state=open&limit=50&page=2") + echo '{"message":"internal server error","url":"https://git.example.com/api/swagger"}' ;; + "repos/acme/errorpage/pulls?state=all&limit=50&page=1") + jq -cn '[range(50)|{number:(9000+.),state:"open",merged:false,head:{ref:"pad"}}]' ;; + "repos/acme/errorpage/pulls?state=all&limit=50&page=2") + echo 'null' ;; + + # --- exactly-full last page: 100 pages of 5, and page 101 is empty. The + # ceiling is reached with every page full, but nothing is actually missing — + # the list length was just an exact multiple of the page size. The probe past + # the ceiling must find that out instead of failing a complete result. + # (The page=101 pattern must precede the catch-all glob below it.) --- + repos/acme/exact/pulls*page=101) + echo '[]' ;; + repos/acme/exact/pulls*) + jq -cn '[range(5)|{number:(5500+.),title:"pad",html_url:"u",body:"",state:"open",created_at:"d",user:{login:"pad"},requested_reviewers:[],draft:false}]' ;; + + # --- heavy pages: each item carries a ~3KB body, so a 50-item page is ~150KB + # — past Linux MAX_ARG_STRLEN (128KiB), the per-argument cap that applies + # regardless of ARG_MAX. Passing a page to the stop filter through + # \`--argjson\` blows up with E2BIG there while passing fine on macOS, which + # has no per-arg cap. Same shape as acme/dated: page 1 all recent, page 2 + # crosses the cutoff, page 3 is a poison pill. --- + "repos/acme/heavy/pulls?state=closed&sort=recentupdate&limit=50&page=1") + jq -cn '[range(50)|(1783537200 - (. * 3600)) as $u|{number:(7500+.),title:"heavy",html_url:"u",body:("x" * 3000),state:"closed",merged:(. == 0),merged_at:(if . == 0 then ($u|todateiso8601) else null end),created_at:"2026-05-01T00:00:00Z",updated_at:($u|todateiso8601),head:{ref:"heavy"}}]' ;; + "repos/acme/heavy/pulls?state=closed&sort=recentupdate&limit=50&page=2") + jq -cn '[range(50)|(1783357200 - (. * 3600)) as $u|{number:(7600+.),title:"heavy",html_url:"u",body:("x" * 3000),state:"closed",merged:false,created_at:"2026-05-01T00:00:00Z",updated_at:($u|todateiso8601),head:{ref:"heavy"}}]' ;; + "repos/acme/heavy/pulls?state=closed&sort=recentupdate&limit=50&page=3") + echo "fake-tea: heavy page 3 requested" >&2; exit 9 ;; + + # --- a non-string \`.message\`, as a proxy or gateway between tea and Gitea + # can produce. String-concatenating it into the error text throws a raw jq + # error instead of the legible message these blocks exist to give. --- + repos/acme/widgets/pulls/500) + echo '{"message":{"nested":"upstream refused"},"url":"https://git.example.com/api/swagger"}' ;; + # issue 96: comments come back as an array of NON-objects, which reaches + # \`$comments[] | .body\` and dies with "Cannot index number with body". + repos/acme/widgets/issues/96) + echo '{"number":96,"title":"Scalar comments","body":"body","state":"open","html_url":"https://git.example.com/acme/widgets/issues/96","comments":2}' ;; + repos/acme/widgets/issues/96/comments) + echo '[1,2]' ;; + + # --- empty bodies at exit 0 (pull/issue 0) ------------------------------ + repos/acme/widgets/pulls/0) : ;; + repos/acme/widgets/issues/0) : ;; + + # --- server that IGNORES sort=recentupdate ------------------------------ + # Page 1 is a full 50 items in arbitrary update order that includes items + # older than the cutoff. The stop filter must NOT fire (the page isn't + # non-increasing), so the walk continues to the short page 2 and the merge + # that lives there is still reported. + "repos/acme/unsorted/pulls?state=closed&sort=recentupdate&limit=50&page=1") + jq -cn '[range(50)|{number:(7000+.),title:"mixed",html_url:"u",body:"",state:"closed",merged:false,created_at:"c",updated_at:(if (. % 2) == 0 then "2026-06-01T00:00:00Z" else "2026-07-09T00:00:00Z" end),head:{ref:"mixed"}}]' ;; + "repos/acme/unsorted/pulls?state=closed&sort=recentupdate&limit=50&page=2") + echo '[{"number":7100,"title":"Deep recent merge","html_url":"https://git.example.com/acme/unsorted/pulls/7100","body":"d","state":"closed","merged":true,"merged_at":"2026-07-07T00:00:00Z","created_at":"2026-07-01T00:00:00Z","updated_at":"2026-07-07T00:00:00Z","head":{"ref":"feature/deep-unsorted"}}]' ;; + + *) echo "fake-tea: no fixture for: $2" >&2; exit 4 ;; +esac +`; + +let fixture: string; +let binDir: string; +let repoDir: string; +let runEnv: NodeJS.ProcessEnv; + +function hasJq(): boolean { + try { + execFileSync('jq', ['--version'], { stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + +const jqAvailable = hasJq(); + +/** Run a gitea forge script under the fake `tea`, return trimmed stdout. */ +function runScript(name: string, env: Record = {}): string { + return execFileSync('sh', [join(giteaDir, name)], { + cwd: repoDir, + env: { ...runEnv, ...env }, + encoding: 'utf-8', + }).trim(); +} + +/** Run a script capturing stdout, stderr and exit status (for failure paths). */ +function runScriptFull( + name: string, + env: Record = {}, + cwd: string = repoDir, +): { status: number | null; stdout: string; stderr: string } { + const r = spawnSync('sh', [join(giteaDir, name)], { + cwd, + env: { ...runEnv, ...env }, + encoding: 'utf-8', + }); + return { status: r.status, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; +} + +describe.skipIf(!jqAvailable)('bugfix #1137: gitea preset routes reads through `tea api`', () => { + beforeAll(() => { + fixture = mkdtempSync(join(tmpdir(), 'codev-1137-')); + binDir = join(fixture, 'bin'); + repoDir = join(fixture, 'repo'); + mkdirSync(binDir, { recursive: true }); + mkdirSync(repoDir, { recursive: true }); + + const teaPath = join(binDir, 'tea'); + writeFileSync(teaPath, FAKE_TEA, { mode: 0o755 }); + chmodSync(teaPath, 0o755); + + // Throwaway repo with a scp-style gitea remote → owner/repo = acme/widgets. + execFileSync('git', ['init', '-q'], { cwd: repoDir }); + execFileSync('git', ['remote', 'add', 'origin', 'git@git.example.com:acme/widgets.git'], { cwd: repoDir }); + + // Hermetic: the scripts read CODEV_* from the environment, and several tests + // assert on the ABSENCE of one (recently-merged with no CODEV_SINCE_DATE + // must walk unbounded). Inheriting the developer's shell — or a codev + // invocation path that exports CODEV_REPO — would quietly change what is + // being tested, so every CODEV_* is stripped from the base env and each + // test supplies exactly the ones it means. + const inherited = Object.fromEntries( + Object.entries(process.env).filter(([k]) => !k.startsWith('CODEV_')), + ); + runEnv = { ...inherited, PATH: `${binDir}:${process.env.PATH ?? ''}` }; + }); + + afterAll(() => { + rmSync(fixture, { recursive: true, force: true }); + }); + + it('user-identity emits the bare login (not JSON)', () => { + expect(runScript('user-identity.sh')).toBe('octo'); + }); + + it('pr-view returns the PrViewResult shape from the PR object', () => { + const pr = JSON.parse(runScript('pr-view.sh', { CODEV_PR_NUMBER: '42' })); + expect(pr).toEqual({ + title: 'Add widget', + body: 'PR body', + state: 'open', + // PIR #1179: `url` is the browser page. The fixture carries both fields, + // so this also pins that Gitea's `url` (the API endpoint, which would + // render raw JSON) is NOT what lands in the contract. + url: 'https://git.example.com/acme/widgets/pulls/42', + author: { login: 'alice' }, + baseRefName: 'main', + headRefName: 'feature/x', + additions: 10, + deletions: 3, + }); + }); + + it('pr-list normalizes to PrListItem[] incl. real reviewRequests/isDraft/body', () => { + const list = JSON.parse(runScript('pr-list.sh')); + const first = list.find((p: { number: number }) => p.number === 42); + expect(first).toMatchObject({ + number: 42, + title: 'Add widget', + url: 'https://git.example.com/acme/widgets/pulls/42', + reviewDecision: '', + body: 'PR body', + createdAt: '2026-07-01T10:00:00Z', + author: { login: 'alice' }, + reviewRequests: ['bob'], // null-login (team) reviewers dropped + isDraft: true, + }); + expect(typeof first.number).toBe('number'); + }); + + it('pr-list paginates: a PR only on page 2 still appears (51 total)', () => { + const list = JSON.parse(runScript('pr-list.sh')); + expect(list).toHaveLength(51); // 50 (page 1) + 1 (page 2) + expect(list.some((p: { number: number }) => p.number === 900)).toBe(true); + }); + + it('pr-exists is true for an OPEN pull on the branch', () => { + expect(runScript('pr-exists.sh', { CODEV_BRANCH_NAME: 'feature/x' })).toBe('true'); + }); + + it('pr-exists is true for a MERGED pull on the branch', () => { + expect(runScript('pr-exists.sh', { CODEV_BRANCH_NAME: 'feature/done' })).toBe('true'); + }); + + it('pr-exists is false for a closed-not-merged branch', () => { + expect(runScript('pr-exists.sh', { CODEV_BRANCH_NAME: 'feature/abandoned' })).toBe('false'); + }); + + it('pr-exists is false when no PR matches the branch', () => { + expect(runScript('pr-exists.sh', { CODEV_BRANCH_NAME: 'no-such-branch' })).toBe('false'); + }); + + it('pr-exists paginates: a merged PR only on page 2 is found', () => { + // page 1 is a full 50 items; feature/deep exists ONLY on page 2, so this + // would false-negative (and block a porch pr_exists gate) without paging. + expect(runScript('pr-exists.sh', { CODEV_BRANCH_NAME: 'feature/deep' })).toBe('true'); + }); + + it('issue-view returns body, browser url, and comments as an ARRAY', () => { + const issue = JSON.parse(runScript('issue-view.sh', { CODEV_ISSUE_ID: '99' })); + expect(issue.title).toBe('Bug here'); + expect(issue.body).toBe('issue body'); + expect(issue.state).toBe('open'); + // html_url (browser page), NOT the API endpoint + expect(issue.url).toBe('https://git.example.com/acme/widgets/issues/99'); + // Contract requires an array — Gitea's issue object reports `comments` as an + // integer count, which would crash `issue.comments.filter(...)`. + expect(Array.isArray(issue.comments)).toBe(true); + expect(issue.comments).toEqual([ + { body: 'On it! Working on a fix now.', createdAt: '2026-07-06T08:00:00Z', author: { login: 'carol' } }, + { body: 'second', createdAt: '2026-07-06T09:00:00Z', author: { login: 'dave' } }, + ]); + }); + + it('issue-view degrades to [] comments AND warns on stderr when the fetch fails', () => { + // Issue 98's comments endpoint errors. stdout must stay pure JSON with an + // empty array; stderr must carry a trace so [] is distinguishable from + // "genuinely no comments". + const { status, stdout, stderr } = runScriptFull('issue-view.sh', { CODEV_ISSUE_ID: '98' }); + expect(status).toBe(0); + const issue = JSON.parse(stdout); + expect(issue.comments).toEqual([]); + expect(stderr).toContain('comments fetch failed for issue 98'); + }); + + it('recently-merged keeps merged pulls only and uses merged_at', () => { + const merged = JSON.parse(runScript('recently-merged.sh')); + const done = merged.find((p: { number: number }) => p.number === 40); + expect(done).toEqual({ + number: 40, + title: 'Done PR', + url: 'https://git.example.com/acme/widgets/pulls/40', + body: 'merged body', + createdAt: '2026-07-02T09:00:00Z', + mergedAt: '2026-07-05T12:00:00Z', + headRefName: 'feature/done', + }); + // closed-not-merged pulls are excluded. + expect(merged.some((p: { number: number }) => p.number === 39)).toBe(false); + }); + + it('recently-merged paginates: a merged PR only on page 2 is included', () => { + const merged = JSON.parse(runScript('recently-merged.sh')); + expect(merged).toHaveLength(2); // #40 (page 1) + #901 (page 2) + const deep = merged.find((p: { number: number }) => p.number === 901); + expect(deep).toMatchObject({ + number: 901, + mergedAt: '2026-07-06T12:00:00Z', + headRefName: 'feature/deep-merge', + }); + }); + + it('issue-comment uses `tea comment` (not the 0.14.2-only `tea comments add`) and exits 0', () => { + // Would exit non-zero (throwing) if it invoked the non-existent + // `tea issues comment` subcommand, or the 0.14.2+-only `tea comments add`. + expect(runScript('issue-comment.sh', { CODEV_ISSUE_ID: '99', CODEV_COMMENT_BODY: 'hi' })).toBe('commented'); + }); + + it('pr-exists exits non-zero (not "false") on a mid-walk pagination failure', () => { + // Page 1 is a full 50 items so the paginator commits to fetching page 2, + // which the fixture makes error. Without capturing tea_api_paged's output + // before the jq pipe, POSIX sh (no pipefail) reports jq's exit status (0) + // on empty stdin, which prints "false" — a silent false-negative that + // would pass a porch pr_exists gate instead of surfacing the failure. + const { status, stdout } = runScriptFull('pr-exists.sh', { + CODEV_BRANCH_NAME: 'whatever', + CODEV_REPO: 'acme/failing', + }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + }); + + it('pr-list exits non-zero on a mid-walk pagination failure', () => { + const { status, stdout } = runScriptFull('pr-list.sh', { CODEV_REPO: 'acme/failing' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + }); + + it('recently-merged exits non-zero on a mid-walk pagination failure', () => { + const { status, stdout } = runScriptFull('recently-merged.sh', { CODEV_REPO: 'acme/failing' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + }); + + it('pr-list keeps walking past a page capped below the requested limit', () => { + // Server's max_response_items is tuned to 30 (below the requested 50), so + // every page — including the last — returns exactly 30 or fewer. Stopping + // when a page is shorter than the *requested* limit (50) would break after + // page 1, even though pages 2 and 3 carry real items. + const list = JSON.parse(runScript('pr-list.sh', { CODEV_REPO: 'acme/capped' })); + expect(list).toHaveLength(66); // 30 + 30 + 6 + expect(list.some((p: { number: number }) => p.number === 4299)).toBe(true); + }); + + it('CODEV_REPO overrides the git-remote-derived owner/repo', () => { + // A repo whose remote does NOT resolve to acme/widgets still works when + // CODEV_REPO is supplied explicitly (the repo-archive-style callers). + const other = mkdtempSync(join(tmpdir(), 'codev-1137-other-')); + try { + execFileSync('git', ['init', '-q'], { cwd: other }); + execFileSync('git', ['remote', 'add', 'origin', 'https://git.example.com/someone/else.git'], { cwd: other }); + const out = execFileSync('sh', [join(giteaDir, 'pr-view.sh')], { + cwd: other, + env: { ...runEnv, CODEV_REPO: 'acme/widgets', CODEV_PR_NUMBER: '42' }, + encoding: 'utf-8', + }).trim(); + expect(JSON.parse(out).title).toBe('Add widget'); + } finally { + rmSync(other, { recursive: true, force: true }); + } + }); + + it('fails fast (non-zero + stderr naming CODEV_REPO) with no usable origin remote', () => { + const bare = mkdtempSync(join(tmpdir(), 'codev-1137-noremote-')); + try { + execFileSync('git', ['init', '-q'], { cwd: bare }); + // No origin remote at all. + const { status, stdout, stderr } = runScriptFull( + 'pr-exists.sh', + { CODEV_BRANCH_NAME: 'feature/x' }, + bare, + ); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('CODEV_REPO'); + } finally { + rmSync(bare, { recursive: true, force: true }); + } + }); + + it('fails fast with a garbage origin URL that has no owner/repo', () => { + const garbage = mkdtempSync(join(tmpdir(), 'codev-1137-garbage-')); + try { + execFileSync('git', ['init', '-q'], { cwd: garbage }); + execFileSync('git', ['remote', 'add', 'origin', 'https://example.com/'], { cwd: garbage }); + const { status, stderr } = runScriptFull( + 'issue-view.sh', + { CODEV_ISSUE_ID: '99' }, + garbage, + ); + expect(status).not.toBe(0); + expect(stderr).toContain('CODEV_REPO'); + } finally { + rmSync(garbage, { recursive: true, force: true }); + } + }); + + // --- maintainer review follow-up (2026-09-03) ---------------------------- + + it('every script that hides `tea` behind a helper declares forge-executable', () => { + // `codev doctor` infers the CLI a concept needs from the script's first + // substantive line. In these six that line is `. _lib.sh` / an assignment / + // `printf`, none of which is on PATH, so doctor reported them as missing + // tools and stopped checking for `tea`. The `# forge-executable:` header + // (#1458) declares it explicitly. + // + // This asserts the declaration is PRESENT, not that doctor reads it: + // `extractExecutable` only learns to honor the header when #1458 lands, and + // until then it still reports `.` for these five. The header is an inert + // comment in the meantime, which is why it is safe to add first. + for (const name of [ + 'pr-exists.sh', + 'pr-list.sh', + 'pr-view.sh', + 'recently-merged.sh', + 'issue-view.sh', + 'user-identity.sh', + ]) { + const src = readFileSync(join(giteaDir, name), 'utf-8'); + expect(src, name).toMatch(/^#\s*forge-executable:\s*tea$/m); + } + }); + + it('pagination fails loudly at the page ceiling instead of truncating', () => { + // Every page from this repo is a full 50 items, so no terminal short/empty + // page is ever reached. Returning the partial array at exit 0 would be the + // silent truncation the paginator exists to prevent — for `pr-exists` it + // reads as "no PR exists" and passes a porch pr_exists gate. + const { status, stdout, stderr } = runScriptFull('pr-exists.sh', { + CODEV_BRANCH_NAME: 'feature/x', + CODEV_REPO: 'acme/endless', + }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('page ceiling'); + // 100 sequential fake-`tea` + jq invocations; the default 5s is too tight. + }, 30_000); + + it('pr-view fails with the server message on an error body (not an all-null PR)', () => { + // `tea api` exits 0 and prints the error body. Unvalidated, that produced a + // structurally valid PrViewResult with every field null except `url`, which + // took the error body's own `url` — the swagger link — and shipped it as + // the PR's browser page. + const { status, stdout, stderr } = runScriptFull('pr-view.sh', { CODEV_PR_NUMBER: '404' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('pull request does not exist'); + expect(stderr).not.toContain('swagger'); + }); + + it('user-identity fails on an error body instead of printing "null"', () => { + const { status, stdout, stderr } = runScriptFull('user-identity.sh', { + FAKE_TEA_USER_ERROR: '1', + }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('token does not exist'); + }); + + it('issue-view fails with the server message on an error body', () => { + const { status, stdout, stderr } = runScriptFull('issue-view.sh', { CODEV_ISSUE_ID: '404' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('issue does not exist'); + // The issue is validated BEFORE its comments are fetched, so a bad id + // doesn't also warn about degraded comments on an issue that isn't there + // (and doesn't spend a request on them). + expect(stderr).not.toContain('comments fetch failed'); + }); + + it('issue-view degrades to [] when the comments endpoint answers with an error OBJECT', () => { + // Distinct from issue 98 (comments fetch FAILS): here the fetch succeeds at + // exit 0 with an error object, which reached `jq --argjson` and blew up + // with a raw iteration error instead of the warned [] degrade. + const { status, stdout, stderr } = runScriptFull('issue-view.sh', { CODEV_ISSUE_ID: '97' }); + expect(status).toBe(0); + const issue = JSON.parse(stdout); + expect(issue.title).toBe('Comments error body'); + expect(issue.comments).toEqual([]); + expect(stderr).toContain('comments fetch failed for issue 97'); + }); + + it('recently-merged bounds its walk with CODEV_SINCE_DATE', () => { + // Page 1 is entirely after the cutoff; page 2 continues the same descent + // and crosses it, so the walk stops there. The fixture's page 3 errors, so + // a clean exit is itself the assertion that no third request was made. + const { status, stdout, stderr } = runScriptFull('recently-merged.sh', { + CODEV_REPO: 'acme/dated', + CODEV_SINCE_DATE: '2026-07-05T00:00:00Z', + }); + expect(stderr).toBe(''); + expect(status).toBe(0); + const merged = JSON.parse(stdout); + // Three merges above the cutoff (two on page 1, one on page 2); page 2 also + // carries one merged BELOW it, which must be filtered out. + expect(merged.map((p: { number: number }) => p.number).sort((a: number, b: number) => a - b)) + .toEqual([9, 10, 6100]); + expect(merged[0]).toMatchObject({ + number: 10, + title: 'Recent merge', + url: 'https://git.example.com/acme/dated/pulls/10', + mergedAt: '2026-07-09T00:00:00Z', + headRefName: 'feature/recent', + }); + // #9's merged_at is `+02:00`, not `Z`. It survives the cutoff comparison + // only because the offset is parsed rather than compared lexicographically. + expect(merged.find((p: { number: number }) => p.number === 9)).toMatchObject({ + mergedAt: '2026-07-08T22:00:00+02:00', + headRefName: 'feature/offset', + }); + }); + + it('recently-merged does not stop on page 1 alone, however well ordered', () => { + // The reviewer counterexample. A server that ignores `sort=recentupdate` + // can still return an internally descending page 1 — here one entirely + // older than the cutoff — while a genuinely recent merge sits on page 2. + // Stopping on page-local ordering would silently drop it, so a stop needs + // ordering that survives a page boundary, which this fixture breaks. + const merged = JSON.parse(runScript('recently-merged.sh', { + CODEV_REPO: 'acme/lagging', + CODEV_SINCE_DATE: '2026-07-05T00:00:00Z', + })); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ number: 8100, headRefName: 'feature/late' }); + }); + + it('recently-merged accepts a bare YYYY-MM-DD CODEV_SINCE_DATE', () => { + // `github.ts` passes a full ISO timestamp but `team-update.ts` passes a bare + // date, so both must bound the walk. A bare date reads as midnight UTC — + // same cutoff as the test above, same fixture, same result. + const { status, stdout, stderr } = runScriptFull('recently-merged.sh', { + CODEV_REPO: 'acme/dated', + CODEV_SINCE_DATE: '2026-07-05', + }); + expect(stderr).toBe(''); + expect(status).toBe(0); + expect(JSON.parse(stdout).map((p: { number: number }) => p.number) + .sort((a: number, b: number) => a - b)).toEqual([9, 10, 6100]); + }); + + it('an unparseable CODEV_SINCE_DATE falls back to the unbounded walk', () => { + // Degrade toward MORE work, never toward silently dropping merges: with no + // usable cutoff the stop filter must not fire, so page 3 IS requested (and + // this fixture's page 3 errors, which is how we can see it happened). + const { status, stderr } = runScriptFull('recently-merged.sh', { + CODEV_REPO: 'acme/dated', + CODEV_SINCE_DATE: 'last tuesday', + }); + expect(status).not.toBe(0); + expect(stderr).toContain('dated page 3 requested'); + }); + + it('a non-array page mid-walk is an error, not the end of the list', () => { + // `tea api` exits 0 on HTTP errors, and `jq length` is 0 for both `{}` and + // `null` — so an error body looked exactly like an exhausted list and the + // paginator returned the pages it had at exit 0. + const asObject = runScriptFull('pr-list.sh', { CODEV_REPO: 'acme/errorpage' }); + expect(asObject.status).not.toBe(0); + expect(asObject.stdout.trim()).toBe(''); + expect(asObject.stderr).toContain('not an array'); + + const asNull = runScriptFull('pr-exists.sh', { + CODEV_BRANCH_NAME: 'pad', + CODEV_REPO: 'acme/errorpage', + }); + expect(asNull.status).not.toBe(0); + expect(asNull.stdout.trim()).toBe(''); + expect(asNull.stderr).toContain('not an array'); + }); + + it('an empty body at exit 0 fails instead of succeeding silently', () => { + // jq given empty stdin emits nothing and exits 0, so without an explicit + // guard these scripts "succeed" with empty stdout and the validators never + // run at all. + for (const [script, env] of [ + ['pr-view.sh', { CODEV_PR_NUMBER: '0' }], + ['issue-view.sh', { CODEV_ISSUE_ID: '0' }], + ['user-identity.sh', { FAKE_TEA_USER_EMPTY: '1' }], + ] as Array<[string, Record]>) { + const { status, stdout, stderr } = runScriptFull(script, env); + expect(status, script).not.toBe(0); + expect(stdout.trim(), script).toBe(''); + expect(stderr, script).toContain('empty'); + } + }); + + it('an exactly-full last page at the ceiling is not mistaken for truncation', () => { + // 100 full pages then an empty page 101: the ceiling is reached with every + // page full, but the list length was just an exact multiple of the page + // size and nothing is missing. Failing here would be a hard error on a + // complete result. + const list = JSON.parse(runScript('pr-list.sh', { CODEV_REPO: 'acme/exact' })); + expect(list).toHaveLength(500); // 100 pages x 5 + }, 30_000); + + it('the stop filter handles pages larger than the Linux per-argument cap', () => { + // Each page here is ~150KB, past Linux MAX_ARG_STRLEN (128KiB) — the cap on + // a SINGLE argv string, which applies regardless of ARG_MAX. Handing the + // page to the stop filter through `--argjson` fails with E2BIG on Linux + // while passing on macOS, which has no per-argument cap; both pages go in + // on stdin instead. This assertion is therefore mostly load-bearing on CI. + const { status, stdout, stderr } = runScriptFull('recently-merged.sh', { + CODEV_REPO: 'acme/heavy', + CODEV_SINCE_DATE: '2026-07-05T00:00:00Z', + }); + expect(stderr).toBe(''); + expect(status).toBe(0); + const merged = JSON.parse(stdout); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ number: 7500, headRefName: 'heavy' }); + }); + + it('reports a non-string `.message` instead of dying on the concatenation', () => { + // A proxy or gateway between tea and Gitea can put an object there. + const { status, stdout, stderr } = runScriptFull('pr-view.sh', { CODEV_PR_NUMBER: '500' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + expect(stderr).toContain('upstream refused'); + expect(stderr).not.toContain('jq: error'); + }); + + it('issue-view degrades when the comments array holds non-objects', () => { + // `[1,2]` passes an outer type check but dies on `$comments[] | .body`. + const { status, stdout, stderr } = runScriptFull('issue-view.sh', { CODEV_ISSUE_ID: '96' }); + expect(status).toBe(0); + expect(JSON.parse(stdout).comments).toEqual([]); + expect(stderr).toContain('comments fetch failed for issue 96'); + }); + + it('user-identity rejects a whitespace-only login', () => { + const { status, stdout } = runScriptFull('user-identity.sh', { FAKE_TEA_USER_BLANK: '1' }); + expect(status).not.toBe(0); + expect(stdout.trim()).toBe(''); + }); + + it('gitea_epoch normalizes Gitea\'s server-timezone timestamps', () => { + // Gitea marshals RFC3339 in the SERVER's timezone, so `Z` is not + // guaranteed. These four spell the same instant; a lexicographic compare + // would order them wrongly, which is why the offset is parsed out. + const program = `. "${join(giteaDir, '_lib.sh')}"; ` + + `printf '%s' "$INPUT" | jq -c "\${GITEA_JQ_LIB} [ .[] | gitea_epoch ]"`; + const out = execFileSync('sh', ['-c', program], { + encoding: 'utf-8', + env: { + ...runEnv, + INPUT: JSON.stringify([ + '2026-07-05T12:00:00Z', + '2026-07-05T14:00:00+02:00', + '2026-07-05T10:00:00-02:00', + '2026-07-05T12:00:00.123Z', + '2026-07-05', // bare date -> midnight UTC + '2026-13-99', // right shape, impossible date -> null, not a throw + 'garbage', + null, + 42, + ]), + }, + }).trim(); + expect(JSON.parse(out)).toEqual([ + 1783252800, 1783252800, 1783252800, 1783252800, + 1783209600, + null, null, null, null, + ]); + }); + + it('recently-merged keeps walking when the server ignores sort=recentupdate', () => { + // The since-date bound must not cost data on a server that doesn't honor + // the sort parameter: page 1 contains items older than the cutoff but is + // NOT in descending update order, so the stop filter must not fire and the + // merge that lives only on page 2 must still be reported. + const merged = JSON.parse(runScript('recently-merged.sh', { + CODEV_REPO: 'acme/unsorted', + CODEV_SINCE_DATE: '2026-07-05T00:00:00Z', + })); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ number: 7100, headRefName: 'feature/deep-unsorted' }); + }); + + it('recently-merged without CODEV_SINCE_DATE still walks the whole history', () => { + // The unbounded path is unchanged: no `sort` parameter, every page walked. + const merged = JSON.parse(runScript('recently-merged.sh')); + expect(merged).toHaveLength(2); // #40 (page 1) + #901 (page 2) + }); +}); diff --git a/packages/codev/src/commands/porch/__tests__/bugfix-568-pr-exists-state-all.test.ts b/packages/codev/src/commands/porch/__tests__/bugfix-568-pr-exists-state-all.test.ts index e8f54c1bb..48cc69f63 100644 --- a/packages/codev/src/commands/porch/__tests__/bugfix-568-pr-exists-state-all.test.ts +++ b/packages/codev/src/commands/porch/__tests__/bugfix-568-pr-exists-state-all.test.ts @@ -63,9 +63,12 @@ describe('pr-exists forge scripts', () => { expect(fs.existsSync(scriptPath)).toBe(true); }); - it('fetches all pull states (--state all) to catch merged pulls (#568)', () => { + it('fetches all pull states (state=all) to catch merged pulls (#568)', () => { const content = fs.readFileSync(scriptPath, 'utf-8'); - expect(content).toContain('--state all'); + // #1137: routed through `tea api …/pulls?state=all` (the raw REST + // passthrough) instead of `tea pulls list --state all`, whose flattened + // output can't satisfy the `.head.ref` / `.merged` predicate. + expect(content).toContain('state=all'); }); it('filters out closed-not-merged PRs (#653)', () => { diff --git a/packages/codev/src/lib/forge-contracts.ts b/packages/codev/src/lib/forge-contracts.ts index ae6b39af6..48a572610 100644 --- a/packages/codev/src/lib/forge-contracts.ts +++ b/packages/codev/src/lib/forge-contracts.ts @@ -79,13 +79,17 @@ export interface PrListItem { mergedAt?: string; author?: { login: string }; /** - * Logins of users requested as reviewers. Emitted by every forge's `pr-list` - * script (GitHub flattens gh's reviewer objects to logins; GitLab/Gitea emit - * `[]` as they expose no GitHub-equivalent per-user review-request list). - * Consumed by the VSCode PR sidebar to bucket "review-requested" PRs. + * Logins of users requested as reviewers. Every preset populates it for real: + * GitHub flattens gh's reviewer objects to logins, GitLab maps glab's + * `.reviewers[].username`, and Gitea maps the REST `requested_reviewers[]` + * (team reviewers carry no login and are dropped). Consumed by the VSCode PR + * sidebar to bucket "review-requested" PRs. */ reviewRequests: string[]; - /** Whether the PR is a draft. GitLab/Gitea emit `false` (not exposed). */ + /** + * Whether the PR is a draft. Every preset reports it for real — GitLab from + * the merge request's `draft`/WIP flag, Gitea from the REST `draft` flag. + */ isDraft: boolean; }