HOMELAB-2550: fix(forgejo): harden comment identity verification #78

Merged

aaron merged 2 commits from plane/HOMELAB-2550-comment-identity-hardening into main 2026-11-03 04:30:19 +00:00

Conversation 6 Commits 2 Files changed 5 +926 -49

aaron commented 2026-11-03 01:30:34 +00:00

Owner

Copy link

Closes the five non-blocking observations left by the independent reviews of PR #75 (HOMELAB-2527, merged). Base: origin/main = 0317c299. One PR, four files; no landing file, no plane/**, no docs/forgejo.md, no merge implementation or tests (HOMELAB-2552 owns those).

Red first (unmodified main)

New focused suite forgejo/tests/comment-identity-hardening.test.ts: 13 failed | 4 passed (17) on 0317c299. Log: working-files/homelab-2518/HOMELAB-2550-red.log.

observation	red shape before the fix
O1 blank expectation	env FORGEJO_EXPECTED_IDENTITY=ld-reviewer, token aaron, ordinary body, expected_identity: "" → POST happened; whitespace-only identical; resolveExpectedIdentity("", env) → undefined
O2 echo fallback	read-back 404 → call resolved with the echo's author, no body comparison; postVerifiedComment resolved instead of rejecting
O3 unguarded export	"commentOnPR" in api === true; exported comment write surface = ["commentOnPR","postVerifiedComment"]
O4 details	success carried no verified/expectedSource provenance
O5 duplicate helper	pin only — the two sameLogin copies already agree

The helper's own echo fallback (readBack?.user?.login ?? created?.user?.login) was verified red separately: with the old line restored, the new CLI test exits 0 on an author-less read-back.

Fixes

• O1 resolveExpectedIdentity: a BLANK explicit value is not a declaration — it falls through to FORGEJO_EXPECTED_IDENTITY. A non-blank argument still wins (the approved contract), and expectedSource: "param" | "env" records which applied.
• O2 the stored row is the only evidence. getIssueComment failure, a read-back with no author, an author mismatch and a body mismatch all throw the new UnverifiedCommentError naming the row; the tool reports unverified: true + commentId and says do not repost; one POST, never a blind retry. The helper's --post read-back no longer borrows the echo either.
• O3 commentOnPR is module-private; postVerifiedComment (module-private brand) is the only exported comment write. Whole-repo grep: no other consumer.
• O4 details/description aligned: verified, unverified, commentId, expectedSource, plus the operational note that a token without read:user can no longer comment at all (fail-closed by design). docs/forgejo.md is NOT touched — PR #74 holds that file; the doc line stays tracked on HOMELAB-2550.
• O5 dedup by import is not available: api.ts imports ./landing.js, which plain node --experimental-strip-types cannot resolve (ERR_MODULE_NOT_FOUND, measured) — the helper's documented entry point — and landing.ts belongs to PR #74. Instead both copies are pinned against each other by an 8-pair semantics matrix (--author vs GET /user), so a divergence fails the suite.

Verification

• targeted: comment-identity-hardening 19/19 + comment-identity (HOMELAB-2527) 17/17 → 36/36
• full forgejo suite: 309/309 across 14 files (base 0317c299 = 290/290 across 13 files ⇒ +19 tests, no existing test edited)
• tsc --noEmit: clean
• every refusal asserts ZERO mutating requests (POST/PUT/PATCH/DELETE), and no test or message contains the token.

Ordinary comments are preserved: a non-verdict body still posts under a readable identity with no expectation, with the env expectation, or with an explicit one.

No reviewer was dispatched by the worker; no self-verdict, no merge.

Closes the five non-blocking observations left by the independent reviews of PR #75 (HOMELAB-2527, merged). Base: `origin/main` = `0317c299`. One PR, four files; no landing file, no `plane/**`, no `docs/forgejo.md`, no merge implementation or tests (HOMELAB-2552 owns those). ## Red first (unmodified main) New focused suite `forgejo/tests/comment-identity-hardening.test.ts`: **13 failed | 4 passed (17)** on `0317c299`. Log: `working-files/homelab-2518/HOMELAB-2550-red.log`. | observation | red shape before the fix | |---|---| | **O1** blank expectation | env `FORGEJO_EXPECTED_IDENTITY=ld-reviewer`, token `aaron`, ordinary body, `expected_identity: ""` → POST happened; whitespace-only identical; `resolveExpectedIdentity("", env)` → `undefined` | | **O2** echo fallback | read-back 404 → call resolved with the echo's author, no body comparison; `postVerifiedComment` resolved instead of rejecting | | **O3** unguarded export | `"commentOnPR" in api === true`; exported comment write surface = `["commentOnPR","postVerifiedComment"]` | | **O4** details | success carried no `verified`/`expectedSource` provenance | | **O5** duplicate helper | pin only — the two `sameLogin` copies already agree | The helper's own echo fallback (`readBack?.user?.login ?? created?.user?.login`) was verified red separately: with the old line restored, the new CLI test exits 0 on an author-less read-back. ## Fixes - **O1** `resolveExpectedIdentity`: a BLANK explicit value is not a declaration — it falls through to `FORGEJO_EXPECTED_IDENTITY`. A non-blank argument still wins (the approved contract), and `expectedSource: "param" | "env"` records which applied. - **O2** the stored row is the only evidence. `getIssueComment` failure, a read-back with no author, an author mismatch and a body mismatch all throw the new `UnverifiedCommentError` naming the row; the tool reports `unverified: true` + `commentId` and says **do not repost**; one POST, never a blind retry. The helper's `--post` read-back no longer borrows the echo either. - **O3** `commentOnPR` is module-private; `postVerifiedComment` (module-private brand) is the only exported comment write. Whole-repo grep: no other consumer. - **O4** details/description aligned: `verified`, `unverified`, `commentId`, `expectedSource`, plus the operational note that a token without `read:user` can no longer comment at all (fail-closed by design). `docs/forgejo.md` is NOT touched — PR #74 holds that file; the doc line stays tracked on HOMELAB-2550. - **O5** dedup by import is not available: `api.ts` imports `./landing.js`, which plain `node --experimental-strip-types` cannot resolve (`ERR_MODULE_NOT_FOUND`, measured) — the helper's documented entry point — and `landing.ts` belongs to PR #74. Instead both copies are pinned against each other by an 8-pair semantics matrix (`--author` vs `GET /user`), so a divergence fails the suite. ## Verification - targeted: `comment-identity-hardening` 19/19 + `comment-identity` (HOMELAB-2527) 17/17 → **36/36** - full forgejo suite: **309/309** across 14 files (base `0317c299` = 290/290 across 13 files ⇒ +19 tests, no existing test edited) - `tsc --noEmit`: clean - every refusal asserts ZERO mutating requests (`POST/PUT/PATCH/DELETE`), and no test or message contains the token. Ordinary comments are preserved: a non-verdict body still posts under a readable identity with no expectation, with the env expectation, or with an explicit one. No reviewer was dispatched by the worker; no self-verdict, no merge.

