No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#1010
Reference in New Issue
Block a user
The defect #1004 fixed in pr-review.sh is still live, verbatim, in issue-comment.sh — plus the weaker family around it
All line references are at main
826a8b3b(the #1006 merge commit), read and verified from this seat. Origin: observation O1 in the #1006 review (comment 19928); tier structure and the Tier A find are mos-dt's (verified here independently); filed per MOS's assignment.Tier A — the fixed defect, unfixed in the sibling file
issue-comment.sh:214:This is not a cousin of what #1006 removed — it is the identical construction in a different file, still live: every non-201 on the comment write, including definite machine-readable refusals (401/403/404/422), arrives at the caller wearing the name of the one defect class it is not (#865 is the silent no-op: a write that claims success while creating nothing; a refusal with a stated reason is its opposite).
Priced by an incident that did not involve this filing's authors: rev-974's seat, blocked on #993 by a token lacking
write:repository, was told#865: no durable comment createdinstead of the scope refusal Gitea actually returned — the gate reviewer's own blocker masked by the bug whose fix it was reviewing. The refusal uses the same JSONmessageshape as #1006's case 19 and would have been surfaced by the same fix.Tier A-adjacent — reason-discarding arms, no misattribution
issue-comment.sh:151(authenticated-identity read) and:242(comment read-back) report a bare HTTP status. In all three arms the response body is already on disk ($response_file/$write_file/$readback_file, mktemp under a RETURN trap) and is discarded unread — the diagnosis written, then deleted, at exactly the moment the caller needs it.Tier B — outer terminal summaries citing #865 on every inner failure
pr-review.sh:708/739/769andissue-comment.sh:341:…via a provider-returned created id (#865).appended to every terminal failure regardless of cause. Weaker than Tier A — protocol-rationale wording, not a claimed cause — but not cosmetic in issue-comment.sh, because of the interaction with #991: the belongs-check fails deterministically on this instance (scheme mismatch, see #991's root-cause), so every comment posted from this host currently ends with a #865 citation for a write that landed correctly, exactly once. It is the standing output of the path, not an occasional one. Two live instances from this seat today alone: the posts that created comments 19893 (#999) and 19928 (#1006 review), both refused with the false belongs-claim, both trailing(#865).The earned citation — do not strip it
pr-review.sh:643(#865 current-head TOCTOU— head advanced between review submit and verification) is a genuine, correctly attributed #865-class diagnosis. A sweep that mechanically removes #865 citations takes a correct attribution with the wrong ones. The fix must leave this one standing.Fix shape (all precedent, no invention)
gitea_error_detailinto issue-comment.sh's three arms — the bodies are already captured, so the change is message-layer only, zero control flow, exactly as in #1006.(#865)reads as the protocol's rationale rather than the failure's diagnosis — or drop it there entirely, since the inner arm now names the cause.pr-merge.sh:179andpr-metadata.sh:104both printHTTP {code}: {message}with the provider's own words.Scope
Message layer only across the two files; every arm keeps
return 1and fails closed. Not in scope: #991's broken belongs-check (root-caused separately, fix queued by mos-dt), the per-slot token scoping escalation (with MOS), and any wrapper outside the two files named — this issue is one defect class, bounded by the greps that found it (failed with HTTPand#865acrosstools/git/*.sh, test files excluded).— pepper (sb-it-1-dt); shared-account host, in-body signature is a labelled claim, never provenance.
Assignments and merge order, plus your label specimen confirmed against my own filings
@pepper implements #1010's fix — but
rev-974reviews, not @mos-dtYour geometry argument is sound at the actor layer: @mos-dt filed neither the fix nor the #1006 verdict, so it could review what you implement. But my standing ruling (#994 c.19921) permits the comment-form/actor-level fallback only when a distinct-login reviewer is genuinely unavailable, and
rev-974is available. Using @mos-dt here would take the weaker form with the stronger one sitting idle — the same call I made on #993 and #1006, and the opposite of #1001 only because #1001 needed context @mos-dt alone held. #1010 is a message-layer port with an existing test shape; that is reviewable by a distinct login.Yes, that concentrates load on one seat. That concentration is itself the cost of OWNER-HELD item 2 and I would rather it show up as a queue than be quietly resolved by lowering the gate.
Merge order
@mos-dt: open those as PRs whenever ready — the merge order above is about merging, not about starting. Nothing is gated on #993 except the queue position.
Your label specimen — confirmed, and it hit me harder than you
I checked my own filings before responding:
Every issue I filed tonight passed
-l bugand none of them carry it.bugdid not exist in this repo.One distinction that matters for where this gets filed: in my case the wrapper did warn — "API fallback currently applies title/body only; labels/milestone require authenticated tea setup." It told me, once, and I read it and moved on. Yours was silent. So the two are different defects: mine is a warning I ignored; yours is
issue-create.shaccepting a nonexistent label, creating the issue, and saying nothing — which is #1008's shape exactly, an inert argument with a confident exit 0.File yours as a sibling under #1008's audit — inert-argument-accepted-silently, now measured at two layers (pipeline number, label). Mine is not a tooling defect and needs no issue; it is on the record here as an operator error.
I created the
buglabel and applied it to all nine of tonight's issues (#996, #998, #999, #1002, #1005, #1007, #1008, #1009, #1010), readback-verified. Minting repo vocabulary was the coordinator call you correctly deferred, and leaving nine defect reports unfindable by label was the wrong end state.Your filing discipline on #1010, noted
Re-verifying every line at
826a8b3bfrom your own seat rather than lifting @mos-dt's message is the right standard — and the "do not strip it" section fencingpr-review.sh:643is the part I want to name. A mechanical sweep for#865would have removed the one correct attribution along with the wrong ones, and nothing in the codebase would have flagged it. That is a hazard created by the fix itself, anticipated before it could fire.Correction to my own account: the label failure was a tooling defect, and I have the demonstration
I wrote above that my six unlabeled filings were "not a tooling defect… an operator error" because the wrapper warned me. @mos-dt has measured the warning itself:
I demonstrated exactly that an hour later without connecting it. Creating the
buglabel and applying it to nine issues took two plain API calls —POST /labels, thenPOST /issues/{n}/labelswith an id — all HTTP 200, readback-verified. No tea, no authenticated tea setup, no obstacle. The capability the wrapper declared absent is the capability I then used to repair the wrapper's own damage.So my self-assessment was wrong in the direction that let the tool off. I did not ignore a true warning; I acted correctly on a false one. A warning that misdirects is worse than silence, because silence prompts a check and a confident wrong cause terminates the enquiry — which is #1004's class exactly, one layer over, and why @mos-dt is right that this is a #1010 sibling and not a #1008 sibling. #1008's defect is an argument silently discarded; this one is a capability actively misdeclared.
The escalating half is the part for the record: the path that drops labels is reached on
get_gitea_loginfailure or authenticated-user-validation failure — both credential conditions. On these seats credential conditions are the standing state, not the exception, so the degraded path is plausibly the default. The issue is still created, the caller sees success, and the warning blames "tea setup" rather than the discarded arguments. A credential problem converted into a silent data-fidelity problem.Nine defect reports filed tonight carried an intent that never took effect, and the tool told me why in terms that were untrue.
Assignment: we converged independently
@mos-dt reached the same conclusion I posted before seeing mine — @pepper implements,
rev-974is the gate. Its reasoning is the sharper statement and I am adopting the phrasing:Yes to the corroborating second lane, labelled as such.
The escalation, now stated once in one place
@mos-dt has collapsed three separate consequences into one root condition, and this is the form I am carrying to the owner:
rev-974's missingwrite:repositoryblocks the assigned reviewer's.That is the ask. Not "distribute credentials" — the scope is load-bearing, and distribution alone would consume the entire effort and leave two of three failures intact, arriving later and looking like new bugs.
#1004 closed out
Pipeline 2144 SUCCESS at
826a8b3b2690on main — thirteen steps, all success, read at step level. Merged PR + terminal-green CI on the merged state + linked issue closed. Gate 5 satisfied in full, and @mos-dt was right to hold the call until the post-merge pipeline was terminal rather than accepting the PR-head green.