[PR #186] [MERGED] feat(linear-ticket): make the warn-only pilot fail the check without blocking the merge #195

New issue

Closed

opened 2026-10-26 22:37:13 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:37:13 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/186
Author: @mattmillerai
Created: 8/20/2026
Status: ✅ Merged
Merged: 8/20/2026
Merged by: @mattmillerai

Base: main ← Head: matt/linear-ticket-soft-fail

📝 Commits (2)

26339cd feat(linear-ticket): make the warn-only pilot fail the check without blocking the merge
feca179 fix(linear-ticket): make the soft-fail copy true in every caller configuration

📊 Changes

7 files changed (+418 additions, -29 deletions)

View changed files

📝 .github/workflows/linear-ticket.yml (+35 -3)
📝 README.md (+1 -1)
📝 docs/callers/linear-ticket.md (+53 -5)
📝 scripts/linear-ticket/README.md (+19 -3)
📝 scripts/linear-ticket/lib.py (+139 -0)
📝 scripts/linear-ticket/tests/test_lib.py (+144 -0)
📝 scripts/linear-ticket/validate.py (+27 -17)

📄 Description

Why

On Comfy-Org/cloud#6690 the gate worked perfectly — it found three referenced-but-unlinked identifiers (PILOT-449, BE-7081, BE-3033) and explained exactly how to fix them. Nobody noticed. In warn-only the status publishes green, so the verdict lives only in a marker comment sitting among a dozen other bot comments on a busy PR.

A pilot exists to be observed. A passing check is a check nobody reads.

What changes

New soft-fail input (boolean, default true), read only when enforce: false:

mode	status	job exit	on the PR
`enforce: true`	🔴 `failure`	1	red check (blocks once the context is required)
`enforce: false`, `soft-fail: true` (new default)	🔴 `failure`	0	red check + comment — blocks nothing
`enforce: false`, `soft-fail: false`	🟢 `success`	0	green; verdict only in summary + comment (previous behaviour)

Nothing becomes a blocker. Blocking is branch protection requiring the linear-ticket status context — never this workflow's call. I checked both live callers before writing this: neither Comfy-Org/cloud's "Requirements to Merge to Main" ruleset nor Comfy-Org/comfy-infra's main ruleset lists linear-ticket among its required contexts, so the red status is advisory on both today. A non-required failing check leaves the merge button live.

The marker comment now says so, so a red X is never misread as a merge block. The wording is
deliberately hedged — the validator never reads the caller's ruleset, so it cannot assert
that nothing is blocked (see the review round below):

⚠️ Advisory — this check is warn-only. It is reported as failing so it shows up in the PR's check list, but nothing this workflow does blocks a merge: what blocks is your repository's ruleset requiring the linear-ticket status context, and a warn-only pilot is meant to run before that context is required. If your merge button is actually blocked on linear-ticket, then the context is already required while the pilot is still warn-only — tell the repository owners, because that combination is a misconfiguration.

…plus one category-aware sentence: link the ticket, or — for infra_error — there is nothing to
fix on the PR at all. The headline becomes ❌ fail (exists_not_linked) — warn-only.

Two deliberate calls

The job stays green in warn-only. This validator runs on the default branch off workflow_run, so its run is never attached to the PR — the commit status is the only PR-visible signal. A red default-branch run for a check that is deliberately not gating would read as "main is broken" in the Actions tab: louder, in the wrong place. Only enforce: true exits nonzero.

Default true, so the SHA bump alone delivers it. bump-callers.sh moves pins, not inputs; defaulting off would mean hand-editing every caller. The one thing this makes possible is a footgun, documented at the input, in the docs table, and as a gotcha: enforce: false + soft-fail: true + the context already required would block merges. Don't require the context before flipping enforce: true — which was already the documented rollout order (step 6, after step 5).

Rollout ladder, restated

Land warn-only, context not required → red-but-advisory check. (← the pilot now lives here)
Observe.
enforce: true, still not required → PR-visible result unchanged; the validator's own run now also goes red, so systemic breakage shows in the Actions tab.
Add linear-ticket to the ruleset. Only now does anything block.

Implementation

The mode decision moves out of validate.py into a pure lib.failure_outcome() returning (verdict, state, description, exit_code, advisory) — lib.py's stated contract is that every decision is testable with no network call, and this was the last one inline in the orchestrator.

New tests pin the invariants worth pinning: only enforce ever exits nonzero, the advisory note never rides along with a gating verdict, enforce ignores soft-fail entirely, and every mode's status description fits GitHub's 140-char truncation (a description that only survives truncation would silently lose the category).

One behaviour deliberately unchanged: a failed terminal status write is still exit 1 even in warn-only. Soft-fail's whole point is that the red status lands; a missing write leaves the previous status standing as this commit's answer.

Tests

python3 -m unittest discover -s scripts/linear-ticket/tests -p 'test_*.py'   # 63 passed (was 46)
python3 -m py_compile scripts/linear-ticket/{lib,validate}.py                # ok
python3 .github/workflow-pins/check_workflow_pins.py                         # 10 workflows, 0 defaults
bash .github/bump-callers/tests/test_paths_contract.sh                       # 66 passed

Review round (cursor-review panel, all 7 threads)

Four fixes landed in feca179; three findings answered on the thread without a code change.

The advisory note asserted something it cannot know. "This does not block the merge" is a
claim about the caller's ruleset, which this validator never reads — and it is false in
exactly the footgun this PR documents. Rewritten to state only what is knowable, and to name
the misconfiguration to report if the merge button really is blocked.
Status descriptions are now category-aware. infra_error means Linear could not be
queried, so no linked Linear issue (infra_error) was a diagnosis the run never made; the
note likewise no longer tells an author to link a ticket to fix a broken credential. (This
string predates the PR — soft-fail just promoted it to a louder surface.)
An absent SOFT_FAIL env now means the silent variant. The workflow always passes it,
so absent can only mean the uses: and workflows_ref pins skewed apart — and a caller that
cannot even express the input must not be silently upgraded from green to red. Pin skew is
now inert in the safe direction, matching ENFORCE's fail-closed default.
"the job never exits nonzero" narrowed to "a failing verdict never exits the job
nonzero" across the input description, both READMEs, the caller doc and the validate.py
contract — warn-only still exits 1 on a failed terminal status write, a missing token/repo,
malformed team-keys, or a non-pull_request event.
New "Upgrading an existing enforce: false caller" doc section, plus a BEHAVIOUR CHANGE
paragraph on the input itself, so the green→red flip is documented at the callers it changes
along with the one-line soft-fail: false opt-out. The default stays true — see below.

Answered without a change: the soft-fail name (the polarity matches Buildkite's
soft_fail — a real failure that does not fail the build; happy to rename while no caller sets
it yet), the comment-before-status write order (reversing it trades an unexplained red check
for an unlanded one — pre-existing since #183), and the default true itself.

Note

One open call for the merger. soft-fail defaults to true, so taking a SHA bump flips
an existing enforce: false caller from a green status to a red (still non-required) one.
That is the intent — a green pilot is a pilot nobody reads — and it is now documented at the
input and in the caller doc rather than shipping silently. Flipping the default to false and
making the loud mode opt-in is a one-line change if you would rather ship it additive.

Follow-up (not in this PR)

The auto-bump PR into Comfy-Org/cloud will move the SHA only, leaving linear-ticket-validate.yml's header comment saying "WARN-ONLY pilot (enforce: false): every outcome is green" — true before this change, stale after. Happy to open a one-line comment fix there; say the word.

Provenance

Authored by: agent-work loop
Verified: python3 -m unittest discover -s scripts/linear-ticket/tests -p 'test_*.py' — 63 passed; python3 -m py_compile scripts/linear-ticket/{lib,validate}.py — ok. All PR checks green on the pre-review-round commit; the review-round commit re-runs them.
Deviations: Three of the seven panel findings were answered on the thread rather than fixed — the soft-fail input name, the comment-before-status write order, and the default: true behaviour change (mitigated by documentation rather than by flipping the default). Rationale on each thread. AGENTS.md fails the repo's own 200-line integrity ceiling at 273 lines, but that is pre-existing on main, is not a check on this PR, and was left untouched.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/186 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/20/2026 **Status:** ✅ Merged **Merged:** 8/20/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/linear-ticket-soft-fail` --- ### 📝 Commits (2) - [`26339cd`](https://github.com/Comfy-Org/github-workflows/commit/26339cdda5a335dbf7d835df42a92db6b91693a4) feat(linear-ticket): make the warn-only pilot fail the check without blocking the merge - [`feca179`](https://github.com/Comfy-Org/github-workflows/commit/feca17901a6b5db5bd76976141cedbd859f07e1c) fix(linear-ticket): make the soft-fail copy true in every caller configuration ### 📊 Changes **7 files changed** (+418 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/linear-ticket.yml` (+35 -3) 📝 `README.md` (+1 -1) 📝 `docs/callers/linear-ticket.md` (+53 -5) 📝 `scripts/linear-ticket/README.md` (+19 -3) 📝 `scripts/linear-ticket/lib.py` (+139 -0) 📝 `scripts/linear-ticket/tests/test_lib.py` (+144 -0) 📝 `scripts/linear-ticket/validate.py` (+27 -17) </details> ### 📄 Description ## Why On [Comfy-Org/cloud#6690](https://github.com/Comfy-Org/cloud/pull/6690) the gate worked perfectly — it found three referenced-but-unlinked identifiers (`PILOT-449, BE-7081, BE-3033`) and explained exactly how to fix them. Nobody noticed. In warn-only the status publishes **green**, so the verdict lives only in a marker comment sitting among a dozen other bot comments on a busy PR. A pilot exists to be observed. A passing check is a check nobody reads. ## What changes New `soft-fail` input (boolean, **default `true`**), read only when `enforce: false`: | mode | status | job exit | on the PR | |---|---|---|---| | `enforce: true` | 🔴 `failure` | 1 | red check (blocks once the context is required) | | `enforce: false`, `soft-fail: true` **(new default)** | 🔴 `failure` | 0 | red check + comment — **blocks nothing** | | `enforce: false`, `soft-fail: false` | 🟢 `success` | 0 | green; verdict only in summary + comment (previous behaviour) | **Nothing becomes a blocker.** Blocking is branch protection requiring the `linear-ticket` status context — never this workflow's call. I checked both live callers before writing this: neither `Comfy-Org/cloud`'s "Requirements to Merge to Main" ruleset nor `Comfy-Org/comfy-infra`'s `main` ruleset lists `linear-ticket` among its required contexts, so the red status is advisory on both today. A non-required failing check leaves the merge button live. The marker comment now says so, so a red X is never misread as a merge block. The wording is deliberately **hedged** — the validator never reads the caller's ruleset, so it cannot assert that nothing is blocked (see the review round below): > ⚠️ **Advisory — this check is warn-only.** It is reported as failing so it shows up in the PR's check list, but nothing this workflow does blocks a merge: what blocks is your repository's ruleset requiring the `linear-ticket` status context, and a warn-only pilot is meant to run *before* that context is required. If your merge button is actually blocked on `linear-ticket`, then the context is already required while the pilot is still warn-only — tell the repository owners, because that combination is a misconfiguration. …plus one category-aware sentence: link the ticket, or — for `infra_error` — there is nothing to fix on the PR at all. The headline becomes `❌ fail (exists_not_linked) — warn-only`. ## Two deliberate calls **The job stays green in warn-only.** This validator runs on the default branch off `workflow_run`, so its run is never attached to the PR — the commit status is the only PR-visible signal. A red default-branch run for a check that is deliberately not gating would read as "main is broken" in the Actions tab: louder, in the wrong place. Only `enforce: true` exits nonzero. **Default `true`, so the SHA bump alone delivers it.** `bump-callers.sh` moves pins, not inputs; defaulting off would mean hand-editing every caller. The one thing this makes possible is a footgun, documented at the input, in the docs table, and as a gotcha: `enforce: false` + `soft-fail: true` + the context *already required* would block merges. Don't require the context before flipping `enforce: true` — which was already the documented rollout order (step 6, after step 5). ## Rollout ladder, restated 1. Land warn-only, context **not** required → red-but-advisory check. *(← the pilot now lives here)* 2. Observe. 3. `enforce: true`, still not required → PR-visible result unchanged; the validator's own run now also goes red, so systemic breakage shows in the Actions tab. 4. Add `linear-ticket` to the ruleset. **Only now does anything block.** ## Implementation The mode decision moves out of `validate.py` into a pure `lib.failure_outcome()` returning `(verdict, state, description, exit_code, advisory)` — `lib.py`'s stated contract is that every decision is testable with no network call, and this was the last one inline in the orchestrator. New tests pin the invariants worth pinning: only enforce ever exits nonzero, the advisory note never rides along with a gating verdict, enforce ignores `soft-fail` entirely, and every mode's status description fits GitHub's 140-char truncation (a description that only survives truncation would silently lose the category). One behaviour deliberately unchanged: a failed *terminal status write* is still exit 1 even in warn-only. Soft-fail's whole point is that the red status lands; a missing write leaves the previous status standing as this commit's answer. ## Tests ``` python3 -m unittest discover -s scripts/linear-ticket/tests -p 'test_*.py' # 63 passed (was 46) python3 -m py_compile scripts/linear-ticket/{lib,validate}.py # ok python3 .github/workflow-pins/check_workflow_pins.py # 10 workflows, 0 defaults bash .github/bump-callers/tests/test_paths_contract.sh # 66 passed ``` ## Review round (cursor-review panel, all 7 threads) Four fixes landed in `feca179`; three findings answered on the thread without a code change. - **The advisory note asserted something it cannot know.** "This does not block the merge" is a claim about the *caller's* ruleset, which this validator never reads — and it is false in exactly the footgun this PR documents. Rewritten to state only what is knowable, and to name the misconfiguration to report if the merge button really is blocked. - **Status descriptions are now category-aware.** `infra_error` means Linear could not be queried, so `no linked Linear issue (infra_error)` was a diagnosis the run never made; the note likewise no longer tells an author to link a ticket to fix a broken credential. (This string predates the PR — soft-fail just promoted it to a louder surface.) - **An absent `SOFT_FAIL` env now means the *silent* variant.** The workflow always passes it, so absent can only mean the `uses:` and `workflows_ref` pins skewed apart — and a caller that cannot even express the input must not be silently upgraded from green to red. Pin skew is now inert in the safe direction, matching `ENFORCE`'s fail-closed default. - **"the job never exits nonzero" narrowed to "a failing *verdict* never exits the job nonzero"** across the input description, both READMEs, the caller doc and the `validate.py` contract — warn-only still exits 1 on a failed terminal status write, a missing token/repo, malformed `team-keys`, or a non-`pull_request` event. - **New "Upgrading an existing `enforce: false` caller" doc section**, plus a BEHAVIOUR CHANGE paragraph on the input itself, so the green→red flip is documented at the callers it changes along with the one-line `soft-fail: false` opt-out. The default stays `true` — see below. Answered without a change: the `soft-fail` **name** (the polarity matches Buildkite's `soft_fail` — a real failure that does not fail the build; happy to rename while no caller sets it yet), the comment-before-status **write order** (reversing it trades an unexplained red check for an unlanded one — pre-existing since #183), and the **default `true`** itself. > [!NOTE] > **One open call for the merger.** `soft-fail` defaults to `true`, so taking a SHA bump flips > an existing `enforce: false` caller from a green status to a red (still non-required) one. > That is the intent — a green pilot is a pilot nobody reads — and it is now documented at the > input and in the caller doc rather than shipping silently. Flipping the default to `false` and > making the loud mode opt-in is a one-line change if you would rather ship it additive. ## Follow-up (not in this PR) The auto-bump PR into `Comfy-Org/cloud` will move the SHA only, leaving `linear-ticket-validate.yml`'s header comment saying *"WARN-ONLY pilot (enforce: false): every outcome is green"* — true before this change, stale after. Happy to open a one-line comment fix there; say the word. ## Provenance **Authored by:** agent-work loop **Verified:** `python3 -m unittest discover -s scripts/linear-ticket/tests -p 'test_*.py'` — 63 passed; `python3 -m py_compile scripts/linear-ticket/{lib,validate}.py` — ok. All PR checks green on the pre-review-round commit; the review-round commit re-runs them. **Deviations:** Three of the seven panel findings were answered on the thread rather than fixed — the `soft-fail` input name, the comment-before-status write order, and the `default: true` behaviour change (mitigated by documentation rather than by flipping the default). Rationale on each thread. `AGENTS.md` fails the repo's own 200-line integrity ceiling at 273 lines, but that is pre-existing on `main`, is not a check on this PR, and was left untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:37:13 +00:00