From 68c9a573553b37816fce5c7034e38e5b16dce411 Mon Sep 17 00:00:00 2001 From: "cmeans-claude-dev[bot]" <272174644+cmeans-claude-dev[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:55:23 -0500 Subject: [PATCH] =?UTF-8?q?fix(workflows):=20pr-labels-ci.yml=20bidirectio?= =?UTF-8?q?nal=20CI=20=E2=86=94=20label=20transitions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the stale-label trap where a PR sitting at `CI Failed` whose follow-up push made CI go green silently no-op'd and stayed at `CI Failed`. Root cause: `on-ci-pass`'s outer guard `if echo "$LABELS" | grep -q "^Awaiting CI$"` only fired on `Awaiting CI`, so the `CI Failed → Ready for QA` arc was missing. Symmetric fix on `on-ci-fail`: a PR at `Ready for QA` whose CI was re-run (manual re-trigger after a flake) and turned red kept the green label, hiding the regression from QA. `on-ci-fail` now also fires when `Ready for QA` is the current label, removing it and applying `CI Failed`. Both jobs gain an explicit short-circuit on the in-flight review labels `QA Active` / `Ready for QA Signoff` / `QA Approved` so the broadened triggers do not disrupt review-machine state — review advances independently of CI re-runs, and a re-run's success or failure on those states is visible via the check itself rather than a label transition. `Dev Active` short-circuit preserved unchanged. No new contributor-controlled inputs introduced. Label list still read via `gh pr view --json labels` (repo-owned strings, not fork-controlled). Anchored grep patterns (`^Label$`) stay anchored. Existing env-routing of `HEAD_BRANCH` / `RUN_ID` / `PR` / `REPO` (hardened in #332/#333) unchanged. Closes #378. Verification is intrinsically post-merge — `workflow_run` triggers always run from the default branch, so changes to `pr-labels-ci.yml` cannot be exercised on the PR that introduces them. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/pr-labels-ci.yml | 63 +++++++++++++++++++++++++----- CHANGELOG.md | 1 + 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-labels-ci.yml b/.github/workflows/pr-labels-ci.yml index d6411b8..4a3de71 100644 --- a/.github/workflows/pr-labels-ci.yml +++ b/.github/workflows/pr-labels-ci.yml @@ -37,8 +37,11 @@ permissions: checks: read jobs: - # When CI passes on a PR branch, promote from Awaiting CI to Ready for QA. - # Skips if Dev Active is present (dev isn't done yet). + # When CI passes on a PR branch, promote from Awaiting CI OR CI Failed + # to Ready for QA. Skips if Dev Active is present (dev isn't done yet) + # or if a review-state label (QA Active / Ready for QA Signoff / + # QA Approved) is present — review-machine state is not torn down by + # a CI re-run. on-ci-pass: if: >- github.event.workflow_run.conclusion == 'success' @@ -83,18 +86,39 @@ jobs: exit 0 fi - # Promote if Awaiting CI is present + # Don't disrupt in-flight review state. QA Active implies the + # PR has already advanced past Ready for QA; Ready for QA + # Signoff and QA Approved are even further along. A passing + # CI re-run on those states is a no-op — the success status + # is visible on its own check, no label transition needed. + for QA_STATE in "QA Active" "Ready for QA Signoff" "QA Approved"; do + if echo "$LABELS" | grep -q "^$QA_STATE$"; then + echo "$QA_STATE present — skipping promotion (review in progress)" + exit 0 + fi + done + + # Promote if Awaiting CI OR CI Failed is the current pre-state. + # Closes #378: previous code only fired on Awaiting CI, so a + # PR sitting at CI Failed never auto-promoted when a fix push + # made CI go green. + REMOVE="" if echo "$LABELS" | grep -q "^Awaiting CI$"; then REMOVE="Awaiting CI" - if echo "$LABELS" | grep -q "^CI Failed$"; then - REMOVE="Awaiting CI,CI Failed" - fi + fi + if echo "$LABELS" | grep -q "^CI Failed$"; then + REMOVE="${REMOVE:+$REMOVE,}CI Failed" + fi + if [ -n "$REMOVE" ]; then gh pr edit "$PR" --repo "$REPO" --remove-label "$REMOVE" gh pr edit "$PR" --repo "$REPO" --add-label "Ready for QA" fi # When CI fails on a PR branch, set CI Failed. - # Skips if Dev Active is present (dev is already working). + # Skips if Dev Active is present (dev is already working) or if a + # review-state label (QA Active / Ready for QA Signoff / QA Approved) + # is present — a CI re-run failure does not retroactively invalidate + # an in-flight review; the failed check is visible on its own. on-ci-fail: if: >- github.event.workflow_run.conclusion == 'failure' @@ -133,8 +157,29 @@ jobs: exit 0 fi - # Set CI Failed if Awaiting CI is present (normal flow) + # Don't disrupt in-flight review state. The failure is visible + # via the CI status check on the PR; tearing down review-machine + # state on a (possibly transient) re-run failure would erase + # work the reviewer has already done. + for QA_STATE in "QA Active" "Ready for QA Signoff" "QA Approved"; do + if echo "$LABELS" | grep -q "^$QA_STATE$"; then + echo "$QA_STATE present — skipping CI Failed (review in progress)" + exit 0 + fi + done + + # Set CI Failed if state is Awaiting CI OR Ready for QA. + # Adding Ready for QA closes the symmetric trap noted in #378: + # a CI re-run failure on a PR sitting at Ready for QA used to + # leave the PR labelled green even though CI was now red. + REMOVE="" if echo "$LABELS" | grep -q "^Awaiting CI$"; then - gh pr edit "$PR" --repo "$REPO" --remove-label "Awaiting CI" + REMOVE="Awaiting CI" + fi + if echo "$LABELS" | grep -q "^Ready for QA$"; then + REMOVE="${REMOVE:+$REMOVE,}Ready for QA" + fi + if [ -n "$REMOVE" ]; then + gh pr edit "$PR" --repo "$REPO" --remove-label "$REMOVE" gh pr edit "$PR" --repo "$REPO" --add-label "CI Failed" fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 49843ff..b2b78b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Cross-linked intention-firing semantics across the `awareness://briefing` resource and the `get_briefing`, `get_intentions`, and `remind` tool docstrings.** Each surface was previously documented in isolation, leaving an LLM (or human) reading them cold unable to determine the cross-tool relationship — most commonly producing a redundant `get_briefing` followed by `get_intentions(state="fired")`, even though `get_briefing` already both fires any pending intentions whose `deliver_at` has passed and surfaces currently-fired intentions inline. Same shape of discoverability gap as the one closed by [#399](https://github.com/cmeans/mcp-awareness/pull/399) for `remind`'s three `deliver_at` modes — correct behavior already in place; just not visible from the documented surface alone. The `awareness://briefing` resource (which the server's own `instructions.md` directs agents to read at conversation start, and which Claude Desktop reads instead of the tool path) gets the same firing language as `get_briefing` — both call the same `generate_briefing()` and fire identically; `get_briefing` now mentions that it fires due intentions and surfaces fired ones inline (and that a follow-up `get_intentions(state="fired")` is redundant); `get_intentions` positions itself as the drill-in for non-fired states or filtered queries and notes that pending intentions without `deliver_at` are not auto-fired by briefing; `remind` adds a reader-side cross-link making the one-call workflow explicit. Docs-only — no code, tests, or schema changes; behavior identical to v0.18.4. Closes [#404](https://github.com/cmeans/mcp-awareness/issues/404). Surfaced by Claude Desktop during a session walking through an inbound/outbound handoff workflow (awareness logical_key `tool-desc-gap-intention-firing-semantics`). ### Fixed +- **`pr-labels-ci.yml` now handles bidirectional CI ↔ label transitions, closing two stale-label traps.** Previous `on-ci-pass` only promoted to `Ready for QA` when `Awaiting CI` was the current label; a PR sitting at `CI Failed` whose follow-up push made CI go green silently no-op'd and stayed at `CI Failed` (root cause: outer guard `if echo "$LABELS" | grep -q "^Awaiting CI$"`). Symmetric trap on `on-ci-fail`: a PR at `Ready for QA` whose CI was re-run (e.g., manual re-trigger after a flake) and turned red kept the green label, hiding the regression from QA. Both jobs now broaden the eligible pre-state set — `on-ci-pass` fires on `Awaiting CI` OR `CI Failed`, `on-ci-fail` fires on `Awaiting CI` OR `Ready for QA` — and remove whichever pre-state labels are present before applying the new one. To prevent the broadened triggers from disrupting an in-flight review, both jobs gain an explicit short-circuit when any of `QA Active` / `Ready for QA Signoff` / `QA Approved` is present: review-machine state advances independently of CI re-runs, and a passing or failing CI run on those states is visible via the check itself rather than a label transition. `Dev Active` short-circuit preserved unchanged. No new contributor-controlled inputs are introduced — label list is read via `gh pr view --json labels` (repo-owned strings, not fork-controlled), all anchored grep patterns (`^Label$`) stay anchored, and existing env-routing of `HEAD_BRANCH` / `RUN_ID` / `PR` / `REPO` (hardened in #332/#333) is unchanged. Closes [#378](https://github.com/cmeans/mcp-awareness/issues/378). Verification is intrinsically post-merge — `workflow_run` triggers always run from the default branch, so changes to `pr-labels-ci.yml` cannot be exercised on the PR that introduces them. - **`pr-labels.yml` `on-unlabel` job correctly promotes `Awaiting CI` → `Ready for QA` when CI is green at the time `Dev Active` is removed.** Previous lookup used `gh api ".../actions/workflows/ci.yml/runs?head_sha=$HEAD_SHA"`, but the GitHub Workflow Runs API filter parameters (`?head_sha=`, `?branch=`, `?event=`, `?status=`) intermittently return `total_count: 0` for this repo's `ci.yml` even when matching runs exist (suspected stale internal index from the workflow's history). The script then fell through to the "CI hasn't passed" branch and re-added `Awaiting CI`, leaving the PR stuck. Replaced with an unfiltered list (`?per_page=50`) plus a client-side `jq` filter on `head_sha` via `env.HEAD_SHA` — unfiltered listings hit a different code path and return correct results. `pr-labels-ci.yml` was audited and does **not** have the same problem (it reads `github.event.workflow_run.conclusion` directly from the trigger payload, never API-filters by SHA). Hit twice on PR #405 (2026-04-27); manual `Awaiting CI` → `Ready for QA` flips both times. Closes [#406](https://github.com/cmeans/mcp-awareness/issues/406). ### Security