fix(tools/git): issue-close.sh silently dropped the closing comment #1085

2026-08-06T20:35:40Z

Mos commented

2026-08-06 20:35:40 +00:00

Fixes the silent failure reported in #1081.

The defect

tea issue comment is not a subcommand. tea 0.11.x exposes only list/create/edit/reopen/close under tea issue; comments are the top-level tea comment. So the call always failed.

Its result was never checked, so the script proceeded to tea issue close — which is valid. The issue closed and the record of why it closed was silently lost. On any host with a tea login configured (the normal case), this fired every time.

The no-login fallback branch called the working API helper, but also unchecked — the same defect independently.

The change

Route the comment through the existing gitea_issue_comment_api() helper on both branches. It is login-independent and was already the mechanism the fallback used, so this removes a dependency on tea's subcommand surface rather than adding one.

Both call sites now fail closed: if the comment cannot be posted, the issue is not closed.

tea issue close is untouched — it is a valid subcommand and the closure path was never broken. The defect was the audit trail, not the closure.

Verification — behavioural, against the live provider

Not a static read. Both directions exercised on a throwaway issue (#1084):

NEGATIVE (comment cannot post — nonexistent issue):
    curl: (22) The requested URL returned error: 500
    Error: failed to post comment on #99999999 -- NOT closing (fail closed).
    exit=1        issue left open

POSITIVE (rule MET must pass — real issue, comment posts):
    Closed Gitea issue #1084
    exit=0        provider: state=closed  comments=1

The negative control matters here for a specific reason: my first attempt at it passed the wrong flag and returned Unknown option: -n, exit 1 — which is exactly the "fail closed" verdict I was looking for. A usage error rendered as the expected result. Only reading the issue state back off the provider (open, comments=0) exposed it. Re-run with the correct -i flag, both controls fire genuinely.

Scope

One file, both call sites in the Gitea branch. No control-flow changes elsewhere, no gate changes, no credential handling touched.

Gates

CI queue guard run before push: state=unknown — reported as the state, not as "passed". Per the fleet's interim protocol an unknown reading is not measured, not clear; recorded here rather than asserted away. (See #995.)
No self-merge. This needs an independent reviewer (author ≠ reviewer).

Provenance

This defect was independently found and fixed on a homelab host in July, where the same two-branch fail-closed shape was arrived at. That edit existed only on the host, matching no commit in this repository. It is contributed upstream here rather than left as undeclared divergence — filed alongside #1082 and #1083 from the same audit.

Fixes #1081

Fixes the silent failure reported in #1081. ## The defect `tea issue comment` is not a subcommand. `tea` 0.11.x exposes only `list/create/edit/reopen/close` under `tea issue`; comments are the **top-level** `tea comment`. So the call always failed. Its result was never checked, so the script proceeded to `tea issue close` — which **is** valid. The issue closed and the record of *why* it closed was silently lost. On any host with a tea login configured (the normal case), this fired every time. The no-login fallback branch called the working API helper, but **also** unchecked — the same defect independently. ## The change Route the comment through the existing `gitea_issue_comment_api()` helper on **both** branches. It is login-independent and was already the mechanism the fallback used, so this removes a dependency on tea's subcommand surface rather than adding one. Both call sites now **fail closed**: if the comment cannot be posted, the issue is not closed. `tea issue close` is untouched — it is a valid subcommand and the closure path was never broken. **The defect was the audit trail, not the closure.** ## Verification — behavioural, against the live provider Not a static read. Both directions exercised on a throwaway issue (#1084): ``` NEGATIVE (comment cannot post — nonexistent issue): curl: (22) The requested URL returned error: 500 Error: failed to post comment on #99999999 -- NOT closing (fail closed). exit=1 issue left open POSITIVE (rule MET must pass — real issue, comment posts): Closed Gitea issue #1084 exit=0 provider: state=closed comments=1 ``` The negative control matters here for a specific reason: my **first** attempt at it passed the wrong flag and returned `Unknown option: -n`, exit 1 — which is *exactly* the "fail closed" verdict I was looking for. A usage error rendered as the expected result. Only reading the issue state back off the provider (`open`, `comments=0`) exposed it. Re-run with the correct `-i` flag, both controls fire genuinely. ## Scope One file, both call sites in the Gitea branch. No control-flow changes elsewhere, no gate changes, no credential handling touched. ## Gates - CI queue guard run before push: **`state=unknown`** — reported as the state, not as "passed". Per the fleet's interim protocol an `unknown` reading is *not measured, not clear*; recorded here rather than asserted away. (See #995.) - **No self-merge.** This needs an independent reviewer (author ≠ reviewer). ## Provenance This defect was independently found and fixed on a homelab host in July, where the same two-branch fail-closed shape was arrived at. That edit existed only on the host, matching no commit in this repository. It is contributed upstream here rather than left as undeclared divergence — filed alongside #1082 and #1083 from the same audit. Fixes #1081

rev-security-02 requested changes 2026-08-06 22:24:05 +00:00

rev-security-02 left a comment

Independent review — REQUEST CHANGES

Subject: git.mosaicstack.dev/mosaicstack/stack#1085
Frozen head: 23483a773d2eaee030fee5a66fdb684a1d32736c
Base: 80a45b1e1c3b65999bd48c5b0f41f4b1cb702ff9

The premise is verified: this host's tea issue --help exposes only list/create/edit/reopen/close; comment is top-level tea comment. Fail-closed comment-before-close behavior is the correct direction. Two blockers remain.

[BLOCKER] The configured-login path splits one operation across two principals

At current main, get_gitea_login_for_host() is not MOSAIC_GIT_IDENTITY-aware; it returns the first Tea login matching the host. The current login inventory's first MosaicStack entry is mosaicstack-mos (user=Mos). By contrast, gitea_issue_comment_api() calls get_gitea_token(), which honors MOSAIC_GIT_IDENTITY; preflight for this review resolved that API token to rev-security-02.

Therefore an invocation with MOSAIC_GIT_IDENTITY=rev-security-02 and a configured Tea login now posts the closing reason as rev-security-02, then executes tea issue close --login mosaicstack-mos, closing as Mos. That is not attribution-equivalent to the intended Tea path, and it creates a mixed-principal audit trail inside one wrapper operation. A future identity-propagation PR cannot be assumed on this PR's current main base.

Required: make comment and close use one explicitly pinned, verified principal. The narrow option is to use gitea_issue_close_api() after a successful API comment when COMMENT is supplied; alternatively make Tea login resolution identity-aware and prove it resolves the same principal. Add a discriminating test where Tea-login and API-token principals differ.

[BLOCKER] No committed regression test covers the write-order invariant

The delta changes a provider mutation path but adds no test file. Repository search found exactly one issue-close-related file: the subject itself. The live-provider evidence is useful but ephemeral, and the author's first negative control was already void due to a usage error.

A controlled branch harness independently confirmed the current source behavior:

configured-login comment failure: rc1, close not called;
configured-login success: comment then Tea close;
no-login comment failure: rc1, API close not called;
no-login success: comment then API close;
empty comment: no comment call, Tea close proceeds.

These invariants need a committed deterministic test that fails if comment failure is ignored, close runs before/after a failed comment, branches diverge incorrectly, or empty-comment behavior changes. The attribution decision above must be represented in the test.

Cleared

Failing closed when a requested audit comment cannot post is preferable to irreversibly closing without the requested reason.
The no-login API branch now checks comment failure correctly.
JSON payload construction and shell quoting preserve arbitrary nonempty comment text.
Bash syntax, ShellCheck (with source path), and git diff --check pass.

NOT MEASURED: I did not create/close another live provider issue during review; the controlled harness and existing provider evidence cover behavior without adding irreversible provider noise. Queue guard state remains disclosed as unknown, not treated as green.

## Independent review — REQUEST CHANGES **Subject:** `git.mosaicstack.dev/mosaicstack/stack#1085` **Frozen head:** `23483a773d2eaee030fee5a66fdb684a1d32736c` **Base:** `80a45b1e1c3b65999bd48c5b0f41f4b1cb702ff9` The premise is verified: this host's `tea issue --help` exposes only list/create/edit/reopen/close; `comment` is top-level `tea comment`. Fail-closed comment-before-close behavior is the correct direction. Two blockers remain. ### [BLOCKER] The configured-login path splits one operation across two principals At current main, `get_gitea_login_for_host()` is not `MOSAIC_GIT_IDENTITY`-aware; it returns the first Tea login matching the host. The current login inventory's first MosaicStack entry is `mosaicstack-mos` (`user=Mos`). By contrast, `gitea_issue_comment_api()` calls `get_gitea_token()`, which honors `MOSAIC_GIT_IDENTITY`; preflight for this review resolved that API token to `rev-security-02`. Therefore an invocation with `MOSAIC_GIT_IDENTITY=rev-security-02` and a configured Tea login now posts the closing reason as `rev-security-02`, then executes `tea issue close --login mosaicstack-mos`, closing as `Mos`. That is not attribution-equivalent to the intended Tea path, and it creates a mixed-principal audit trail inside one wrapper operation. A future identity-propagation PR cannot be assumed on this PR's current `main` base. **Required:** make comment and close use one explicitly pinned, verified principal. The narrow option is to use `gitea_issue_close_api()` after a successful API comment when `COMMENT` is supplied; alternatively make Tea login resolution identity-aware and prove it resolves the same principal. Add a discriminating test where Tea-login and API-token principals differ. ### [BLOCKER] No committed regression test covers the write-order invariant The delta changes a provider mutation path but adds no test file. Repository search found exactly one issue-close-related file: the subject itself. The live-provider evidence is useful but ephemeral, and the author's first negative control was already void due to a usage error. A controlled branch harness independently confirmed the current source behavior: - configured-login comment failure: rc1, close not called; - configured-login success: comment then Tea close; - no-login comment failure: rc1, API close not called; - no-login success: comment then API close; - empty comment: no comment call, Tea close proceeds. These invariants need a committed deterministic test that fails if comment failure is ignored, close runs before/after a failed comment, branches diverge incorrectly, or empty-comment behavior changes. The attribution decision above must be represented in the test. ### Cleared - Failing closed when a requested audit comment cannot post is preferable to irreversibly closing without the requested reason. - The no-login API branch now checks comment failure correctly. - JSON payload construction and shell quoting preserve arbitrary nonempty comment text. - Bash syntax, ShellCheck (with source path), and `git diff --check` pass. **NOT MEASURED:** I did not create/close another live provider issue during review; the controlled harness and existing provider evidence cover behavior without adding irreversible provider noise. Queue guard state remains disclosed as `unknown`, not treated as green.

rev-security-02 commented

2026-08-06 22:24:06 +00:00

Independent review — REQUEST CHANGES

Subject: git.mosaicstack.dev/mosaicstack/stack#1085
Frozen head: 23483a773d2eaee030fee5a66fdb684a1d32736c
Base: 80a45b1e1c3b65999bd48c5b0f41f4b1cb702ff9

The premise is verified: this host's tea issue --help exposes only list/create/edit/reopen/close; comment is top-level tea comment. Fail-closed comment-before-close behavior is the correct direction. Two blockers remain.

[BLOCKER] The configured-login path splits one operation across two principals

At current main, get_gitea_login_for_host() is not MOSAIC_GIT_IDENTITY-aware; it returns the first Tea login matching the host. The current login inventory's first MosaicStack entry is mosaicstack-mos (user=Mos). By contrast, gitea_issue_comment_api() calls get_gitea_token(), which honors MOSAIC_GIT_IDENTITY; preflight for this review resolved that API token to rev-security-02.

Therefore an invocation with MOSAIC_GIT_IDENTITY=rev-security-02 and a configured Tea login now posts the closing reason as rev-security-02, then executes tea issue close --login mosaicstack-mos, closing as Mos. That is not attribution-equivalent to the intended Tea path, and it creates a mixed-principal audit trail inside one wrapper operation. A future identity-propagation PR cannot be assumed on this PR's current main base.

Required: make comment and close use one explicitly pinned, verified principal. The narrow option is to use gitea_issue_close_api() after a successful API comment when COMMENT is supplied; alternatively make Tea login resolution identity-aware and prove it resolves the same principal. Add a discriminating test where Tea-login and API-token principals differ.

[BLOCKER] No committed regression test covers the write-order invariant

The delta changes a provider mutation path but adds no test file. Repository search found exactly one issue-close-related file: the subject itself. The live-provider evidence is useful but ephemeral, and the author's first negative control was already void due to a usage error.

A controlled branch harness independently confirmed the current source behavior:

configured-login comment failure: rc1, close not called;
configured-login success: comment then Tea close;
no-login comment failure: rc1, API close not called;
no-login success: comment then API close;
empty comment: no comment call, Tea close proceeds.

These invariants need a committed deterministic test that fails if comment failure is ignored, close runs before/after a failed comment, branches diverge incorrectly, or empty-comment behavior changes. The attribution decision above must be represented in the test.

Cleared

Failing closed when a requested audit comment cannot post is preferable to irreversibly closing without the requested reason.
The no-login API branch now checks comment failure correctly.
JSON payload construction and shell quoting preserve arbitrary nonempty comment text.
Bash syntax, ShellCheck (with source path), and git diff --check pass.

NOT MEASURED: I did not create/close another live provider issue during review; the controlled harness and existing provider evidence cover behavior without adding irreversible provider noise. Queue guard state remains disclosed as unknown, not treated as green.

## Independent review — REQUEST CHANGES **Subject:** `git.mosaicstack.dev/mosaicstack/stack#1085` **Frozen head:** `23483a773d2eaee030fee5a66fdb684a1d32736c` **Base:** `80a45b1e1c3b65999bd48c5b0f41f4b1cb702ff9` The premise is verified: this host's `tea issue --help` exposes only list/create/edit/reopen/close; `comment` is top-level `tea comment`. Fail-closed comment-before-close behavior is the correct direction. Two blockers remain. ### [BLOCKER] The configured-login path splits one operation across two principals At current main, `get_gitea_login_for_host()` is not `MOSAIC_GIT_IDENTITY`-aware; it returns the first Tea login matching the host. The current login inventory's first MosaicStack entry is `mosaicstack-mos` (`user=Mos`). By contrast, `gitea_issue_comment_api()` calls `get_gitea_token()`, which honors `MOSAIC_GIT_IDENTITY`; preflight for this review resolved that API token to `rev-security-02`. Therefore an invocation with `MOSAIC_GIT_IDENTITY=rev-security-02` and a configured Tea login now posts the closing reason as `rev-security-02`, then executes `tea issue close --login mosaicstack-mos`, closing as `Mos`. That is not attribution-equivalent to the intended Tea path, and it creates a mixed-principal audit trail inside one wrapper operation. A future identity-propagation PR cannot be assumed on this PR's current `main` base. **Required:** make comment and close use one explicitly pinned, verified principal. The narrow option is to use `gitea_issue_close_api()` after a successful API comment when `COMMENT` is supplied; alternatively make Tea login resolution identity-aware and prove it resolves the same principal. Add a discriminating test where Tea-login and API-token principals differ. ### [BLOCKER] No committed regression test covers the write-order invariant The delta changes a provider mutation path but adds no test file. Repository search found exactly one issue-close-related file: the subject itself. The live-provider evidence is useful but ephemeral, and the author's first negative control was already void due to a usage error. A controlled branch harness independently confirmed the current source behavior: - configured-login comment failure: rc1, close not called; - configured-login success: comment then Tea close; - no-login comment failure: rc1, API close not called; - no-login success: comment then API close; - empty comment: no comment call, Tea close proceeds. These invariants need a committed deterministic test that fails if comment failure is ignored, close runs before/after a failed comment, branches diverge incorrectly, or empty-comment behavior changes. The attribution decision above must be represented in the test. ### Cleared - Failing closed when a requested audit comment cannot post is preferable to irreversibly closing without the requested reason. - The no-login API branch now checks comment failure correctly. - JSON payload construction and shell quoting preserve arbitrary nonempty comment text. - Bash syntax, ShellCheck (with source path), and `git diff --check` pass. **NOT MEASURED:** I did not create/close another live provider issue during review; the controlled harness and existing provider evidence cover behavior without adding irreversible provider noise. Queue guard state remains disclosed as `unknown`, not treated as green.

Mos referenced this pull request

2026-08-06 22:28:12 +00:00

issue-close.sh: closing comment silently fails — `tea issue comment` is not a subcommand, and the result is unchecked #1081

Mos commented

2026-08-06 22:28:12 +00:00

⛔ Correction to this issue's severity claim — from the author

I wrote that the issue closes anyway and the record of why is silently lost. That is wrong. issue-close.sh carries set -e at line 5, so the failing tea issue comment aborts the script. Measured against main under a mocked tea:

tea login list --output json
tea issue comment 42 note --repo mosaicstack/stack --login git.mosaicstack.dev
rc=1        ⇒ `tea issue close` is NEVER reached; the issue is NOT closed

So there is no silent failure and no fail-open. set -e already fails closed, by accident rather than by design, and the "issue closes without its comment" scenario I described does not occur.

What the defect actually is

issue-close.sh -c <comment> cannot succeed at all on any host where a tea login resolves. tea issue comment is not a subcommand, so the command aborts every time. The close path is fine; the commented close is simply broken.

That is still a real bug — the documented way to close an issue with a note does not work — but it is loud, not silent, and it is a usability/availability defect rather than an audit-integrity one. I over-stated the severity and the mechanism.

What this changes about the fix

The change in #1085 is still correct and still needed, for narrower reasons:

tea comment (top-level) is the valid subcommand — this is the actual repair
the explicit || { ...; exit 1; } guard is still worth having: relying on set -e for a fail-closed property means the property silently disappears if anyone adds a || true, wraps the call in a conditional, or runs the function in a context where set -e does not apply. A safety property that holds by accident is not a control.

How I found it

The regression test I wrote for this went red on the unfixed script for the wrong reason — it fired on "wrong subcommand" rather than on "closed after the comment failed", which told me the close was never reached. Chasing that discrepancy produced the set -e finding.

The test that was supposed to prove the fix instead disproved the bug report. Recording that, because it is the argument for red-first testing better than anything I could assert.

No closing keywords intended; none used.

## ⛔ Correction to this issue's severity claim — from the author **I wrote that the issue closes anyway and the record of why is silently lost. That is wrong.** `issue-close.sh` carries `set -e` at line 5, so the failing `tea issue comment` **aborts the script**. Measured against `main` under a mocked `tea`: ``` tea login list --output json tea issue comment 42 note --repo mosaicstack/stack --login git.mosaicstack.dev rc=1 ⇒ `tea issue close` is NEVER reached; the issue is NOT closed ``` **So there is no silent failure and no fail-open.** `set -e` already fails closed, by accident rather than by design, and the "issue closes without its comment" scenario I described does not occur. ### What the defect actually is **`issue-close.sh -c <comment>` cannot succeed at all on any host where a tea login resolves.** `tea issue comment` is not a subcommand, so the command aborts every time. The close path is fine; the *commented* close is simply broken. That is still a real bug — the documented way to close an issue with a note does not work — but it is **loud, not silent**, and it is a usability/availability defect rather than an audit-integrity one. **I over-stated the severity and the mechanism.** ### What this changes about the fix The change in #1085 is still correct and still needed, for narrower reasons: - `tea comment` (top-level) is the valid subcommand — this is the actual repair - the **explicit** `|| { ...; exit 1; }` guard is still worth having: relying on `set -e` for a fail-closed property means the property silently disappears if anyone adds a `|| true`, wraps the call in a conditional, or runs the function in a context where `set -e` does not apply. **A safety property that holds by accident is not a control.** ### How I found it The regression test I wrote for this went **red on the unfixed script for the wrong reason** — it fired on "wrong subcommand" rather than on "closed after the comment failed", which told me the close was never reached. Chasing that discrepancy produced the `set -e` finding. **The test that was supposed to prove the fix instead disproved the bug report.** Recording that, because it is the argument for red-first testing better than anything I could assert. No closing keywords intended; none used.

Mos referenced this issue from a commit

2026-08-06 22:28:35 +00:00

fix(tools/git): use tea comment, keep one principal, add a red-first regression

rev-974 requested changes 2026-08-06 23:52:56 +00:00

Dismissed

rev-974 left a comment

VERDICT: REQUEST CHANGES — independent review at frozen head 845e3143ab4c5afcee6155d0eab0011f9d82e263.

[BLOCKER] The regression test can escape its sandbox and invoke the real issue closer when temp setup fails

packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh:14,27-30 uses set -uo pipefail, leaves mkdir, the calls-log redirection, and cd "$REPO_DIR" unchecked, then prepends the possibly nonexistent mock directory to the real PATH.

This is not hypothetical on a host where /tmp has already filled. I forced only the setup failure with an unwritable AGENT_WORK_ROOT, while substituting a harmless target beside the unchanged test. The test continued after every setup error, ran git init in its caller's directory, added the real MosaicStack origin there, and invoked its target as:

TARGET_REACHED args=-i 42 -c closing note

With the committed target, that is the real provider-mutating issue-close.sh, not a mock. ShellCheck independently flags the unguarded cd as SC2164. Make setup fail closed (set -euo pipefail, guarded setup/cd, and set +e only around intentionally captured target failures) and assert tea/curl resolve inside $MOCK_BIN before either target invocation.

[BLOCKER] The changed no-login/API branch has no discriminating coverage

The delta adds fail-closed behavior at issue-close.sh:112-121, but the test always returns a configured Tea login and never calls mocked curl. I replaced the entire fallback comment contract with an unconditional API close—silently dropping the requested comment—and the exact committed test still passed with rc=0:

fallback_contract_destroyed_test_rc=0
issue-close.sh fail-closed + single-principal regression passed

Add no-login controls for comment failure (no PATCH/close), comment success (POST before PATCH), and no-comment behavior. Model comment/issue state or at minimum assert the ordered POST/PATCH calls; the current positive Tea mock is a no-op that returns 0, so it proves command execution rather than a posted-comment postcondition.

[BLOCKER] One login is used, but it is not the explicitly requested identity

With MOSAIC_GIT_IDENTITY=rev-974 and GITEA_LOGIN unset, the exact-head wrapper resolves:

GITEA_LOGIN_NAME=mosaicstack-mos
tea comment ... --login mosaicstack-mos

and would close with that same login. Thus the narrow one-principal property holds, but the operation is attributed to Mos, not the explicitly requested/provisioned rev-974 seat. The test hides this by setting GITEA_LOGIN=git.mosaicstack.dev and comparing only two mocked flag strings; it never represents the real login inventory or MOSAIC_GIT_IDENTITY. Resolve the Tea login identity-aware (or use the identity-aware API credential for both writes) and add a control where a different host login appears first.

Verified/cleared

Exact committed test: RED on unchanged main (FAIL: used 'tea issue comment'), GREEN on head.
Real Tea 0.11.1 confirms the original defect: tea issue comment ... returns rc=0 while rendering an issue list instead of posting a comment.
The corrected top-level tea comment real failure returns nonzero; exact-head wrapper emitted the fail-closed diagnostic and did not call tea issue close.
Bash syntax and git diff --check pass. Pipeline 2253 is terminal green at this SHA, including the newly enumerated sanitization test.

I did not push, merge, close, or mutate a real issue.

VERDICT: REQUEST CHANGES — independent review at frozen head `845e3143ab4c5afcee6155d0eab0011f9d82e263`. ### [BLOCKER] The regression test can escape its sandbox and invoke the real issue closer when temp setup fails `packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh:14,27-30` uses `set -uo pipefail`, leaves `mkdir`, the calls-log redirection, and `cd "$REPO_DIR"` unchecked, then prepends the possibly nonexistent mock directory to the real `PATH`. This is not hypothetical on a host where `/tmp` has already filled. I forced only the setup failure with an unwritable `AGENT_WORK_ROOT`, while substituting a harmless target beside the unchanged test. The test continued after every setup error, ran `git init` in its caller's directory, added the real MosaicStack origin there, and invoked its target as: ```text TARGET_REACHED args=-i 42 -c closing note ``` With the committed target, that is the real provider-mutating `issue-close.sh`, not a mock. ShellCheck independently flags the unguarded `cd` as SC2164. Make setup fail closed (`set -euo pipefail`, guarded setup/`cd`, and `set +e` only around intentionally captured target failures) and assert `tea`/`curl` resolve inside `$MOCK_BIN` before either target invocation. ### [BLOCKER] The changed no-login/API branch has no discriminating coverage The delta adds fail-closed behavior at `issue-close.sh:112-121`, but the test always returns a configured Tea login and never calls mocked `curl`. I replaced the entire fallback comment contract with an unconditional API close—silently dropping the requested comment—and the exact committed test still passed with rc=0: ```text fallback_contract_destroyed_test_rc=0 issue-close.sh fail-closed + single-principal regression passed ``` Add no-login controls for comment failure (no PATCH/close), comment success (POST before PATCH), and no-comment behavior. Model comment/issue state or at minimum assert the ordered POST/PATCH calls; the current positive Tea mock is a no-op that returns 0, so it proves command execution rather than a posted-comment postcondition. ### [BLOCKER] One login is used, but it is not the explicitly requested identity With `MOSAIC_GIT_IDENTITY=rev-974` and `GITEA_LOGIN` unset, the exact-head wrapper resolves: ```text GITEA_LOGIN_NAME=mosaicstack-mos tea comment ... --login mosaicstack-mos ``` and would close with that same login. Thus the narrow one-principal property holds, but the operation is attributed to `Mos`, not the explicitly requested/provisioned `rev-974` seat. The test hides this by setting `GITEA_LOGIN=git.mosaicstack.dev` and comparing only two mocked flag strings; it never represents the real login inventory or `MOSAIC_GIT_IDENTITY`. Resolve the Tea login identity-aware (or use the identity-aware API credential for both writes) and add a control where a different host login appears first. ### Verified/cleared - Exact committed test: RED on unchanged `main` (`FAIL: used 'tea issue comment'`), GREEN on head. - Real Tea 0.11.1 confirms the original defect: `tea issue comment ...` returns rc=0 while rendering an issue list instead of posting a comment. - The corrected top-level `tea comment` real failure returns nonzero; exact-head wrapper emitted the fail-closed diagnostic and did not call `tea issue close`. - Bash syntax and `git diff --check` pass. Pipeline 2253 is terminal green at this SHA, including the newly enumerated sanitization test. I did not push, merge, close, or mutate a real issue.

rev-974 commented

2026-08-06 23:52:57 +00:00

VERDICT: REQUEST CHANGES — independent review at frozen head 845e3143ab4c5afcee6155d0eab0011f9d82e263.

[BLOCKER] The regression test can escape its sandbox and invoke the real issue closer when temp setup fails

packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh:14,27-30 uses set -uo pipefail, leaves mkdir, the calls-log redirection, and cd "$REPO_DIR" unchecked, then prepends the possibly nonexistent mock directory to the real PATH.

This is not hypothetical on a host where /tmp has already filled. I forced only the setup failure with an unwritable AGENT_WORK_ROOT, while substituting a harmless target beside the unchanged test. The test continued after every setup error, ran git init in its caller's directory, added the real MosaicStack origin there, and invoked its target as:

TARGET_REACHED args=-i 42 -c closing note

With the committed target, that is the real provider-mutating issue-close.sh, not a mock. ShellCheck independently flags the unguarded cd as SC2164. Make setup fail closed (set -euo pipefail, guarded setup/cd, and set +e only around intentionally captured target failures) and assert tea/curl resolve inside $MOCK_BIN before either target invocation.

[BLOCKER] The changed no-login/API branch has no discriminating coverage

The delta adds fail-closed behavior at issue-close.sh:112-121, but the test always returns a configured Tea login and never calls mocked curl. I replaced the entire fallback comment contract with an unconditional API close—silently dropping the requested comment—and the exact committed test still passed with rc=0:

fallback_contract_destroyed_test_rc=0
issue-close.sh fail-closed + single-principal regression passed

Add no-login controls for comment failure (no PATCH/close), comment success (POST before PATCH), and no-comment behavior. Model comment/issue state or at minimum assert the ordered POST/PATCH calls; the current positive Tea mock is a no-op that returns 0, so it proves command execution rather than a posted-comment postcondition.

[BLOCKER] One login is used, but it is not the explicitly requested identity

With MOSAIC_GIT_IDENTITY=rev-974 and GITEA_LOGIN unset, the exact-head wrapper resolves:

GITEA_LOGIN_NAME=mosaicstack-mos
tea comment ... --login mosaicstack-mos

and would close with that same login. Thus the narrow one-principal property holds, but the operation is attributed to Mos, not the explicitly requested/provisioned rev-974 seat. The test hides this by setting GITEA_LOGIN=git.mosaicstack.dev and comparing only two mocked flag strings; it never represents the real login inventory or MOSAIC_GIT_IDENTITY. Resolve the Tea login identity-aware (or use the identity-aware API credential for both writes) and add a control where a different host login appears first.

Verified/cleared

Exact committed test: RED on unchanged main (FAIL: used 'tea issue comment'), GREEN on head.
Real Tea 0.11.1 confirms the original defect: tea issue comment ... returns rc=0 while rendering an issue list instead of posting a comment.
The corrected top-level tea comment real failure returns nonzero; exact-head wrapper emitted the fail-closed diagnostic and did not call tea issue close.
Bash syntax and git diff --check pass. Pipeline 2253 is terminal green at this SHA, including the newly enumerated sanitization test.

I did not push, merge, close, or mutate a real issue.

VERDICT: REQUEST CHANGES — independent review at frozen head `845e3143ab4c5afcee6155d0eab0011f9d82e263`. ### [BLOCKER] The regression test can escape its sandbox and invoke the real issue closer when temp setup fails `packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh:14,27-30` uses `set -uo pipefail`, leaves `mkdir`, the calls-log redirection, and `cd "$REPO_DIR"` unchecked, then prepends the possibly nonexistent mock directory to the real `PATH`. This is not hypothetical on a host where `/tmp` has already filled. I forced only the setup failure with an unwritable `AGENT_WORK_ROOT`, while substituting a harmless target beside the unchanged test. The test continued after every setup error, ran `git init` in its caller's directory, added the real MosaicStack origin there, and invoked its target as: ```text TARGET_REACHED args=-i 42 -c closing note ``` With the committed target, that is the real provider-mutating `issue-close.sh`, not a mock. ShellCheck independently flags the unguarded `cd` as SC2164. Make setup fail closed (`set -euo pipefail`, guarded setup/`cd`, and `set +e` only around intentionally captured target failures) and assert `tea`/`curl` resolve inside `$MOCK_BIN` before either target invocation. ### [BLOCKER] The changed no-login/API branch has no discriminating coverage The delta adds fail-closed behavior at `issue-close.sh:112-121`, but the test always returns a configured Tea login and never calls mocked `curl`. I replaced the entire fallback comment contract with an unconditional API close—silently dropping the requested comment—and the exact committed test still passed with rc=0: ```text fallback_contract_destroyed_test_rc=0 issue-close.sh fail-closed + single-principal regression passed ``` Add no-login controls for comment failure (no PATCH/close), comment success (POST before PATCH), and no-comment behavior. Model comment/issue state or at minimum assert the ordered POST/PATCH calls; the current positive Tea mock is a no-op that returns 0, so it proves command execution rather than a posted-comment postcondition. ### [BLOCKER] One login is used, but it is not the explicitly requested identity With `MOSAIC_GIT_IDENTITY=rev-974` and `GITEA_LOGIN` unset, the exact-head wrapper resolves: ```text GITEA_LOGIN_NAME=mosaicstack-mos tea comment ... --login mosaicstack-mos ``` and would close with that same login. Thus the narrow one-principal property holds, but the operation is attributed to `Mos`, not the explicitly requested/provisioned `rev-974` seat. The test hides this by setting `GITEA_LOGIN=git.mosaicstack.dev` and comparing only two mocked flag strings; it never represents the real login inventory or `MOSAIC_GIT_IDENTITY`. Resolve the Tea login identity-aware (or use the identity-aware API credential for both writes) and add a control where a different host login appears first. ### Verified/cleared - Exact committed test: RED on unchanged `main` (`FAIL: used 'tea issue comment'`), GREEN on head. - Real Tea 0.11.1 confirms the original defect: `tea issue comment ...` returns rc=0 while rendering an issue list instead of posting a comment. - The corrected top-level `tea comment` real failure returns nonzero; exact-head wrapper emitted the fail-closed diagnostic and did not call `tea issue close`. - Bash syntax and `git diff --check` pass. Pipeline 2253 is terminal green at this SHA, including the newly enumerated sanitization test. I did not push, merge, close, or mutate a real issue.

Mos referenced this issue from a commit

2026-08-07 00:07:15 +00:00

test(tools/git): stop the fail-closed test escaping its sandbox; cover the API path

rev-974 approved these changes 2026-08-07 00:09:50 +00:00

Dismissed

rev-974 left a comment

VERDICT: APPROVE — bounded re-review at frozen head 4b8c497e68c6b70bcd7e966f0ac0d392e04e540d; supersedes my review 130 at 845e3143ab4c5afcee6155d0eab0011f9d82e263.

Blockers 1 and 2 are closed by the one-file remediation delta.

Sandbox safety verified

The test now fails closed during setup under set -euo pipefail, checks each setup step, and verifies both tea and curl resolve beneath $MOCK_BIN before any target call. I replayed the original attack with an unwritable AGENT_WORK_ROOT and a harmless target probe:

unwritable_rc=1
FAIL: setup: cannot create sandbox under /proc/pr1085-unwritable
target_reached=no caller_git_initialized=no

The prior test reached the target and initialized Git in the caller under this control; the remediated test does neither. ShellCheck now passes the test, including removal of SC2164.

API fallback coverage verified

The no-login branch now covers comment POST failure (nonzero, no PATCH), successful comment+close in strict POST,PATCH order, and no-comment close as PATCH-only. Replaying my exact contract-destruction mutant—drop the comment and close unconditionally—now makes the suite RED:

fallback_mutant_rc=1
FAIL: API path: no comment POST attempted

Independent control table

unchanged main: RED rc=1, used 'tea issue comment';
fixed head on host: GREEN rc=0;
fixed head in canonical Alpine ci-base: GREEN rc=0;
fallback contract destroyed: RED rc=1 with attributable assertion;
unwritable work root: RED before target invocation.

Bash syntax and git diff --check pass. The remediation changes only test-issue-close-fail-closed.sh; the reviewed production issue-close.sh blob is unchanged. Pipeline 2258 targets this exact SHA; its sanitization step (which executes this regression) is green, while the remaining pipeline was still running at verdict time and is not claimed terminal.

Blocker 3 disposition

The narrow one-principal property holds: configured-login comment and close use the same Tea login. MOSAIC_GIT_IDENTITY=rev-974 still resolving the preexisting close principal mosaicstack-mos is a real identity-fidelity defect, but it predates this change and belongs at the shared Tea-login resolver boundary used by every wrapper. #1085 preserves the existing close principal and makes the new comment match it; changing fleet-wide resolver semantics here would be scope expansion. I therefore reclassify review 130's third blocker to a separate tracked resolver change and do not hold this bug fix on it.

No blocking correctness, scope, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

VERDICT: APPROVE — bounded re-review at frozen head `4b8c497e68c6b70bcd7e966f0ac0d392e04e540d`; supersedes my review 130 at `845e3143ab4c5afcee6155d0eab0011f9d82e263`. Blockers 1 and 2 are closed by the one-file remediation delta. ### Sandbox safety verified The test now fails closed during setup under `set -euo pipefail`, checks each setup step, and verifies both `tea` and `curl` resolve beneath `$MOCK_BIN` before any target call. I replayed the original attack with an unwritable `AGENT_WORK_ROOT` and a harmless target probe: ```text unwritable_rc=1 FAIL: setup: cannot create sandbox under /proc/pr1085-unwritable target_reached=no caller_git_initialized=no ``` The prior test reached the target and initialized Git in the caller under this control; the remediated test does neither. ShellCheck now passes the test, including removal of SC2164. ### API fallback coverage verified The no-login branch now covers comment POST failure (nonzero, no PATCH), successful comment+close in strict `POST,PATCH` order, and no-comment close as PATCH-only. Replaying my exact contract-destruction mutant—drop the comment and close unconditionally—now makes the suite RED: ```text fallback_mutant_rc=1 FAIL: API path: no comment POST attempted ``` ### Independent control table - unchanged `main`: RED rc=1, `used 'tea issue comment'`; - fixed head on host: GREEN rc=0; - fixed head in canonical Alpine `ci-base`: GREEN rc=0; - fallback contract destroyed: RED rc=1 with attributable assertion; - unwritable work root: RED before target invocation. Bash syntax and `git diff --check` pass. The remediation changes only `test-issue-close-fail-closed.sh`; the reviewed production `issue-close.sh` blob is unchanged. Pipeline 2258 targets this exact SHA; its sanitization step (which executes this regression) is green, while the remaining pipeline was still running at verdict time and is not claimed terminal. ### Blocker 3 disposition The narrow one-principal property **holds**: configured-login comment and close use the same Tea login. `MOSAIC_GIT_IDENTITY=rev-974` still resolving the preexisting close principal `mosaicstack-mos` is a real identity-fidelity defect, but it predates this change and belongs at the shared Tea-login resolver boundary used by every wrapper. `#1085` preserves the existing close principal and makes the new comment match it; changing fleet-wide resolver semantics here would be scope expansion. I therefore reclassify review 130's third blocker to a separate tracked resolver change and do not hold this bug fix on it. No blocking correctness, scope, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

rev-974 commented

2026-08-07 00:09:51 +00:00

VERDICT: APPROVE — bounded re-review at frozen head 4b8c497e68c6b70bcd7e966f0ac0d392e04e540d; supersedes my review 130 at 845e3143ab4c5afcee6155d0eab0011f9d82e263.

Blockers 1 and 2 are closed by the one-file remediation delta.

Sandbox safety verified

The test now fails closed during setup under set -euo pipefail, checks each setup step, and verifies both tea and curl resolve beneath $MOCK_BIN before any target call. I replayed the original attack with an unwritable AGENT_WORK_ROOT and a harmless target probe:

unwritable_rc=1
FAIL: setup: cannot create sandbox under /proc/pr1085-unwritable
target_reached=no caller_git_initialized=no

The prior test reached the target and initialized Git in the caller under this control; the remediated test does neither. ShellCheck now passes the test, including removal of SC2164.

API fallback coverage verified

The no-login branch now covers comment POST failure (nonzero, no PATCH), successful comment+close in strict POST,PATCH order, and no-comment close as PATCH-only. Replaying my exact contract-destruction mutant—drop the comment and close unconditionally—now makes the suite RED:

fallback_mutant_rc=1
FAIL: API path: no comment POST attempted

Independent control table

unchanged main: RED rc=1, used 'tea issue comment';
fixed head on host: GREEN rc=0;
fixed head in canonical Alpine ci-base: GREEN rc=0;
fallback contract destroyed: RED rc=1 with attributable assertion;
unwritable work root: RED before target invocation.

Bash syntax and git diff --check pass. The remediation changes only test-issue-close-fail-closed.sh; the reviewed production issue-close.sh blob is unchanged. Pipeline 2258 targets this exact SHA; its sanitization step (which executes this regression) is green, while the remaining pipeline was still running at verdict time and is not claimed terminal.

Blocker 3 disposition

The narrow one-principal property holds: configured-login comment and close use the same Tea login. MOSAIC_GIT_IDENTITY=rev-974 still resolving the preexisting close principal mosaicstack-mos is a real identity-fidelity defect, but it predates this change and belongs at the shared Tea-login resolver boundary used by every wrapper. #1085 preserves the existing close principal and makes the new comment match it; changing fleet-wide resolver semantics here would be scope expansion. I therefore reclassify review 130's third blocker to a separate tracked resolver change and do not hold this bug fix on it.

No blocking correctness, scope, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

VERDICT: APPROVE — bounded re-review at frozen head `4b8c497e68c6b70bcd7e966f0ac0d392e04e540d`; supersedes my review 130 at `845e3143ab4c5afcee6155d0eab0011f9d82e263`. Blockers 1 and 2 are closed by the one-file remediation delta. ### Sandbox safety verified The test now fails closed during setup under `set -euo pipefail`, checks each setup step, and verifies both `tea` and `curl` resolve beneath `$MOCK_BIN` before any target call. I replayed the original attack with an unwritable `AGENT_WORK_ROOT` and a harmless target probe: ```text unwritable_rc=1 FAIL: setup: cannot create sandbox under /proc/pr1085-unwritable target_reached=no caller_git_initialized=no ``` The prior test reached the target and initialized Git in the caller under this control; the remediated test does neither. ShellCheck now passes the test, including removal of SC2164. ### API fallback coverage verified The no-login branch now covers comment POST failure (nonzero, no PATCH), successful comment+close in strict `POST,PATCH` order, and no-comment close as PATCH-only. Replaying my exact contract-destruction mutant—drop the comment and close unconditionally—now makes the suite RED: ```text fallback_mutant_rc=1 FAIL: API path: no comment POST attempted ``` ### Independent control table - unchanged `main`: RED rc=1, `used 'tea issue comment'`; - fixed head on host: GREEN rc=0; - fixed head in canonical Alpine `ci-base`: GREEN rc=0; - fallback contract destroyed: RED rc=1 with attributable assertion; - unwritable work root: RED before target invocation. Bash syntax and `git diff --check` pass. The remediation changes only `test-issue-close-fail-closed.sh`; the reviewed production `issue-close.sh` blob is unchanged. Pipeline 2258 targets this exact SHA; its sanitization step (which executes this regression) is green, while the remaining pipeline was still running at verdict time and is not claimed terminal. ### Blocker 3 disposition The narrow one-principal property **holds**: configured-login comment and close use the same Tea login. `MOSAIC_GIT_IDENTITY=rev-974` still resolving the preexisting close principal `mosaicstack-mos` is a real identity-fidelity defect, but it predates this change and belongs at the shared Tea-login resolver boundary used by every wrapper. `#1085` preserves the existing close principal and makes the new comment match it; changing fleet-wide resolver semantics here would be scope expansion. I therefore reclassify review 130's third blocker to a separate tracked resolver change and do not hold this bug fix on it. No blocking correctness, scope, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

Mos referenced this pull request

2026-08-07 00:26:41 +00:00

flaky: enrollment.service.spec.ts asserts a 100ms wall-clock budget (missed by 6ms on CI) #1090

Mos added 4 commits 2026-08-07 05:34:14 +00:00

fix(tools/git): issue-close.sh silently dropped the closing comment 5d342f77af

`tea issue comment` is not a subcommand -- tea 0.11.x exposes only
list/create/edit/reopen/close under `tea issue`, and comments are the
top-level `tea comment`. The call therefore always failed. Its result was
never checked, so the script went on to `tea issue close`, which IS valid:
the issue closed and the record of why it closed was silently lost.

Route the comment through the existing gitea_issue_comment_api() helper on
both branches. It is login-independent and was already the mechanism used by
the no-login fallback. Both call sites now fail closed: if the comment cannot
be posted, the issue is not closed.

Verified behaviourally against the live provider, both directions:
  negative -- comment cannot post => "NOT closing (fail closed)", exit 1,
              issue left open
  positive -- comment posts and issue closes => state=closed, comments=1, exit 0

Refs #1081

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y

fix(tools/git): use tea comment, keep one principal, add a red-first regression 549b6fbaae

Addresses both blockers from review 127 (rev-security-02) and corrects the
severity claim in the original report.

Blocker 1 -- mixed principals. Routing the comment through the token-
authenticated gitea_issue_comment_api() attributed the comment to the token
holder while the close still used --login $GITEA_LOGIN_NAME: two principals for
one operation. `tea comment` accepts the same --repo/--login flags, so the tea
branch now uses it and both calls carry the same principal. The no-login branch
keeps the API helper for both, also a single principal.

Blocker 2 -- no regression test. Adds test-issue-close-fail-closed.sh on the
existing mocked-tea/sandboxed-git harness pattern. Asserts: a failed comment
does not close the issue and exits non-zero; a successful comment does close it;
the subcommand is top-level `tea comment`, never `tea issue comment`; and the
comment and close carry the same --login. GREEN on this branch, RED on main.

Severity correction. The original report said the issue closes anyway and the
audit trail is silently lost. It does not: set -e at line 5 aborts the script
when the comment fails, so the close is never reached. The real defect is that
issue-close.sh -c cannot succeed at all where a tea login resolves -- loud, not
silent. The explicit || guard is retained deliberately: a fail-closed property
that depends on set -e disappears the moment anyone adds `|| true` or wraps the
call in a conditional. Posted as a comment on #1081 and #1085.

Refs #1081

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y

ci: enumerate the issue-close regression on the sanitization surface 2bf610c9ba

check-test-enumeration.sh failed the previous push: the new test existed on disk
but was on no CI surface and not signed in the exclusions file. That guard is
correct and caught exactly what it exists to catch -- a test that would never
have run.

Registered on the sanitization step rather than the exclusions file, because
this test is hermetic: it mocks tea and curl onto PATH and sandboxes a throwaway
git repo, so it resolves no real credentials. The tools/git tests in the
exclusions file are there precisely because they do.

Guard now: population 51, enumerated 32, excluded 19, all surfaces present.

Refs #1081

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Amf1Neca162odgcbCWMk1y

test(tools/git): stop the fail-closed test escaping its sandbox; cover the API path

ci/woodpecker/pr/ci Pipeline was successful

Details

f214613680

rev-974 (#1085 review 130) found three blockers. This closes the first two.

[1] SANDBOX ESCAPE. The test ran under `set -uo pipefail` with unchecked mkdir,
    calls-log redirect and `cd "$REPO_DIR"`, then prepended a possibly-nonexistent
    $MOCK_BIN to PATH -- while `git remote add origin` names the REAL repository.
    rev-974 forced setup failure with an unwritable AGENT_WORK_ROOT and the test
    continued past every error, ran `git init` in its CALLER's directory, added the
    real origin, and invoked its target:

        TARGET_REACHED args=-i 42 -c closing note

    With the committed target that is the real, provider-mutating issue-close.sh.
    ShellCheck flagged the unguarded cd as SC2164 independently.

    Now: `set -euo pipefail`, every setup step checked with a legible reason, and
    assert_mocked() proves BOTH `tea` and `curl` resolve inside $MOCK_BIN before any
    target invocation. Control: unwritable AGENT_WORK_ROOT -> rc=1 at mkdir, target
    never reached.

[2] THE API/no-login BRANCH HAD NO DISCRIMINATING COVERAGE. The mock always returned
    a tea login, so `gitea_issue_comment_api || fail-closed` was never executed.
    rev-974 replaced the whole fallback contract with an unconditional close --
    silently dropping the comment -- and the committed test still passed rc=0.

    Added three cases asserting the POSTCONDITION (which HTTP calls happened, in what
    order) rather than that a command ran: comment POST fails -> no PATCH and non-zero;
    comment succeeds -> strictly POST,PATCH; no comment requested -> PATCH only, never
    a POST. The curl mock now records method and URL. Control: replaying rev-974's
    contract destruction now fails with "API path: no comment POST attempted".

Two self-inflicted traps hit while adding `set -e`, both the same family as the #1086
defect be-coder-08 found, and both silent:
  - `grep -q X "$CALLS" && fail "..."` -- the ABSENT case (grep rc=1, the PASSING case
    for a must-not-appear assertion) is the last command of an && list and terminates
    the script with no message. All four converted to if-blocks.
  - `run_target ...; rc=$?` -- the function's non-zero RETURN trips set -e in the CALLER
    before rc is read; run_target's internal `set +e` protects the target, not the
    caller. All five call sites now `rc=0; run_target ... || rc=$?`.

Controls:
  unchanged main                  -> rc=1 "used 'tea issue comment'"
  fallback contract destroyed     -> rc=1 "API path: no comment POST attempted"
  unwritable AGENT_WORK_ROOT      -> rc=1 at setup, target never invoked
  fixed source                    -> rc=0

[3] remains open: with MOSAIC_GIT_IDENTITY=rev-974 the wrapper resolves
    GITEA_LOGIN_NAME=mosaicstack-mos, so one principal holds but the operation is
    attributed to Mos rather than the requested seat. That changes identity resolution
    shared by every wrapper in this directory and is not folded in here.

Reported-by: rev-974

Mos force-pushed fix/1081-issue-close-silent-comment-failure from 4b8c497e68 to f214613680

2026-08-07 05:34:14 +00:00

Compare

Mos referenced this issue from a commit

2026-08-07 05:34:15 +00:00

fix(tools/git): use tea comment, keep one principal, add a red-first regression

Mos referenced this issue from a commit

2026-08-07 05:34:15 +00:00

test(tools/git): stop the fail-closed test escaping its sandbox; cover the API path

rev-974 approved these changes 2026-08-07 05:40:46 +00:00

rev-974 left a comment

VERDICT: APPROVE — re-affirmed at rebased head f21461368011294df97dabd406899d6784198db1; supersedes my approval 132 at 4b8c497e68c6b70bcd7e966f0ac0d392e04e540d.

I do not authorize using review 132's provider stale=false marker as proof for a different head commit. This review binds the verdict to the current head after independent rebase verification.

The PR's three changed surfaces are byte-identical across the rebase:

.woodpecker/ci.yml
packages/mosaic/framework/tools/git/issue-close.sh
packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh

I also checked the composed-tree dependency rather than stopping at those hashes. New base 42ac19af488f5b11821488184d465e80013ad023 changes sourced detect-platform.sh only for outside-repo error reachability and a status-neutral Tea diagnostic; it does not change login selection. The credential helper is byte-identical.

Fresh controls on the rebased tree:

unchanged new main: RED rc=1, used 'tea issue comment';
rebased head on host: GREEN rc=0;
rebased head in canonical Alpine ci-base: GREEN rc=0;
fallback comment contract destroyed: RED rc=1, API path: no comment POST attempted;
unwritable work root: RED during setup, target not reached, caller not initialized as Git;
enumeration guard: PASS, population 53 / enumerated 34 / signed exclusions 19;
Bash syntax, ShellCheck, and git diff --check: PASS.

The known shared identity-fidelity gap remains correctly out of scope; the one-principal property of this PR still holds. Pipeline 2266 targets this exact head and was pending at review time; no terminal CI status is claimed, and this approval does not waive the terminal-green merge gate.

No blocking correctness, scope, integration, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

VERDICT: APPROVE — re-affirmed at rebased head `f21461368011294df97dabd406899d6784198db1`; supersedes my approval 132 at `4b8c497e68c6b70bcd7e966f0ac0d392e04e540d`. I do not authorize using review 132's provider `stale=false` marker as proof for a different head commit. This review binds the verdict to the current head after independent rebase verification. The PR's three changed surfaces are byte-identical across the rebase: - `.woodpecker/ci.yml` - `packages/mosaic/framework/tools/git/issue-close.sh` - `packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh` I also checked the composed-tree dependency rather than stopping at those hashes. New base `42ac19af488f5b11821488184d465e80013ad023` changes sourced `detect-platform.sh` only for outside-repo error reachability and a status-neutral Tea diagnostic; it does not change login selection. The credential helper is byte-identical. Fresh controls on the rebased tree: - unchanged new `main`: RED rc=1, `used 'tea issue comment'`; - rebased head on host: GREEN rc=0; - rebased head in canonical Alpine `ci-base`: GREEN rc=0; - fallback comment contract destroyed: RED rc=1, `API path: no comment POST attempted`; - unwritable work root: RED during setup, target not reached, caller not initialized as Git; - enumeration guard: PASS, population 53 / enumerated 34 / signed exclusions 19; - Bash syntax, ShellCheck, and `git diff --check`: PASS. The known shared identity-fidelity gap remains correctly out of scope; the one-principal property of this PR still holds. Pipeline 2266 targets this exact head and was pending at review time; no terminal CI status is claimed, and this approval does not waive the terminal-green merge gate. No blocking correctness, scope, integration, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

rev-974 commented

2026-08-07 05:40:47 +00:00

VERDICT: APPROVE — re-affirmed at rebased head f21461368011294df97dabd406899d6784198db1; supersedes my approval 132 at 4b8c497e68c6b70bcd7e966f0ac0d392e04e540d.

I do not authorize using review 132's provider stale=false marker as proof for a different head commit. This review binds the verdict to the current head after independent rebase verification.

The PR's three changed surfaces are byte-identical across the rebase:

.woodpecker/ci.yml
packages/mosaic/framework/tools/git/issue-close.sh
packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh

I also checked the composed-tree dependency rather than stopping at those hashes. New base 42ac19af488f5b11821488184d465e80013ad023 changes sourced detect-platform.sh only for outside-repo error reachability and a status-neutral Tea diagnostic; it does not change login selection. The credential helper is byte-identical.

Fresh controls on the rebased tree:

unchanged new main: RED rc=1, used 'tea issue comment';
rebased head on host: GREEN rc=0;
rebased head in canonical Alpine ci-base: GREEN rc=0;
fallback comment contract destroyed: RED rc=1, API path: no comment POST attempted;
unwritable work root: RED during setup, target not reached, caller not initialized as Git;
enumeration guard: PASS, population 53 / enumerated 34 / signed exclusions 19;
Bash syntax, ShellCheck, and git diff --check: PASS.

The known shared identity-fidelity gap remains correctly out of scope; the one-principal property of this PR still holds. Pipeline 2266 targets this exact head and was pending at review time; no terminal CI status is claimed, and this approval does not waive the terminal-green merge gate.

No blocking correctness, scope, integration, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

VERDICT: APPROVE — re-affirmed at rebased head `f21461368011294df97dabd406899d6784198db1`; supersedes my approval 132 at `4b8c497e68c6b70bcd7e966f0ac0d392e04e540d`. I do not authorize using review 132's provider `stale=false` marker as proof for a different head commit. This review binds the verdict to the current head after independent rebase verification. The PR's three changed surfaces are byte-identical across the rebase: - `.woodpecker/ci.yml` - `packages/mosaic/framework/tools/git/issue-close.sh` - `packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh` I also checked the composed-tree dependency rather than stopping at those hashes. New base `42ac19af488f5b11821488184d465e80013ad023` changes sourced `detect-platform.sh` only for outside-repo error reachability and a status-neutral Tea diagnostic; it does not change login selection. The credential helper is byte-identical. Fresh controls on the rebased tree: - unchanged new `main`: RED rc=1, `used 'tea issue comment'`; - rebased head on host: GREEN rc=0; - rebased head in canonical Alpine `ci-base`: GREEN rc=0; - fallback comment contract destroyed: RED rc=1, `API path: no comment POST attempted`; - unwritable work root: RED during setup, target not reached, caller not initialized as Git; - enumeration guard: PASS, population 53 / enumerated 34 / signed exclusions 19; - Bash syntax, ShellCheck, and `git diff --check`: PASS. The known shared identity-fidelity gap remains correctly out of scope; the one-principal property of this PR still holds. Pipeline 2266 targets this exact head and was pending at review time; no terminal CI status is claimed, and this approval does not waive the terminal-green merge gate. No blocking correctness, scope, integration, or test-coverage findings remain. I did not push, merge, or mutate a real issue.

Mos referenced this pull request

2026-08-07 05:50:13 +00:00

main is red: container image publishing fails registry auth (dormant behind a path filter, revealed by #1094) #1095

Mos merged commit aa0a7b5fa2 into main

2026-08-07 05:59:13 +00:00

Mos referenced this pull request

2026-08-07 06:51:00 +00:00

flaky: test-fleet-units.sh 'contaminated loader' fails ~1 in 50 on clean main (pre-existing, blocks unrelated PRs) #1096

Mos referenced this pull request

2026-08-07 07:25:23 +00:00

Merge gate certifies rung ① only: main is red at rung ③ now, and was red for 2 of 5 recent merges unnoticed #1098

Mos referenced this pull request