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

[framework-bug/tooling][p3-low] pr-review.sh: tea pr approve/tea pr reject pass unsupported --comment flag → durable approval RoR fails closed #835

New Issue
Open
opened 2026-07-18 01:00:27 +00:00 by jason.woltje · 0 comments
jason.woltje commented 2026-07-18 01:00:27 +00:00
Owner
Copy Link

Type: framework-bug / tooling · Priority: LOW (p3) · Fails CLOSED (errored approve, NOT a silent false-approve — not a security hole).

Defect

packages/mosaic/framework/tools/git/pr-review.sh Gitea approval path passes a flag tea v0.11.1 does not define:

  • L88: tea pr approve "$PR_NUMBER" $(get_gitea_repo_args) ${COMMENT:+--comment "$COMMENT"}
  • L96: tea pr reject "$PR_NUMBER" $(get_gitea_repo_args) --comment "$COMMENT"

tea pr approve / tea pr reject have no --comment option in tea v0.11.1, so any durable formal APPROVE / REQUEST-CHANGES review through the wrapper errors with:

flag provided but not defined: -comment

This blocks the durable formal-review RoR path fleet-wide (it is a root reason the durable-issue-comment workaround exists).

Impact

  • LOW / latent debt, not an active blocker. It fails closed (errored approve, never a false-positive approval), and the durable-issue-comment workaround is the ACTIVE RoR path across the fleet today.
  • Surfaced (not owned) via homelab M5-001 #789, whose durable provider RoR is blocked by exactly this error. Ownership is framework (this tool), not #789.

Scope of fix

  1. Remove the unsupported --comment from the tea pr approve (L88) and tea pr reject (L96) invocations.
  2. Route the reviewer comment via the durable path — post the review body as a tea comment / issue-comment and verify by provider read-back (the same pattern #812 applies to the comment action).

Relationship to #812 (IMPORTANT — keep separate)

#812 is a DISTINCT code path: the comment action (L87–110, tea pr comment → tea comment + read-back). Verified by diff d801d6c4...fix/812-pr-review-comment: it touches ONLY the comment action; the approve/reject lines (L87–96) are unchanged. So #812 does NOT fix this defect, and this defect does NOT block #812. They are independent and independently schedulable — do not fold this into the parked #812 (that would hide real debt behind a park).

CO-FIX note: same script, same defect class (tea flag incompatibility). Whoever next opens a pr-review.sh PR — either this issue or #812 unparking — should fix BOTH tea-flag paths in ONE PR to avoid churning the same file twice. They remain SEPARATE issues so neither blocks the other.

Cross-ref: #812 (comment-action read-back, parked) · #789 (surfaced-on, homelab-owned).

Filed by MS-LEAD after root-causing the homelab #789 bus report. Repo label taxonomy is not provisioned (only fleet-enhancement exists), so type/priority are encoded in the title.

**Type:** framework-bug / tooling · **Priority:** LOW (p3) · **Fails CLOSED** (errored approve, NOT a silent false-approve — not a security hole). ## Defect `packages/mosaic/framework/tools/git/pr-review.sh` Gitea approval path passes a flag `tea` v0.11.1 does not define: - **L88:** `tea pr approve "$PR_NUMBER" $(get_gitea_repo_args) ${COMMENT:+--comment "$COMMENT"}` - **L96:** `tea pr reject "$PR_NUMBER" $(get_gitea_repo_args) --comment "$COMMENT"` `tea pr approve` / `tea pr reject` have no `--comment` option in tea v0.11.1, so any durable formal APPROVE / REQUEST-CHANGES review through the wrapper errors with: ``` flag provided but not defined: -comment ``` This blocks the durable formal-review RoR path **fleet-wide** (it is a root reason the durable-issue-comment workaround exists). ## Impact - **LOW / latent debt, not an active blocker.** It fails closed (errored approve, never a false-positive approval), and the durable-issue-comment workaround is the ACTIVE RoR path across the fleet today. - Surfaced (not owned) via homelab M5-001 **#789**, whose durable provider RoR is blocked by exactly this error. Ownership is **framework** (this tool), not #789. ## Scope of fix 1. Remove the unsupported `--comment` from the `tea pr approve` (L88) and `tea pr reject` (L96) invocations. 2. Route the reviewer comment via the durable path — post the review body as a `tea comment` / issue-comment and verify by provider read-back (the same pattern **#812** applies to the `comment` action). ## Relationship to #812 (IMPORTANT — keep separate) **#812** is a DISTINCT code path: the `comment` action (L87–110, `tea pr comment` → `tea comment` + read-back). Verified by diff `d801d6c4...fix/812-pr-review-comment`: it touches ONLY the `comment` action; the approve/reject lines (L87–96) are **unchanged**. So #812 does NOT fix this defect, and this defect does NOT block #812. They are independent and independently schedulable — do not fold this into the parked #812 (that would hide real debt behind a park). **CO-FIX note:** same script, same defect class (tea flag incompatibility). Whoever next opens a `pr-review.sh` PR — either this issue or #812 unparking — should fix BOTH tea-flag paths in ONE PR to avoid churning the same file twice. They remain SEPARATE issues so neither blocks the other. Cross-ref: #812 (comment-action read-back, parked) · #789 (surfaced-on, homelab-owned). _Filed by MS-LEAD after root-causing the homelab #789 bus report. Repo label taxonomy is not provisioned (only `fleet-enhancement` exists), so type/priority are encoded in the title._
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#835
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?