mosaicstack/stack
25
0
Fork 1
Code Issues 344 Pull Requests 13 Actions Packages Projects Releases 15 Wiki Activity

False-success is a CLASS: issue-close.sh has #976's exact shape, and its checked path is the fallback not the default #977

New Issue
Open
opened 2026-07-31 02:31:48 +00:00 by Ghost · 1 comment
Ghost commented 2026-07-31 02:31:48 +00:00
Copy Link
Copy Source

Extends #976 from a single wrapper to a class, and identifies which wrappers are affected. Source-read only — nothing was executed.

Why this matters more than #976 alone

Gate 8 covers wrapper failure (blocked → report → stop). It says nothing about wrapper false success. Gate 5 completion is merged PR + terminal-green CI + issue closed — and two of those three are attested by the wrapper family that has now been confirmed to manufacture a PASS on a write path (#976).

issue-close.sh — identical shape to the confirmed defect

tea issue comment ...      # unconditioned
tea issue close   ...      # unconditioned
echo "Closed Gitea issue #N"    # unconditional, under set -e, no status test, no readback

That is #976 line for line, in the wrapper that attests an issue is closed.

The three open causes from #976 all reach it

The one visible difference is the repo argument — issue-comment.sh passes --repo REPO_SLUG, issue-close.sh passes --repo OWNER/REPO. That is not a discriminator: in detect-platform.sh, get_repo_slug is get_repo_info, and OWNER/REPO is that same value split and rejoined. For a two-segment slug the string handed to tea is byte-identical. Same login resolver, same tea binary, same slug — so login resolution, slug resolution, and tea exiting 0 having done nothing all reach it unchanged.

The inversion is the filing-worthy part

issue-close.sh has a sound path — its Gitea API fallback uses curl -fsS, where -f makes curl exit non-zero on HTTP ≥400 and set -e aborts before the echo. Genuinely fail-closed.

But it is the fallback. It is reached only when login resolution fails. The blind tea path is the default and the checked path is the exception — precisely backwards.

pr-merge.sh — materially better, not clean

Correcting an initial grep screen that flagged both wrappers as the same shape (the screen produced candidates; reading produced the finding):

if tea pr merge ...; then       # exit-code conditioned; non-zero routes to fallback or exit 1
raw_code matched against ^2     # API fallback checks HTTP status and prints a real error

A login failure or slug failure cannot produce a false success here. Its exposure is exactly one of the three causes — tea exiting 0 having done nothing — plus a weaker residual: a 2xx is not a readback of merged=true.

pr-review.sh — the counter-example

It already reads back after writing. The fix pattern exists in-tree, which is what makes this a fixable class rather than a design constraint.

Explicit bound

It has not been shown that tea misbehaves on issue close. Nothing was run. What source establishes is detection capability: issue-close.sh could not detect the failure confirmed on issue-comment.sh, because it tests nothing at all. Whether it would fail is unmeasured — and whatever is learned chasing #976 transfers directly, since the inputs are identical.

Required

  1. Invert issue-close.sh: make the checked path the default, or add a readback (does the issue now read state=closed?) and condition the success line on it.
  2. Add a readback to pr-merge.sh asserting merged=true, not merely a 2xx.
  3. Apply pr-review.sh's existing pattern across the family.
  4. A needle per wrapper: stub tea to exit 0 without effect — each wrapper must fail.

Latent, unrelated

Reconstructing OWNER/REPO by splitting on the first and last slash mangles any slug with more than two segments. Not currently exercised; flagged only.

Extends #976 from a single wrapper to a **class**, and identifies which wrappers are affected. Source-read only — **nothing was executed**. ## Why this matters more than #976 alone Gate 8 covers wrapper **failure** (blocked → report → stop). It says nothing about wrapper **false success**. Gate 5 completion is *merged PR + terminal-green CI + issue closed* — and **two of those three are attested by the wrapper family that has now been confirmed to manufacture a PASS on a write path** (#976). ## `issue-close.sh` — identical shape to the confirmed defect ```bash tea issue comment ... # unconditioned tea issue close ... # unconditioned echo "Closed Gitea issue #N" # unconditional, under set -e, no status test, no readback ``` That is #976 line for line, **in the wrapper that attests an issue is closed.** ### The three open causes from #976 all reach it The one visible difference is the repo argument — `issue-comment.sh` passes `--repo REPO_SLUG`, `issue-close.sh` passes `--repo OWNER/REPO`. **That is not a discriminator:** in `detect-platform.sh`, `get_repo_slug` *is* `get_repo_info`, and `OWNER/REPO` is that same value split and rejoined. For a two-segment slug the string handed to `tea` is **byte-identical**. Same login resolver, same `tea` binary, same slug — so login resolution, slug resolution, and *tea exiting 0 having done nothing* all reach it unchanged. ### The inversion is the filing-worthy part `issue-close.sh` **has** a sound path — its Gitea API fallback uses `curl -fsS`, where `-f` makes curl exit non-zero on HTTP ≥400 and `set -e` aborts before the echo. Genuinely fail-closed. **But it is the fallback.** It is reached only when login resolution *fails*. **The blind `tea` path is the default and the checked path is the exception — precisely backwards.** ## `pr-merge.sh` — materially better, not clean Correcting an initial grep screen that flagged both wrappers as the same shape (the screen produced *candidates*; reading produced the finding): ```bash if tea pr merge ...; then # exit-code conditioned; non-zero routes to fallback or exit 1 raw_code matched against ^2 # API fallback checks HTTP status and prints a real error ``` A login failure or slug failure **cannot** produce a false success here. Its exposure is exactly **one** of the three causes — `tea` exiting 0 having done nothing — plus a weaker residual: **a 2xx is not a readback of `merged=true`.** ## `pr-review.sh` — the counter-example It already reads back after writing. **The fix pattern exists in-tree**, which is what makes this a fixable class rather than a design constraint. ## Explicit bound **It has not been shown that `tea` misbehaves on issue close.** Nothing was run. What source establishes is **detection capability**: `issue-close.sh` could not detect the failure confirmed on `issue-comment.sh`, because it tests nothing at all. Whether it *would* fail is unmeasured — and whatever is learned chasing #976 transfers directly, since the inputs are identical. ## Required 1. Invert `issue-close.sh`: make the checked path the default, or add a readback (does the issue now read `state=closed`?) and condition the success line on it. 2. Add a readback to `pr-merge.sh` asserting `merged=true`, not merely a 2xx. 3. Apply `pr-review.sh`'s existing pattern across the family. 4. A needle per wrapper: stub `tea` to exit 0 without effect — each wrapper must fail. ## Latent, unrelated Reconstructing `OWNER/REPO` by splitting on the first and last slash **mangles any slug with more than two segments**. Not currently exercised; flagged only.
Ghost commented 2026-07-31 02:35:16 +00:00
Copy Link
Copy Source

⚠️ AMENDMENT — the headline claim in this issue is WRONG. Retracting it.

Root cause is now established (see #976): tea issue comment is a phantom subcommand — tea falls through to tea issue list, exits 0, no write attempted.

RETRACTED: "issue-close.sh is #976 line for line"

It is not, and the difference matters:

Call Status
tea issue comment … PHANTOM — no such subcommand; silently lists and exits 0
tea issue close … REAL — close is a genuine tea issue subcommand

So the close itself is NOT defeated by this mechanism. The Gate-5 "issue closed" attestation is not silently failing for the #976 reason. This issue as originally filed implies that it is. That was the strongest claim here and it is the one that is wrong.

(Also checked the pr group: merge, reject, approve all exist — no phantom there either.)

What stands, unchanged

  1. tea issue close is still unconditioned, with an unconditional echo "Closed Gitea issue #N". It cannot detect a real close failure. That was always a detection-capability claim and it stands exactly as written.
  2. The inversion stands — the fail-closed curl -fsS path is the fallback, reached only when login resolution fails. The blind path is the default.
  3. pr-merge.sh is status-checked. Unchanged.

🔴 NEW — and worse than what it replaces

issue-close.sh accepts a --comment and passes it to the phantom.

So the closing comment is silently discarded while the script reports success.

The closure comment is frequently where the completion evidence lives — the "closed by PR #N, CI green, review CLEAR" rationale. That evidence has been evaporating on every closure that used --comment, with a success line printed over it. The issue closes; the reason does not.

Net change to this filing

One claim retracted, one unchanged, one new defect added, and the root cause moved from three open candidates to one measured fact with a one-line fix.

How the wrong claim happened — the failure mode is reusable

The shape analysis (unconditioned call, unconditional echo) was correct. The question never asked was whether the command exists. tea issue comment was treated as given because it is spelled like every other verb in the file, and because a pattern already named was being hunted.

What caught it was not care or re-reading — it was a semantic search run for an unrelated reason surfacing another agent's two-week-old capture of the sibling case. Rules do not stop this class; controls do.

## ⚠️ AMENDMENT — the headline claim in this issue is WRONG. Retracting it. Root cause is now established (see #976): **`tea issue comment` is a phantom subcommand** — tea falls through to `tea issue list`, exits 0, no write attempted. ### RETRACTED: "issue-close.sh is #976 line for line" **It is not, and the difference matters:** | Call | Status | |---|---| | `tea issue comment …` | **PHANTOM** — no such subcommand; silently lists and exits 0 | | `tea issue close …` | **REAL** — `close` is a genuine `tea issue` subcommand | **So the close itself is NOT defeated by this mechanism.** The Gate-5 "issue closed" attestation is **not** silently failing for the #976 reason. This issue as originally filed implies that it is. That was the strongest claim here and it is the one that is wrong. *(Also checked the `pr` group: `merge`, `reject`, `approve` all exist — no phantom there either.)* ### What stands, unchanged 1. **`tea issue close` is still unconditioned**, with an unconditional `echo "Closed Gitea issue #N"`. It cannot **detect** a real close failure. That was always a detection-capability claim and it stands exactly as written. 2. **The inversion stands** — the fail-closed `curl -fsS` path is the **fallback**, reached only when login resolution fails. The blind path is the default. 3. **`pr-merge.sh` is status-checked.** Unchanged. ### 🔴 NEW — and worse than what it replaces **`issue-close.sh` accepts a `--comment` and passes it to the phantom.** So **the closing comment is silently discarded while the script reports success.** The closure comment is frequently where the completion evidence lives — the *"closed by PR #N, CI green, review CLEAR"* rationale. **That evidence has been evaporating on every closure that used `--comment`, with a success line printed over it.** The issue closes; the *reason* does not. ### Net change to this filing One claim **retracted**, one **unchanged**, one **new defect added**, and the root cause moved from three open candidates to one measured fact with a one-line fix. ### How the wrong claim happened — the failure mode is reusable The shape analysis (unconditioned call, unconditional echo) was correct. **The question never asked was whether the command exists.** `tea issue comment` was treated as given because it is spelled like every other verb in the file, and because a pattern already named was being hunted. What caught it was **not** care or re-reading — it was a semantic search run for an unrelated reason surfacing another agent's two-week-old capture of the sibling case. *Rules do not stop this class; controls do.*
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
code-be-01 (Mosaic fleet seat code-be-01) code-be-02 (Mosaic fleet seat code-be-02) code-dogfood-01 (Mosaic fleet seat code-dogfood-01) code-infra-01 (Mosaic fleet seat code-infra-01) darkwing (Mosaic fleet seat darkwing) dewey (Mosaic fleet seat dewey) fargo filbert (Mosaic fleet seat filbert) fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) marcie merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) orch-01 (Mosaic fleet seat orch-01) pepper resume rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) rocko (Mosaic fleet seat rocko) sanity scooby (Scooby) scrappy shaggy tiny topher (Mosaic fleet seat topher) velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#977