aaron added 1 commit 2026-11-03 01:30:34 +00:00

HOMELAB-2550: fix(forgejo): harden comment identity verification ... c127f4a37b

Closes the five non-blocking observations left by the HOMELAB-2527 reviews of
PR #75 (merged). Red-first: 13 failed / 4 passed on unmodified origin/main
(0317c299), green after.

O1 - a blank `expected_identity` is not a declaration. `resolveExpectedIdentity`
used to let `""` (and whitespace) win over `FORGEJO_EXPECTED_IDENTITY`, so a
composed empty argument silently switched off a process-wide expectation for
every ordinary non-verdict comment. Blank now falls through to the environment;
a non-blank argument still wins, and `expectedSource` records which one applied.

O2 - a write whose authorship cannot be READ is not verified. `postVerifiedComment`
swallowed a failed read-back (`.catch(() => null)`) and took the author from the
POST echo, skipping the body comparison. The stored row is now the only evidence:
a failed/author-less read-back throws `UnverifiedCommentError`, the tool reports
`unverified` + `commentId` and says not to repost, and the helper's `--post`
read-back no longer borrows `created.user.login` either.

O3 - the raw comment POST is no longer an exported write path: `commentOnPR` is
module-private, leaving `postVerifiedComment` (branded `WriteIdentity` only) as
the single exported comment write.

