From a55fbaef158c619aeef15f69355ebb487b09cdba Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Wed, 5 Aug 2026 15:01:32 +0000 Subject: [PATCH] =?UTF-8?q?fix(forge):=20forge=5Fcommit=5Fat=20=E2=80=94?= =?UTF-8?q?=20Forgejo=20serves=20a=20single=20commit=20at=20/git/commits/{?= =?UTF-8?q?sha}=20(#209)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the first post-merge sweep after the 0.6.0 merge — #198's own acceptance probe — not by review. Three PRs in one run: labels: #208: could not read the head commit's date: forge_api: HTTP 404 from 'GET repos/heavy-duty/ceremony/commits/f3a1336…' — blocker:unrequested not judged this pass Measured against this instance: forgejo repos/{o}/{r}/commits/{sha} -> 404 forgejo repos/{o}/{r}/git/commits/{sha} -> 200, date under `.created` github repos/{o}/{r}/commits/{sha} -> 200, date nested A fourth asymmetry, alongside the three lib/forge-forgejo.sh's header already records. #198 ported this call site onto the shim with GitHub's path unchanged — correct against GitHub, and the block it lives in (#236 D2) arrived WITH the merge, so nothing here had ever executed it. So it becomes a verb rather than a path at the call site: the caller wants one timestamp and should not have to know either shape. Cost while it stood was bounded and loud rather than silent — guarded_read refused and the sweep said so — but blocker:unrequested could never be judged on this forge. The tests pin each backend's PATH and FIELD, because a stubbed forge_api cannot catch a wrong path; that is exactly how this shipped and why it took a live sweep to find. Swapping the paths reds the forgejo pair; swapping the fields reds the github one. test/run.sh 28/28 under jq 1.7 and jq 1.6; forge-backends 124/124; shellcheck 0.10.0 and actionlint clean. Refs #209 --- actions/labels-reconcile/labels-reconcile.sh | 4 ++-- changelog.d/209.md | 16 +++++++++++++ lib/forge-forgejo.sh | 21 +++++++++++++++++ lib/forge-github.sh | 12 ++++++++++ test/forge-backends.test.sh | 24 ++++++++++++++++++++ 5 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 changelog.d/209.md diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index b6c49f1..449ab12 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -1094,8 +1094,8 @@ main() { HEAD_COMMIT_AT="" if [ "$DRAFT" != true ]; then HEAD_COMMIT_ERR_FILE="$(mktemp)" - HEAD_COMMIT_AT="$(forge_api "repos/$REPO/commits/$HEAD_SHA" \ - --jq '.commit.committer.date' 2>"$HEAD_COMMIT_ERR_FILE" || echo "")" + HEAD_COMMIT_AT="$(forge_commit_at "$HEAD_SHA" \ + 2>"$HEAD_COMMIT_ERR_FILE" || echo "")" HEAD_COMMIT_ERR="$(cat "$HEAD_COMMIT_ERR_FILE")" rm -f "$HEAD_COMMIT_ERR_FILE" case "$HEAD_COMMIT_AT" in diff --git a/changelog.d/209.md b/changelog.d/209.md new file mode 100644 index 0000000..320b984 --- /dev/null +++ b/changelog.d/209.md @@ -0,0 +1,16 @@ +### Fixed + +- `blocker:unrequested` is judged on this forge again. The head-commit date was + read from `repos/{o}/{r}/commits/{sha}`, which Forgejo answers **404** — so + every sweep degraded and left the blocker unjudged (#209). + +- `forge_commit_at` is a verb on both backends: GitHub serves a single commit at + the bare path with the date nested, Forgejo at `git/commits/{sha}` with it + under `.created`. The caller asks for one timestamp and knows neither shape + (#209). + +### Added + +- `test/forge-backends.test.sh` pins each backend's path **and** field, because + a stubbed `forge_api` cannot catch a wrong path — which is how this shipped + and why a live sweep was what found it (#209). diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index 0583f86..4331cc6 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -642,6 +642,27 @@ forge_release_exists() { # forge_commit_pulls — the pull requests whose merge produced , as # a JSON ARRAY in GitHub's shape. An empty array is a completed read that # found nothing; a non-zero exit is a read that did not complete. +# forge_commit_at — the commit's committer date, ISO-8601, or empty. +# +# THE FOURTH ASYMMETRY (#209), measured 2026-08-05: +# +# GET /repos/{o}/{r}/commits/{sha} -> 404 (200 on GitHub) +# GET /repos/{o}/{r}/git/commits/{sha} -> 200 date under `.created` +# +# Found by the first post-merge sweep after the 0.6.0 merge, not by review: +# #198 ported this call site onto the shim with GitHub's path unchanged, and +# the block it lives in had never executed here before. Every sweep printed +# `could not read the head commit's date` and left blocker:unrequested +# unjudged. +# +# `.created` and not `.commit.committer.date`: the /git/commits payload is the +# git object, whose top-level `created` is the committer date. The verb hides +# both differences so the caller keeps asking for one timestamp. +forge_commit_at() { + local sha="${1:?forge_commit_at: sha required}" + forge_api "repos/$REPO/git/commits/$sha" --jq '.created' +} + forge_commit_pulls() { local sha="${1:?forge_commit_pulls: sha required}" body code out body="$(mktemp)" diff --git a/lib/forge-github.sh b/lib/forge-github.sh index 36307e1..c2e268e 100644 --- a/lib/forge-github.sh +++ b/lib/forge-github.sh @@ -186,6 +186,18 @@ forge_release_exists() { # forge_commit_pulls — the pull requests whose merge produced , as # a JSON array. GitHub serves the array directly; the forgejo twin builds # one from its single-object endpoint so this call site is identical. +# forge_commit_at — the commit's committer date, ISO-8601, or empty. +# +# A VERB rather than a path at the call site, because the two forges do not +# agree on where a single commit lives: GitHub serves it at /commits/{sha}, +# Forgejo 404s there and serves it at /git/commits/{sha} with the timestamp +# under a different field (#209). The caller wants one timestamp; it should not +# have to know either shape. +forge_commit_at() { + local sha="${1:?forge_commit_at: sha required}" + forge_api "repos/$REPO/commits/$sha" --jq '.commit.committer.date' +} + forge_commit_pulls() { local sha="${1:?forge_commit_pulls: sha required}" errf out rc err errf="$(mktemp)" diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh index fd44a84..d304218 100644 --- a/test/forge-backends.test.sh +++ b/test/forge-backends.test.sh @@ -775,6 +775,21 @@ writes_after() { "$@" >/dev/null 2>&1; cat "$WRITES"; } repo_empty_release() { REPO='' forge_release_exists 1.2.3; } repo_empty_pulls() { REPO='' forge_commit_pulls deadbeef; } +# forge_commit_at — the FOURTH asymmetry (#209). Forgejo 404s on /commits/{sha} +# and serves the object at /git/commits/{sha}, with the committer date under +# `.created` rather than `.commit.committer.date`. A stubbed forge_api cannot +# catch a wrong PATH, which is how #198 shipped GitHub's path here and every +# sweep printed `could not read the head commit's date`. +release_stub 200 '{"created":"2026-08-05T13:11:33Z","commit":{"committer":{"date":"WRONG"}}}' +check "forgejo: the commit date comes from .created" 0 "2026-08-05T13:11:33Z" \ + forge_commit_at deadbeef +fj_not_nested() { ! forge_commit_at deadbeef | grep -q WRONG; } +check "...and never from GitHub's nested field" 0 "" fj_not_nested +check "forgejo: it asks /git/commits/{sha}" 0 "git/commits/deadbeef" \ + writes_after forge_commit_at deadbeef +fj_not_bare_path() { ! grep -qE 'repos/o/r/commits/deadbeef( |$)' "$WRITES"; } +check "...and never the bare /commits/{sha}, which 404s here" 0 "" fj_not_bare_path + release_stub 200 '{"number":7,"merged_at":"2026-01-01T00:00:00Z","labels":[{"name":"release"}]}' check "forgejo: one PR object becomes a one-element array" 0 '"number":7' \ forge_commit_pulls deadbeef @@ -857,5 +872,14 @@ check "github: the tag goes to /git/refs" 0 "git/refs" \ gh_after forge_tag_create 1.2.3 cafebabe check "github: PRs behind a commit use the PLURAL path" 0 "commits/deadbeef/pulls" \ gh_after forge_commit_pulls deadbeef +# The other half of #209's asymmetry: GitHub serves a single commit at the bare +# path, with the date nested. Swapping the two backends' paths must red one of +# these two files, which is the whole point of pinning both. +check "github: a single commit is the BARE path" 0 "commits/deadbeef" \ + gh_after forge_commit_at deadbeef +gh_not_git_commits() { ! gh_after forge_commit_at deadbeef | grep -q 'git/commits'; } +check "...and never Forgejo's git/commits" 0 "" gh_not_git_commits +check "...reading the nested committer date" 0 "commit.committer.date" \ + gh_after forge_commit_at deadbeef summary -- 2.45.2