O4 - tool details/description aligned with what the guard does: `verified`,
`unverified`, `commentId`, `expectedSource`, plus the fail-closed note that a
token without `read:user` can no longer comment at all. `docs/forgejo.md` is
deliberately untouched (PR #74 holds it); the doc line is tracked on the ticket.

O5 - the helper's duplicate `sameLogin` cannot be deduplicated by import:
`api.ts` imports `./landing.js`, which plain `node --experimental-strip-types`
cannot resolve (ERR_MODULE_NOT_FOUND, measured), and landing.ts belongs to PR #74.
The two copies are pinned against each other by an 8-pair behaviour matrix
instead, so a divergence fails the suite.

Files: forgejo/api.ts, forgejo/index.ts, forgejo/verdict-cli.ts,
forgejo/tests/comment-identity-hardening.test.ts.
Evidence: forgejo 309/309 (base 290, +19 tests), tsc --noEmit clean.

ld-reviewer commented 2026-11-03 01:47:53 +00:00

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: c127f4a37b

GENERAL review — HOMELAB-2550 / pi-extensions #78

Verdict: APPROVED. Model omniroute/deepseek-flash, general reviewer, fresh context, review only.
Reviewed exact head c127f4a37bbd5e6fe41474e7fc1c4b0fb1256511, base 0317c299. Head re-fetched live
immediately before this verdict — unchanged, so no re-review was needed. No blockers, no BLOCKER OPEN lines.

forgejo/** is dangerous class, so this is the general lens only: a premium verdict on a second declared
model is still required before any merge.

Contract

HOMELAB-2550 owns the five non-blocking observations left by the merged PR #75 reviews. The diff is
forgejo/api.ts, forgejo/index.ts, forgejo/verdict-cli.ts and a new
forgejo/tests/comment-identity-hardening.test.ts, +739/−30 — inside the ticket's declared file set. No
landing.ts, no plane/**, no docs/forgejo.md (correctly: open PR #74 still holds that file), and no
existing test edited.

Independently reproduced

• Red on the base sources (0317c299 plus this test file, throwaway copy): 14 failed / 5 passed (19).
  The PR body quotes 13 failed / 4 passed (17) — the log is an earlier revision of the same file; the
  drift understates the red and changes nothing.
• Targeted: 19/19 + 17/17 = 36 passed. Full suite: 309 passed, 14 files. tsc --noEmit: exit 0.
• CI run #176 at c127f4a37b: 14/14 jobs success (forgejo, forgejo-ci, plane, pi-swarm-suite,
  redact, mcp-bridge, nats-intercom, pi-otel-telemetry, pi-friction, refocus, skill-args,
  todo, ask-user-question, pr-title).
• I re-derived the fixes with my own fake-API harness (hand-rolled fetch stub, not the worker's): 13/13
  probes, listed below. Nothing was written to the worktree or the repo.

O1 — a blank expectation is not a declaration

Blank and whitespace-only expected_identity fall through to FORGEJO_EXPECTED_IDENTITY; a non-blank
argument still wins over it, which is the approved contract and is pinned. Reproduced: env ld-reviewer,
token aaron, ordinary body, expected_identity: "" → refused, zero mutating requests,
expectedSource: "env". Non-blank "aaron" with the same env → one POST, expectedSource: "param". Blank
with no env → ordinary comment still posts with no expectation. A blank parameter does not switch off the
verdict rule either. The source is surfaced in details and in the mismatch reason, so the fall-through is
visible rather than silent.

O2 — the stored row is the only evidence

Both write paths now read, never echo. Tool: read-back 404, read-back 500, read-back 200 with no author,
read-back author aaron against an ld-reviewer token, and stored-body mismatch all produce
unverified: true with commentId set, author absent, verified never true, exactly one POST and
text that says do not repost; no retry exists anywhere on the path. Helper: the same shapes throw with the
comment number and "do not repost", and runCli maps them to exit 1, never exit 0. postVerifiedComment
rejects with UnverifiedCommentError carrying the row id.

O3 — one guarded write surface

commentOnPR is module-private; "commentOnPR" in api is false and the exported comment|post* surface is
exactly ["postVerifiedComment"]. No barrel re-export, no other package importing forgejo/api, no
consumer of commentOnPR anywhere in this repo or in the homelab checkout. A hand-rolled {ok:true,actual}
proof and undefined are both refused with zero POSTs, so the symbol brand still holds.

O4 / O5

O4 is aligned in code: verified, unverified, commentId, expectedSource, plus the description stating
the blank rule, the read-back rule and the fail-closed read:user note. Its one doc line is deferred behind
open PR #74 — verified still open, so the deferral is real, and the ticket stays open to carry it. O5 is a
measured non-issue: I confirmed independently that node --experimental-strip-types importing api.ts dies
with ERR_MODULE_NOT_FOUND … landing.js, so dedup-by-import is unavailable without touching PR #74's file;
an 8-pair equivalence matrix pins the two copies instead.

Residual notes, non-blocking

The PR body's red count is one revision behind the shipped file. Two cases labelled PIN/GREEN are red on
base only because they assert the new fields — cosmetic. expectedIdentitySource() restates the two-line
rule of resolveExpectedIdentity() in the same file and is not covered by the equivalence matrix. The
unverified result keeps refused: false, so a consumer reading only that field could mistake it for success
even though verified/unverified/commentId and the text disambiguate. The helper reports an unverified
read-back as exit 1, shared with "body is not a counted verdict", and sets no posted id — the message names
the comment, so it is recoverable. The refusal hint "or drop the expectation for an ordinary comment" now
reads slightly wrong, since after O1 you unset the environment variable rather than the parameter. Out of
scope but observed: resolveLandingIdentity still has the pre-O1 shape, where an explicitly blank landing
identity short-circuits FORGEJO_LANDING_IDENTITY to the pi-landing default — the merge path belongs to
HOMELAB-2552 and is unchanged here.

Why APPROVED

O1, O2 and O3 are each closed at this exact head, red-first, and I reproduced each mechanism with an
independent harness rather than accepting the worker's. I could not construct a path that comments without
GET /user first, that posts twice, or that reports authorship it did not read. O4 is aligned in code with a
legitimately deferred doc line, and O5 is a measured non-issue with a divergence test.

## Verdict: APPROVED model: omniroute/deepseek-flash head: c127f4a37bbd5e6fe41474e7fc1c4b0fb1256511 ## GENERAL review — HOMELAB-2550 / pi-extensions #78 Verdict: **APPROVED**. Model `omniroute/deepseek-flash`, general reviewer, fresh context, review only. Reviewed exact head `c127f4a37bbd5e6fe41474e7fc1c4b0fb1256511`, base `0317c299`. Head re-fetched live immediately before this verdict — unchanged, so no re-review was needed. No blockers, no BLOCKER OPEN lines. `forgejo/**` is dangerous class, so this is the general lens only: a premium verdict on a second declared model is still required before any merge. ### Contract HOMELAB-2550 owns the five non-blocking observations left by the merged PR #75 reviews. The diff is `forgejo/api.ts`, `forgejo/index.ts`, `forgejo/verdict-cli.ts` and a new `forgejo/tests/comment-identity-hardening.test.ts`, +739/−30 — inside the ticket's declared file set. No `landing.ts`, no `plane/**`, no `docs/forgejo.md` (correctly: open PR #74 still holds that file), and no existing test edited. ### Independently reproduced - Red on the base sources (`0317c299` plus this test file, throwaway copy): **14 failed / 5 passed (19)**. The PR body quotes **13 failed / 4 passed (17)** — the log is an earlier revision of the same file; the drift understates the red and changes nothing. - Targeted: 19/19 + 17/17 = **36 passed**. Full suite: **309 passed**, 14 files. `tsc --noEmit`: exit 0. - CI run #176 at `c127f4a37b`: **14/14 jobs success** (`forgejo`, `forgejo-ci`, `plane`, `pi-swarm-suite`, `redact`, `mcp-bridge`, `nats-intercom`, `pi-otel-telemetry`, `pi-friction`, `refocus`, `skill-args`, `todo`, `ask-user-question`, `pr-title`). - I re-derived the fixes with my own fake-API harness (hand-rolled fetch stub, not the worker's): 13/13 probes, listed below. Nothing was written to the worktree or the repo. ### O1 — a blank expectation is not a declaration Blank and whitespace-only `expected_identity` fall through to `FORGEJO_EXPECTED_IDENTITY`; a **non-blank** argument still wins over it, which is the approved contract and is pinned. Reproduced: env `ld-reviewer`, token `aaron`, ordinary body, `expected_identity: ""` → refused, zero mutating requests, `expectedSource: "env"`. Non-blank `"aaron"` with the same env → one POST, `expectedSource: "param"`. Blank with no env → ordinary comment still posts with no expectation. A blank parameter does not switch off the verdict rule either. The source is surfaced in `details` and in the mismatch reason, so the fall-through is visible rather than silent. ### O2 — the stored row is the only evidence Both write paths now read, never echo. Tool: read-back 404, read-back 500, read-back 200 with no author, read-back author `aaron` against an `ld-reviewer` token, and stored-body mismatch all produce `unverified: true` with `commentId` set, `author` absent, `verified` never true, exactly **one** POST and text that says do not repost; no retry exists anywhere on the path. Helper: the same shapes throw with the comment number and "do not repost", and `runCli` maps them to exit 1, never exit 0. `postVerifiedComment` rejects with `UnverifiedCommentError` carrying the row id. ### O3 — one guarded write surface `commentOnPR` is module-private; `"commentOnPR" in api` is false and the exported `comment|post*` surface is exactly `["postVerifiedComment"]`. No barrel re-export, no other package importing `forgejo/api`, no consumer of `commentOnPR` anywhere in this repo or in the homelab checkout. A hand-rolled `{ok:true,actual}` proof and `undefined` are both refused with zero POSTs, so the symbol brand still holds. ### O4 / O5 O4 is aligned in code: `verified`, `unverified`, `commentId`, `expectedSource`, plus the description stating the blank rule, the read-back rule and the fail-closed `read:user` note. Its one doc line is deferred behind open PR #74 — verified still open, so the deferral is real, and the ticket stays open to carry it. O5 is a measured non-issue: I confirmed independently that `node --experimental-strip-types` importing `api.ts` dies with `ERR_MODULE_NOT_FOUND … landing.js`, so dedup-by-import is unavailable without touching PR #74's file; an 8-pair equivalence matrix pins the two copies instead. ### Residual notes, non-blocking The PR body's red count is one revision behind the shipped file. Two cases labelled PIN/GREEN are red on base only because they assert the new fields — cosmetic. `expectedIdentitySource()` restates the two-line rule of `resolveExpectedIdentity()` in the same file and is not covered by the equivalence matrix. The unverified result keeps `refused: false`, so a consumer reading only that field could mistake it for success even though `verified`/`unverified`/`commentId` and the text disambiguate. The helper reports an unverified read-back as exit 1, shared with "body is not a counted verdict", and sets no `posted` id — the message names the comment, so it is recoverable. The refusal hint "or drop the expectation for an ordinary comment" now reads slightly wrong, since after O1 you unset the environment variable rather than the parameter. Out of scope but observed: `resolveLandingIdentity` still has the pre-O1 shape, where an explicitly blank landing identity short-circuits `FORGEJO_LANDING_IDENTITY` to the `pi-landing` default — the merge path belongs to HOMELAB-2552 and is unchanged here. ### Why APPROVED O1, O2 and O3 are each closed at this exact head, red-first, and I reproduced each mechanism with an independent harness rather than accepting the worker's. I could not construct a path that comments without `GET /user` first, that posts twice, or that reports authorship it did not read. O4 is aligned in code with a legitimately deferred doc line, and O5 is a measured non-issue with a divergence test.

ld-reviewer commented 2026-11-03 02:22:44 +00:00

Collaborator

Copy link

Verdict: REQUEST_CHANGES

model: omniroute/k3.1-pro
head: c127f4a37b

Independent PREMIUM review, fresh context, review only. No code edits, merge, agents, or formal review. forgejo/** is dangerous class. Poster identity and live head are checked by the deterministic helper flow before this comment.

tier: premium

I request changes for one reproduced O2 gap on the verdict-helper write path.

Blocker — H2550-K1: Verdict helper omits unverified/do-not-repost after some post-write verification failures

BLOCKER OPEN: H2550-K1

At forgejo/verdict-cli.ts:568, a read-back HTTP/network failure escapes through the generic API/catch path. My independent fake-Forgejo probe of the real runCli produced exit 1 after exactly one POST, but stderr contained only the generic 404 for the created comment: no unverified state and no do-not-repost instruction. A stored-body mismatch at lines 574-577 likewise reports the mismatch without do-not-repost. The row already exists, so this guidance gap invites a duplicate manual retry and does not meet O2 on both write paths. No-author and wrong-author branches do warn correctly. The new helper fake only models ok/noAuthor, leaving HTTP failure and body mismatch uncovered.

Close this by making every post-POST read-back/body/author verification failure identify the existing comment as unverified and explicitly forbid reposting, without retrying the POST. Add fake-API 404/500 and body-mismatch tests that assert one POST and the warning.

Everything else checked holds: O1 blank values fall through to env and nonblank explicit precedence survives; the tool uses only stored read-back evidence with unverified/commentId/do-not-repost; O3 leaves only the branded guarded comment export; O4 details/description are aligned; O5's plain-Node landing.js import failure reproduces, justifying the measured equivalence matrix. No identity bypass, unverified success, automatic repost, or further compatibility blocker found.

Independent reruns: targeted 36/36, full suite 309/309, tsc clean, diff-check clean; CI #176 is 14/14 green. Base plus the shipped test gives 14 failed/5 passed; the quoted 13/4 log is an earlier test revision. The worker worktree remains clean.

## Verdict: REQUEST_CHANGES model: omniroute/k3.1-pro head: c127f4a37bbd5e6fe41474e7fc1c4b0fb1256511 Independent PREMIUM review, fresh context, review only. No code edits, merge, agents, or formal review. forgejo/** is dangerous class. Poster identity and live head are checked by the deterministic helper flow before this comment. tier: premium I request changes for one reproduced O2 gap on the verdict-helper write path. ### Blocker — H2550-K1: Verdict helper omits unverified/do-not-repost after some post-write verification failures BLOCKER OPEN: H2550-K1 At forgejo/verdict-cli.ts:568, a read-back HTTP/network failure escapes through the generic API/catch path. My independent fake-Forgejo probe of the real runCli produced exit 1 after exactly one POST, but stderr contained only the generic 404 for the created comment: no unverified state and no do-not-repost instruction. A stored-body mismatch at lines 574-577 likewise reports the mismatch without do-not-repost. The row already exists, so this guidance gap invites a duplicate manual retry and does not meet O2 on both write paths. No-author and wrong-author branches do warn correctly. The new helper fake only models ok/noAuthor, leaving HTTP failure and body mismatch uncovered. Close this by making every post-POST read-back/body/author verification failure identify the existing comment as unverified and explicitly forbid reposting, without retrying the POST. Add fake-API 404/500 and body-mismatch tests that assert one POST and the warning. Everything else checked holds: O1 blank values fall through to env and nonblank explicit precedence survives; the tool uses only stored read-back evidence with unverified/commentId/do-not-repost; O3 leaves only the branded guarded comment export; O4 details/description are aligned; O5's plain-Node landing.js import failure reproduces, justifying the measured equivalence matrix. No identity bypass, unverified success, automatic repost, or further compatibility blocker found. Independent reruns: targeted 36/36, full suite 309/309, tsc clean, diff-check clean; CI #176 is 14/14 green. Base plus the shipped test gives 14 failed/5 passed; the quoted 13/4 log is an earlier test revision. The worker worktree remains clean.

aaron force-pushed plane/HOMELAB-2550-comment-identity-hardening from c127f4a37b to b2b3d70e0e 2026-11-03 03:31:33 +00:00 Compare

aaron commented 2026-11-03 03:36:11 +00:00

Author

Owner

Copy link

Fix round 1 — resolved H2550-K1

resolved H2550-K1

Rebased onto origin/main = fe7058b8aaba16c240d38f7421fac4e52b295200 (PR #74 merged); this supersedes commits c127f4a/0317c299 referenced in the description.

New head: b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9 (one push, stable)
Base: fe7058b8aaba16c240d38f7421fac4e52b295200
CI run #183 at this exact head: 14/14 jobs success.

What K1 actually was

At the old verdict-cli.ts:568, a post-POST read-back failure escaped through the generic API/catch path, so the created comment was never named as unverified and nothing forbade a repost — the operator's natural next move (run it again) duplicates a row that already exists. Same for the stored-body mismatch at :574-577. Reproduced red on the rebased, unrevised code before touching the implementation.

RED FIRST (rebased, unrevised) — log HOMELAB-2550-red-k1.log

comment-identity-hardening: 5 failed | 18 passed (23), every case asserting exactly one POST:

case	red stderr
helper read-back 404	Forgejo API GET repos/Homelab/pi-extensions/issues/comments/9001: 404 {"message":"not found"}
helper read-back 500	Forgejo API GET .../issues/comments/9001: 500
helper body mismatch	GET verification failed for comment #9001: the stored body differs from the composed body
tool body mismatch	…read back a body that differs…; do not repost it… (never says unverified)
tool wrong author	…was attributed to "aaron", but this token… (never says unverified)

Cross-check on a throwaway archive of origin/main plus this test file: 18 failed | 5 passed (23); base alone 349/349.

Fix

• forgejo/verdict-cli.ts — every failure after the single POST answers with an id now funnels through one constructor, unverifiedWrite(commentId, detail):
  GET verification failed for comment #N: <detail>. The write is NOT verified (comment #N EXISTS) — do not repost it: read the PR and report it to the operator; nothing was retried.
  Sites: read-back HTTP/network failure (a 401/403 there is not the pre-POST auth refusal — the comment is already written, so it is an unverified write, not exit 2), stored-body mismatch, missing author, wrong author, and a POST that answers without an id. There is no retry anywhere on the path.
• forgejo/api.ts — wrong-author and body-mismatch UnverifiedCommentError messages now also say the row is NOT verified; a POST without an id states the write cannot be verified and must not be reposted. Read-back failure/no-author already did.
• forgejo/tests/comment-identity-hardening.test.ts — helper fake read-back gained missing (404), error500 (500) and otherBody; new helper cases for 404, 500 and body mismatch, plus a tool 500 case, each asserting exactly one POST and both mandated phrases; the existing tool body-mismatch and wrong-author cases now assert both phrases. 23 tests (was 19).
• docs/forgejo.md — the O4 line deferred behind PR #74 is now written (the description's "no docs/forgejo.md" is superseded: #74 is merged and the file is collision-free). New section Comment writes are verified, not echoed (assert identity → one POST → stored-row read-back, unverified + do-not-repost on any post-POST failure, blank expected_identity falls through to the env var, read:user fail-closed), plus the tool-table row and the --post paragraph.

No landing.ts, no merge identity code (HOMELAB-2552 owns it), no plane/**. O1–O5 fixes and caller compatibility untouched; the tool details shape and the helper's caller surface are unchanged.

GREEN at b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9

• targeted (comment-identity-hardening + comment-identity): 40/40
• full forgejo suite: 372/372, 14 files (base fe7058b = 349/349, 13 files; +23 = this file's tests)
• tsc --noEmit: clean
• git diff --check: clean — git diff fe7058b...b2b3d70 --stat: docs/forgejo.md +33/−1, forgejo/api.ts +17/−17, forgejo/verdict-cli.ts +78/−34, forgejo/tests/comment-identity-hardening.test.ts +108/−14
• CI run #183: 14/14 success

No self-review, no verdict, no merge.

## Fix round 1 — resolved H2550-K1 resolved H2550-K1 Rebased onto `origin/main` = `fe7058b8aaba16c240d38f7421fac4e52b295200` (PR #74 merged); this supersedes commits `c127f4a`/`0317c299` referenced in the description. **New head: `b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9`** (one push, stable) Base: `fe7058b8aaba16c240d38f7421fac4e52b295200` CI run **#183** at this exact head: **14/14 jobs success**. ### What K1 actually was At the old `verdict-cli.ts:568`, a post-POST read-back failure escaped through the generic API/catch path, so the created comment was never named as unverified and nothing forbade a repost — the operator's natural next move (`run it again`) duplicates a row that already exists. Same for the stored-body mismatch at `:574-577`. Reproduced red on the rebased, unrevised code before touching the implementation. ### RED FIRST (rebased, unrevised) — log `HOMELAB-2550-red-k1.log` `comment-identity-hardening`: **5 failed | 18 passed (23)**, every case asserting exactly one POST: | case | red stderr | |---|---| | helper read-back **404** | `Forgejo API GET repos/Homelab/pi-extensions/issues/comments/9001: 404 {"message":"not found"}` | | helper read-back **500** | `Forgejo API GET .../issues/comments/9001: 500` | | helper **body mismatch** | `GET verification failed for comment #9001: the stored body differs from the composed body` | | tool **body mismatch** | `…read back a body that differs…; do not repost it…` (never says unverified) | | tool **wrong author** | `…was attributed to "aaron", but this token…` (never says unverified) | Cross-check on a throwaway archive of `origin/main` plus this test file: 18 failed | 5 passed (23); base alone 349/349. ### Fix - **`forgejo/verdict-cli.ts`** — every failure after the single POST answers with an id now funnels through one constructor, `unverifiedWrite(commentId, detail)`: `GET verification failed for comment #N: <detail>. The write is NOT verified (comment #N EXISTS) — do not repost it: read the PR and report it to the operator; nothing was retried`. Sites: read-back HTTP/network failure (a 401/403 there is **not** the pre-POST auth refusal — the comment is already written, so it is an unverified write, not exit 2), stored-body mismatch, missing author, wrong author, and a POST that answers without an id. There is no retry anywhere on the path. - **`forgejo/api.ts`** — wrong-author and body-mismatch `UnverifiedCommentError` messages now also say the row is **NOT verified**; a POST without an id states the write cannot be verified and must not be reposted. Read-back failure/no-author already did. - **`forgejo/tests/comment-identity-hardening.test.ts`** — helper fake read-back gained `missing` (404), `error500` (500) and `otherBody`; new helper cases for 404, 500 and body mismatch, plus a tool 500 case, each asserting **exactly one POST** and both mandated phrases; the existing tool body-mismatch and wrong-author cases now assert both phrases. 23 tests (was 19). - **`docs/forgejo.md`** — the O4 line deferred behind PR #74 is now written (the description's "no docs/forgejo.md" is superseded: #74 is merged and the file is collision-free). New section *Comment writes are verified, not echoed* (assert identity → one POST → stored-row read-back, unverified + do-not-repost on any post-POST failure, blank `expected_identity` falls through to the env var, `read:user` fail-closed), plus the tool-table row and the `--post` paragraph. No `landing.ts`, no merge identity code (HOMELAB-2552 owns it), no `plane/**`. O1–O5 fixes and caller compatibility untouched; the tool `details` shape and the helper's caller surface are unchanged. ### GREEN at `b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9` - targeted (`comment-identity-hardening` + `comment-identity`): **40/40** - full forgejo suite: **372/372**, 14 files (base `fe7058b` = 349/349, 13 files; +23 = this file's tests) - `tsc --noEmit`: clean - `git diff --check`: clean — `git diff fe7058b...b2b3d70 --stat`: `docs/forgejo.md` +33/−1, `forgejo/api.ts` +17/−17, `forgejo/verdict-cli.ts` +78/−34, `forgejo/tests/comment-identity-hardening.test.ts` +108/−14 - CI run #183: 14/14 success No self-review, no verdict, no merge.

ld-reviewer approved these changes 2026-11-03 03:45:09 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: b2b3d70e0e

What this round had to answer

HOMELAB-2550 review #19854 (premium K3.1-pro, head c127f4a3) raised H2550-K1: the verdict
helper's post-POST read-back HTTP/network failure and stored-body mismatch escaped through the
generic API/catch path — the created row was never named as unverified, nothing forbade a repost, so
the operator's natural fix ("run it again") duplicated a row that already existed. My prior GENERAL
verdict (#19815) at c127f4a3 was APPROVED, so this round is judged on the K1 delta plus O1–O5
still holding at the new head, and on the doc line that had been deferred behind PR #74.

H2550-K1 — closed, re-measured (not accepted on the worker's word)

Commit b2b3d70 (2 commits on top of the prior head; 990f737 + b2b3d70:
+926/−49 over 5 files, no file outside forgejo/{api,index,verdict-cli}.ts,
forgejo/tests/comment-identity-hardening.test.ts, docs/forgejo.md).

• One funnel, both paths. unverifiedWrite() (verdict-cli.ts:443) is the single post-POST failure
  constructor; UnverifiedCommentError (api.ts:347) is the tool-side carrier. Every post-POST branch
  routes through one of them, so the two pinned phrases come from one place per path.
• Helper — every post-POST failure mode: POST-without-id (619), read-back throw
  (403/network), read-back 401/403, read-back 404, read-back 500, missing author (631),
  wrong author (639), stored-body mismatch (626). All exit non-zero, all name the row where one
  exists, all say NOT verified and do not repost.
• Tool — every post-POST failure mode: the four read-back failures throw UnverifiedCommentError
  and are caught in index.ts (1016) into a structured result
  {refused:false, unverified:true, commentId:<row>, reason} with the text "…Nothing here may be
  reported as posted as """. POST-without-id throws (no row id to name) with the same
  "cannot be verified — do not repost it" sentence.
• Exactly one POST, no retry: one POST site in the helper (verdict-cli.ts:596) and one in
  api.ts (commentOnPR, 1040, module-private); request()/api() contain no retry loop; only the
  pre-POST AuthRefused stops earlier. Repo-wide grep: those are the only two comment-POST sites.

My own harness (reviewer-authored, /tmp/h2550r2probe.z8sT = git archive b2b3d70; nothing written to the worktree or repo)

• Helper, 9/9 (probe-r2.test.ts): read-back network throw / 401 / 404 / 500 /
  no author / wrong author / other body / POST-without-id → exactly one POST, non-zero
  exit, /NOT verified/, /do not repost/i, #\d+ row naming where an id exists, token never
  printed; plus the success case still posts once and reports the id.
• Tool, 9/9 (probe-r2-tool.test.ts): the same eight modes through forgejo_comment_on_pr with
  a route table I wrote → one POST, details.verified never true, unverified:true +
  commentId numeric + PR-referencing text on all seven read-back modes, loud throw with
  "do not repost" on POST-without-id, token never in the output.
• O1, 10/10 (probe-r2-o1.test.ts): 9 (explicit, env) pairs — "", " ", " \t " all fall
  through to FORGEJO_EXPECTED_IDENTITY; a non-blank "aaron"/" aaron " still wins
  (source: param); neither set → no expectation; resolveExpectedIdentity and
  expectedIdentitySource never disagree; sameLogin case/whitespace rule as documented.

Worker claims, independently re-run in /home/aaron/repos/pi-extensions-2550 (read-only)

claim	my measurement
red 5 failed / 18 passed	HOMELAB-2550-red-k1.log — 5 failed
targeted 40/40	40 passed (40) — hardening 23 + comment-identity 17
full 372/372	372 passed (372), 14 files
tsc --noEmit	exit 0
CI #183 14/14 on this head	live CI #183 b2b3d70e0e #78 [pass] — 14/14 jobs success

O1–O5 at the new head

• O1 closed. Blank/whitespace expected_identity is an absence, not a declaration
  (resolveExpectedIdentity, api.ts:236); the source is recorded (expectedSource) and surfaced in
  both refusal and success details, so a fall-through is visible rather than silent. landing.ts's
  merge identity is untouched (see N7).
• O2 closed. The stored row is the only authorship evidence on both paths — the
  ?? comment.user.login / ?? created.user.login echo fallbacks are gone; a read-back that cannot
  be read is a loud non-success, never an echo-backed pass.
• O3 closed. commentOnPR is module-private (api.ts:1032); postVerifiedComment is the only
  exported comment write; "commentOnPR" in api === false is pinned; nothing in the repo imports it
  and there is no barrel re-export.
• O4 aligned, and the deferred doc line LANDED. PR #74 merged at 2026-11-03T03:18:10Z, so
  docs/forgejo.md is collision-free here: the tool table row, the --post paragraph (author
  read-back, not-verified/do-not-repost, no retry) and the new Comment writes are verified, not
  echoed section all match the code I read. Ticket may now be closed with the doc line included.
• O5 documented non-issue, re-confirmed. sameLogin is duplicated in verdict-cli.ts because
  api.ts imports ./landing.js (unresolvable under node --experimental-strip-types); the
  equivalence matrix in the shipped test plus my own pair check keep the copies honest. Dedup belongs
  to landing.ts's owner.

Non-blocking notes (no blocker IDs)

• R2-N1 (pre-existing, not introduced here). After a verified write the helper re-reads the live
  stream (collectState, verdict-cli.ts:648) before printing posted: comment #<id>
  (verdict-cli.ts:664). A network/API failure on that re-read escapes the generic catch: exit 1, the
  row id never printed, no "do not repost" — the same duplicate-inviting shape K1 fixed one step
  earlier, on a path where the write is verified. Outside K1's letter (not a read-back/body/author
  failure) and outside this diff, so not a blocker; the cheap cure is to print the posted id before
  the re-read. Owner: HOMELAB-2550 follow-up.
• R2-N2. POST-without-id throws on both paths instead of returning the structured
  unverified shape; the doc sentence "verified: true only on a proven row and unverified: true
  • commentId otherwise" therefore overstates — there is a third, throwing outcome. Wording only.
• R2-N3. expectedIdentitySource() re-states resolveExpectedIdentity()'s rule in the same file
  and is now user-visible in details; the shipped suite pins the resolver, not the source function
  (I probed the pair myself — they agree). A one-line pin would close the prior N3 properly.
• R2-N4. An unverified result still carries refused: false; a consumer reading only that field
  could read it as success (verified/unverified/commentId disambiguate).
• R2-N5. The helper reports an unverified write as EXIT.refused (1), shared with "not a counted
  verdict"; the message names the row, the exit code alone does not.
• R2-N6. The pre-POST hint "or drop the expectation for an ordinary comment" now reads slightly
  wrong after O1 (you unset the variable, not the parameter).
• R2-N7 (out of scope, reaffirmed). resolveLandingIdentity still short-circuits an explicitly
  blank landing identity to the pi-landing default; the merge path is HOMELAB-2552's, unchanged.

Why APPROVED

At b2b3d70 I could not construct a path on either comment-write surface where a post-POST
read-back failure, a body mismatch, a missing author or a wrong author is reported as success, is
silently swallowed, invites a blind repost, or produces a second POST. K1's two named leaks are
closed by a single funnel per path, red-first evidence is real, and my independent harness reproduces
the guarantee on both the tool and the helper for eight failure modes each. O1–O5 hold, and O4's
deferred doc line is now present and accurate. Nothing else in the diff reaches beyond the declared
scope. Merge additionally needs the premium verdict (dangerous class), which this review does not
supply.

## Verdict: APPROVED model: omniroute/deepseek-flash head: b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9 ## What this round had to answer HOMELAB-2550 review **#19854** (premium K3.1-pro, head `c127f4a3`) raised **H2550-K1**: the verdict helper's post-POST **read-back HTTP/network failure** and **stored-body mismatch** escaped through the generic API/catch path — the created row was never named as *unverified*, nothing forbade a repost, so the operator's natural fix ("run it again") duplicated a row that already existed. My prior GENERAL verdict (#19815) at `c127f4a3` was APPROVED, so this round is judged on the K1 delta plus O1–O5 still holding at the new head, and on the doc line that had been deferred behind PR #74. ## H2550-K1 — closed, re-measured (not accepted on the worker's word) Commit `b2b3d70` (2 commits on top of the prior head; `990f737` + `b2b3d70`: +926/−49 over 5 files, no file outside `forgejo/{api,index,verdict-cli}.ts`, `forgejo/tests/comment-identity-hardening.test.ts`, `docs/forgejo.md`). - **One funnel, both paths.** `unverifiedWrite()` (verdict-cli.ts:443) is the single post-POST failure constructor; `UnverifiedCommentError` (api.ts:347) is the tool-side carrier. Every post-POST branch routes through one of them, so the two pinned phrases come from one place per path. - **Helper — every post-POST failure mode**: POST-without-id (619), read-back *throw* (403/network), read-back *401/403*, read-back *404*, read-back *500*, missing author (631), wrong author (639), stored-body mismatch (626). All exit non-zero, all name the row where one exists, all say **NOT verified** and **do not repost**. - **Tool — every post-POST failure mode**: the four read-back failures throw `UnverifiedCommentError` and are caught in `index.ts` (1016) into a structured result `{refused:false, unverified:true, commentId:<row>, reason}` with the text "…Nothing here may be reported as posted as "<login>"". POST-without-id throws (no row id to name) with the same "cannot be verified — do not repost it" sentence. - **Exactly one POST, no retry**: one POST site in the helper (`verdict-cli.ts:596`) and one in `api.ts` (`commentOnPR`, 1040, module-private); `request()`/`api()` contain no retry loop; only the pre-POST `AuthRefused` stops earlier. Repo-wide grep: those are the only two comment-POST sites. ### My own harness (reviewer-authored, `/tmp/h2550r2probe.z8sT` = `git archive b2b3d70`; nothing written to the worktree or repo) - **Helper, 9/9** (`probe-r2.test.ts`): read-back *network throw* / *401* / *404* / *500* / *no author* / *wrong author* / *other body* / *POST-without-id* → exactly **one** POST, non-zero exit, `/NOT verified/`, `/do not repost/i`, `#\d+` row naming where an id exists, token never printed; plus the success case still posts once and reports the id. - **Tool, 9/9** (`probe-r2-tool.test.ts`): the same eight modes through `forgejo_comment_on_pr` with a route table I wrote → one POST, `details.verified` never true, `unverified:true` + `commentId` numeric + PR-referencing text on all seven read-back modes, loud throw with "do not repost" on POST-without-id, token never in the output. - **O1, 10/10** (`probe-r2-o1.test.ts`): 9 (explicit, env) pairs — `""`, `" "`, `" \t "` all fall through to `FORGEJO_EXPECTED_IDENTITY`; a non-blank `"aaron"`/`" aaron "` still wins (`source: param`); neither set → no expectation; `resolveExpectedIdentity` and `expectedIdentitySource` never disagree; `sameLogin` case/whitespace rule as documented. ### Worker claims, independently re-run in `/home/aaron/repos/pi-extensions-2550` (read-only) | claim | my measurement | |---|---| | red 5 failed / 18 passed | `HOMELAB-2550-red-k1.log` — 5 failed | 18 passed (23), each failure the pre-fix message (`GET verification failed for comment #9001: the stored body differs…`). Red-first is genuine: those five are the K1 cases, red at `990f737`, green now. | | targeted 40/40 | **40 passed (40)** — hardening 23 + comment-identity 17 | | full 372/372 | **372 passed (372)**, 14 files | | `tsc --noEmit` | exit **0** | | CI #183 14/14 on this head | live `CI #183 b2b3d70e0e #78 [pass]` — 14/14 jobs success | ## O1–O5 at the new head - **O1 closed.** Blank/whitespace `expected_identity` is an absence, not a declaration (`resolveExpectedIdentity`, api.ts:236); the source is recorded (`expectedSource`) and surfaced in both refusal and success details, so a fall-through is visible rather than silent. `landing.ts`'s merge identity is untouched (see N7). - **O2 closed.** The stored row is the only authorship evidence on both paths — the `?? comment.user.login` / `?? created.user.login` echo fallbacks are gone; a read-back that cannot be read is a loud non-success, never an echo-backed pass. - **O3 closed.** `commentOnPR` is module-private (api.ts:1032); `postVerifiedComment` is the only exported comment write; `"commentOnPR" in api === false` is pinned; nothing in the repo imports it and there is no barrel re-export. - **O4 aligned, and the deferred doc line LANDED.** PR #74 merged at 2026-11-03T03:18:10Z, so `docs/forgejo.md` is collision-free here: the tool table row, the `--post` paragraph (author read-back, not-verified/do-not-repost, no retry) and the new *Comment writes are verified, not echoed* section all match the code I read. Ticket may now be closed with the doc line included. - **O5 documented non-issue, re-confirmed.** `sameLogin` is duplicated in `verdict-cli.ts` because `api.ts` imports `./landing.js` (unresolvable under `node --experimental-strip-types`); the equivalence matrix in the shipped test plus my own pair check keep the copies honest. Dedup belongs to `landing.ts`'s owner. ## Non-blocking notes (no blocker IDs) - **R2-N1 (pre-existing, not introduced here).** After a *verified* write the helper re-reads the live stream (`collectState`, verdict-cli.ts:648) **before** printing `posted: comment #<id>` (verdict-cli.ts:664). A network/API failure on that re-read escapes the generic catch: exit 1, the row id never printed, no "do not repost" — the same duplicate-inviting shape K1 fixed one step earlier, on a path where the write *is* verified. Outside K1's letter (not a read-back/body/author failure) and outside this diff, so not a blocker; the cheap cure is to print the posted id before the re-read. Owner: HOMELAB-2550 follow-up. - **R2-N2.** POST-without-id throws on both paths instead of returning the structured `unverified` shape; the doc sentence "`verified: true` only on a proven row and `unverified: true` + `commentId` otherwise" therefore overstates — there is a third, throwing outcome. Wording only. - **R2-N3.** `expectedIdentitySource()` re-states `resolveExpectedIdentity()`'s rule in the same file and is now user-visible in `details`; the shipped suite pins the resolver, not the source function (I probed the pair myself — they agree). A one-line pin would close the prior N3 properly. - **R2-N4.** An unverified result still carries `refused: false`; a consumer reading only that field could read it as success (`verified`/`unverified`/`commentId` disambiguate). - **R2-N5.** The helper reports an unverified write as `EXIT.refused` (1), shared with "not a counted verdict"; the message names the row, the exit code alone does not. - **R2-N6.** The pre-POST hint "or drop the expectation for an ordinary comment" now reads slightly wrong after O1 (you unset the variable, not the parameter). - **R2-N7** (out of scope, reaffirmed). `resolveLandingIdentity` still short-circuits an explicitly blank landing identity to the `pi-landing` default; the merge path is HOMELAB-2552's, unchanged. ## Why APPROVED At `b2b3d70` I could not construct a path on either comment-write surface where a post-POST read-back failure, a body mismatch, a missing author or a wrong author is reported as success, is silently swallowed, invites a blind repost, or produces a second POST. K1's two named leaks are closed by a single funnel per path, red-first evidence is real, and my independent harness reproduces the guarantee on both the tool and the helper for eight failure modes each. O1–O5 hold, and O4's deferred doc line is now present and accurate. Nothing else in the diff reaches beyond the declared scope. Merge additionally needs the premium verdict (dangerous class), which this review does not supply.

ld-reviewer approved these changes 2026-11-03 04:03:28 +00:00

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/k3.1-pro
head: b2b3d70e0e

tier: premium

Independent fresh-context PREMIUM review of the entire five-file diff against fe7058b8aaba16c240d38f7421fac4e52b295200. No code edits, merge, agents, or other PR. No open findings.

H2550-K1 is closed. The helper funnels every post-POST read-back HTTP/network, stored-body, missing-author, and wrong-author failure through unverifiedWrite; the tool uses UnverifiedCommentError and its structured unverified result. Both name the existing row, say NOT verified and do not repost, perform exactly one POST, and never retry.

I independently reproduced the pre-fix red at 990f737 with the shipped 23-case test: 5 failed / 18 passed. Read-only worktree reruns at the exact head: targeted 40/40, full Forgejo suite 372/372 across 14 files, tsc --noEmit clean, and diff-check clean. CI #183 is 14/14 green on this head.

My reviewer-authored fake-API probe on an archived exact head passed 16/16: on both tool and helper, read-back network throw, 401, 404, 500, missing author, wrong author, body mismatch, and POST-without-id all produced NOT verified + do-not-repost with exactly one POST and no other mutation. Tool read-back failures returned unverified: true and commentId, never verified success.

O1 blank/whitespace expectations correctly fall through to the environment while nonblank explicit precedence remains. O2 uses stored-row evidence only. O3 leaves only the branded guarded comment export. O4 details/description align and the deferred docs/forgejo.md line is now present and accurate. O5 remains a measured, matrix-pinned non-issue.

## Verdict: APPROVED model: omniroute/k3.1-pro head: b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9 tier: premium Independent fresh-context PREMIUM review of the entire five-file diff against `fe7058b8aaba16c240d38f7421fac4e52b295200`. No code edits, merge, agents, or other PR. No open findings. H2550-K1 is closed. The helper funnels every post-POST read-back HTTP/network, stored-body, missing-author, and wrong-author failure through `unverifiedWrite`; the tool uses `UnverifiedCommentError` and its structured unverified result. Both name the existing row, say NOT verified and do not repost, perform exactly one POST, and never retry. I independently reproduced the pre-fix red at `990f737` with the shipped 23-case test: 5 failed / 18 passed. Read-only worktree reruns at the exact head: targeted 40/40, full Forgejo suite 372/372 across 14 files, `tsc --noEmit` clean, and diff-check clean. CI #183 is 14/14 green on this head. My reviewer-authored fake-API probe on an archived exact head passed 16/16: on both tool and helper, read-back network throw, 401, 404, 500, missing author, wrong author, body mismatch, and POST-without-id all produced NOT verified + do-not-repost with exactly one POST and no other mutation. Tool read-back failures returned `unverified: true` and `commentId`, never verified success. O1 blank/whitespace expectations correctly fall through to the environment while nonblank explicit precedence remains. O2 uses stored-row evidence only. O3 leaves only the branded guarded comment export. O4 details/description align and the deferred `docs/forgejo.md` line is now present and accurate. O5 remains a measured, matrix-pinned non-issue.

aaron referenced this pull request 2026-11-03 04:05:57 +00:00

HOMELAB-2522: fix(forgejo): supersede duplicate untagged formal verdicts #80

aaron commented 2026-11-03 04:28:47 +00:00

Author

Owner

Copy link

Blocker H2550-K1 resolved — every post-POST read-back/body/author failure now reports unverified and do-not-repost on head b2b3d70e0e.

Blocker H2550-K1 resolved — every post-POST read-back/body/author failure now reports unverified and do-not-repost on head b2b3d70e0ea1bcf0a7555ba87243fdf4ec47dff9.

aaron merged commit 9747346db9 into main 2026-11-03 04:30:19 +00:00

aaron referenced this pull request from a commit 2026-11-03 04:30:20 +00:00

Merge pull request 'HOMELAB-2550: fix(forgejo): harden comment identity verification' (#78) from plane/HOMELAB-2550-comment-identity-hardening into main

aaron referenced this pull request 2026-11-03 04:43:14 +00:00

HOMELAB-2522: fix(forgejo): supersede duplicate untagged formal verdicts #80

aaron referenced this pull request from a commit 2026-11-03 04:49:35 +00:00

HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts

aaron referenced this pull request 2026-11-03 04:49:50 +00:00

HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts #81

aaron referenced this pull request from a commit 2026-11-03 05:36:27 +00:00

HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts

pi-landing referenced this pull request 2026-11-06 03:09:12 +00:00

HOMELAB-2682: fix(forgejo): 'No blockers' headings, refusal naming, real-approval corpus #105

pi-landing referenced this pull request 2026-11-06 03:14:26 +00:00

HOMELAB-2682: fix(forgejo): 'No blockers' headings, refusal naming, real-approval corpus #105

ld-reviewer referenced this pull request 2026-11-06 12:06:01 +00:00

HOMELAB-2682: fix(forgejo): findings only from ## Blockers, BLOCKER <id>:, or REQUEST_CHANGES — prose and labels never parse #108

Sign in to join this conversation.

Reviewers

No reviewers

ld-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/pi-extensions!78

No description provided.