From d9089a9a563f72d01bfb74447729ca5ce0dd29c3 Mon Sep 17 00:00:00 2001
From: openhands <openhands@all-hands.dev>
Date: Mon, 24 Aug 2026 02:01:44 +0000
Subject: [PATCH] fix(ci): align ready-for-dev gates with OpenHands
 pipefail-safe approach

Bring the ready-for-dev readiness gates in line with the current
OpenHands/OpenHands approach while keeping the SDK-specific reproduction
criteria (python/pytest/uv/pip for bugs, Desired Behavior + Acceptance
Criteria for enhancements):

- check_issue_readiness.py exits 1 for not-ready issues in both text and
  --json modes; the workflow absorbs it with `|| true` so
  `set -euo pipefail` no longer aborts the run before label/comment
  handling, and JSON output stays machine-readable.
- Post the upserted readiness comment when the ready-for-dev label is
  applied manually (labeled event), not only on open/reopen/transition.
- Share a markdown_sections helper that ignores headings and HUMAN:/AGENT:
  markers inside fenced code blocks in both the issue readiness and PR
  description checks, so pasted logs or quoted templates cannot create or
  spoof sections.
- Keep the SDK-specific linked-PR gate refresh job and the PR description
  ready-for-dev enforcement unchanged.

Tests cover ready/not-ready JSON execution (exit code + parseable JSON),
fenced-heading parsing, and label add/remove transitions refreshing
linked PR gates.

Fixes #4605

Co-authored-by: openhands <openhands@all-hands.dev>
---
 .github/scripts/check_issue_readiness.py     |  19 +--
 .github/scripts/check_pr_description.py      |  19 ++-
 .github/scripts/markdown_sections.py         |  95 ++++++++++++++
 .github/workflows/issue-readiness-check.yml  |  10 +-
 tests/cross/test_check_issue_readiness.py    | 129 ++++++++++++++++++-
 tests/cross/test_check_pr_description.py     |  52 +++++++-
 tests/cross/test_refresh_linked_pr_checks.py |  24 +++-
 7 files changed, 321 insertions(+), 27 deletions(-)
 create mode 100644 .github/scripts/markdown_sections.py

diff --git a/.github/scripts/check_issue_readiness.py b/.github/scripts/check_issue_readiness.py
index 208fdb1aa8..e037b722a5 100644
--- a/.github/scripts/check_issue_readiness.py
+++ b/.github/scripts/check_issue_readiness.py
@@ -14,7 +14,13 @@
 GitHub issue forms render each field as an `### <Label>` (h3) heading followed
 by the field text, with empty optional fields rendered as `_No response_`. This
 parser splits the body on those headings so each criterion is checked against
-the right field rather than the whole body.
+the right field rather than the whole body. Headings inside fenced code blocks
+(pasted logs, quoted templates) are ignored.
+
+The exit code is `0` when the issue is ready and `1` when it is not, in both
+text and `--json` modes. JSON output stays machine-readable on stdout either
+way; the workflow invokes the script with `|| true` so a not-ready result does
+not abort the run under `set -euo pipefail` before label/comment handling.
 
 Local usage:
 
@@ -33,6 +39,8 @@
 from dataclasses import dataclass, field
 from pathlib import Path
 
+from markdown_sections import find_headings
+
 
 BUG_LABEL = "bug"
 ENHANCEMENT_LABEL = "enhancement"
@@ -85,7 +93,7 @@ def extract_sections(body: str) -> dict[str, str]:
     form) may still use `###` headings; if they don't, the map is empty and the
     caller falls back to whole-body checks.
     """
-    matches = list(HEADING_RE.finditer(body))
+    matches = find_headings(body, HEADING_RE)
     sections: dict[str, str] = {}
     for index, match in enumerate(matches):
         start = match.end()
@@ -246,12 +254,7 @@ def main() -> int:
 
     if args.json:
         print(json.dumps({"ready": result.ready, "reasons": result.reasons}))
-        # In --json mode the exit code is not meaningful: the result is consumed
-        # via the printed JSON, and the workflow must run to completion for both
-        # ready and not-ready issues (label add/remove, feedback comment).
-        return 0
-
-    if result.ready:
+    elif result.ready:
         print("Issue meets ready-for-dev criteria.")
     else:
         print("Issue does not meet ready-for-dev criteria:")
diff --git a/.github/scripts/check_pr_description.py b/.github/scripts/check_pr_description.py
index 53df93d20d..37c6f25a48 100644
--- a/.github/scripts/check_pr_description.py
+++ b/.github/scripts/check_pr_description.py
@@ -9,6 +9,8 @@
 import urllib.request
 from pathlib import Path
 
+from markdown_sections import find_headings, without_fenced_code_blocks
+
 
 # Reject placeholders while allowing a concise human-written sentence.
 MIN_HUMAN_NOTE_CHARS = 20
@@ -51,7 +53,7 @@ def first_visible_line(text: str) -> str:
 
 
 def extract_sections(body: str) -> dict[str, str]:
-    matches = list(HEADING_RE.finditer(body))
+    matches = find_headings(body, HEADING_RE)
     sections: dict[str, str] = {}
     for index, match in enumerate(matches):
         start = match.end()
@@ -61,12 +63,19 @@ def extract_sections(body: str) -> dict[str, str]:
 
 
 def extract_human_note(body: str) -> str:
-    """Return human-written text in the required location before AGENT."""
-    human_match = HUMAN_HEADING_RE.search(body)
+    """Return human-written text in the required location before `AGENT:`.
+
+    The markers are located outside fenced code blocks so that quoting the
+    template does not stand in for filling it out. Offsets are preserved by the
+    masking, so the note itself is still read from the original body.
+    """
+    outside_fences = without_fenced_code_blocks(body)
+
+    human_match = HUMAN_HEADING_RE.search(outside_fences)
     if human_match is None:
         return ""
 
-    agent_match = AGENT_HEADING_RE.search(body, human_match.end())
+    agent_match = AGENT_HEADING_RE.search(outside_fences, human_match.end())
     if agent_match is None:
         return ""
 
@@ -164,7 +173,7 @@ def validate_pr_body(body: str) -> list[str]:
     if len(human_note) < MIN_HUMAN_NOTE_CHARS:
         errors.append("Add a short human-written note between `HUMAN:` and `AGENT:`.")
 
-    if AGENT_HEADING_RE.search(body) is None:
+    if AGENT_HEADING_RE.search(without_fenced_code_blocks(body)) is None:
         errors.append("Keep the `AGENT:` marker from the PR template.")
 
     sections = extract_sections(body)
diff --git a/.github/scripts/markdown_sections.py b/.github/scripts/markdown_sections.py
new file mode 100644
index 0000000000..9f34a6ea69
--- /dev/null
+++ b/.github/scripts/markdown_sections.py
@@ -0,0 +1,95 @@
+"""Helpers for parsing Markdown sections in GitHub issue and PR bodies."""
+
+import re
+from dataclasses import dataclass
+
+
+# CommonMark allows an opening fence to carry leading indentation, and inside a
+# list item that indentation is measured from the list's content column rather
+# than the left margin. Rather than parse lists, accept any indentation on the
+# opening fence and bound the closing fence relative to it (below).
+FENCE_LINE_RE = re.compile(
+    r"^(?P<indent>[ ]*)(?P<marker>`{3,}|~{3,})(?P<rest>[^\r\n]*)"
+)
+
+# CommonMark lets a closing fence sit up to three spaces further in than the
+# fence it closes; beyond that it is content, not a terminator.
+MAX_CLOSING_INDENT_OFFSET = 3
+
+
+def _mask_line(line: str) -> str:
+    """Replace line content while preserving offsets and line endings."""
+    return "".join(char if char in "\r\n" else " " for char in line)
+
+
+@dataclass(frozen=True)
+class _Fence:
+    """The marker used to open a fenced Markdown code block."""
+
+    char: str
+    length: int
+    indent: int
+
+    @classmethod
+    def opened_by(cls, line: str) -> "_Fence | None":
+        """Return the fence opened by ``line``, if any."""
+        match = FENCE_LINE_RE.match(line)
+        if match is None:
+            return None
+        marker = match.group("marker")
+        return cls(
+            char=marker[0],
+            length=len(marker),
+            indent=len(match.group("indent")),
+        )
+
+    def closed_by(self, line: str) -> bool:
+        """Return whether ``line`` closes this fence."""
+        match = FENCE_LINE_RE.match(line)
+        if match is None:
+            return False
+        marker = match.group("marker")
+        return (
+            marker[0] == self.char
+            and len(marker) >= self.length
+            and len(match.group("indent")) <= self.indent + MAX_CLOSING_INDENT_OFFSET
+            and not match.group("rest").strip()
+        )
+
+
+def without_fenced_code_blocks(body: str) -> str:
+    """Mask fenced code blocks without changing character offsets.
+
+    A fence that is never closed is left untouched. CommonMark would run it to
+    the end of the document, but these parsers gate contributions: one stray
+    marker in a pasted log would hide every heading after it and reject a report
+    whose sections are plainly there. Masking only balanced fences keeps the
+    failure on the side of accepting.
+    """
+    masked_lines: list[str] = []
+    fenced_lines: list[str] = []
+    fence: _Fence | None = None
+
+    for line in body.splitlines(keepends=True):
+        if fence is None:
+            fence = _Fence.opened_by(line)
+            if fence is None:
+                masked_lines.append(line)
+            else:
+                fenced_lines.append(line)
+        else:
+            fenced_lines.append(line)
+            if fence.closed_by(line):
+                masked_lines.extend(_mask_line(text) for text in fenced_lines)
+                fenced_lines.clear()
+                fence = None
+
+    # Whatever is left belongs to an unclosed fence, so it stays as written.
+    masked_lines.extend(fenced_lines)
+
+    return "".join(masked_lines)
+
+
+def find_headings(body: str, heading_re: re.Pattern[str]) -> list[re.Match[str]]:
+    """Return heading matches outside fenced code blocks."""
+    return list(heading_re.finditer(without_fenced_code_blocks(body)))
diff --git a/.github/workflows/issue-readiness-check.yml b/.github/workflows/issue-readiness-check.yml
index a7b742a478..f76ff36020 100644
--- a/.github/workflows/issue-readiness-check.yml
+++ b/.github/workflows/issue-readiness-check.yml
@@ -11,12 +11,14 @@ name: Issue Readiness Check
 # Acceptance Criteria sections, the latter with at least one checklist item.
 #
 # When criteria are met the label is added (idempotently); when they are not the
-# label is removed. A feedback comment is posted (or updated) in three cases:
+# label is removed. A feedback comment is posted (or updated) in four cases:
 #   1. The issue is first opened or reopened — always comment so the author
 #      knows whether their issue is ready or what is missing.
 #   2. The label is being added (issue became ready) — comment celebrating it.
 #   3. The label is being removed (issue was ready but is no longer) — comment
 #      explaining what changed.
+#   4. The ready-for-dev label is applied manually — comment confirming whether
+#      the issue meets the criteria.
 # Edits that do not change the label state do not produce a new comment. All
 # comments are upserted by a hidden marker so there is at most one per issue.
 
@@ -48,7 +50,10 @@ jobs:
                   GITHUB_EVENT_PATH: ${{ github.event_path }}
               run: |
                   set -euo pipefail
-                  python .github/scripts/check_issue_readiness.py --json > /tmp/result.json
+                  # The script exits 1 for a not-ready issue; `|| true` keeps
+                  # pipefail from aborting the workflow before label/comment
+                  # handling. The JSON result stays machine-readable.
+                  python .github/scripts/check_issue_readiness.py --json > /tmp/result.json || true
                   echo "ready=$(jq -r '.ready' /tmp/result.json)" >> "$GITHUB_OUTPUT"
                   # Write reasons to a file so the comment step can read them
                   # without shell-escaping multiline text.
@@ -89,6 +94,7 @@ jobs:
               if: >-
                   github.event.action == 'opened'
                   || github.event.action == 'reopened'
+                  || (github.event.action == 'labeled' && github.event.label.name == 'ready-for-dev')
                   || (steps.readiness.outputs.ready == 'true' && !contains(github.event.issue.labels.*.name, 'ready-for-dev'))
                   || (steps.readiness.outputs.ready != 'true' && contains(github.event.issue.labels.*.name, 'ready-for-dev'))
               env:
diff --git a/tests/cross/test_check_issue_readiness.py b/tests/cross/test_check_issue_readiness.py
index 67e21112d6..44af6b8608 100644
--- a/tests/cross/test_check_issue_readiness.py
+++ b/tests/cross/test_check_issue_readiness.py
@@ -1,14 +1,15 @@
 from __future__ import annotations
 
 import importlib.util
+import json
 import sys
 from pathlib import Path
 
 
-def _load_prod_module():
-    repo_root = Path(__file__).resolve().parents[2]
-    script_path = repo_root / ".github" / "scripts" / "check_issue_readiness.py"
-    name = "check_issue_readiness"
+def _load(name: str, script_name: str):
+    script_path = (
+        Path(__file__).resolve().parents[2] / ".github" / "scripts" / script_name
+    )
     spec = importlib.util.spec_from_file_location(name, script_path)
     assert spec and spec.loader
     mod = importlib.util.module_from_spec(spec)
@@ -17,9 +18,13 @@ def _load_prod_module():
     return mod
 
 
-_prod = _load_prod_module()
+# Import markdown_sections first so check_issue_readiness can resolve its
+# `from markdown_sections import ...` against the module we loaded above.
+_load("markdown_sections", "markdown_sections.py")
+_prod = _load("check_issue_readiness", "check_issue_readiness.py")
 evaluate_readiness = _prod.evaluate_readiness
 extract_sections = _prod.extract_sections
+main = _prod.main
 
 
 ENHANCEMENT_READY = """### Problem or Use Case
@@ -114,3 +119,117 @@ def test_no_bug_or_enhancement_label_not_ready():
     result = evaluate_readiness(ENHANCEMENT_READY, [])
     assert result.ready is False
     assert any("bug" in r and "enhancement" in r for r in result.reasons)
+
+
+def test_extract_sections_ignores_heading_inside_fence():
+    body = """### Notes
+The template says:
+
+```markdown
+### Acceptance Criteria
+- [ ] Add criteria here
+```
+"""
+    sections = extract_sections(body)
+    assert set(sections) == {"notes"}
+
+
+def test_fenced_heading_does_not_truncate_actual_behavior():
+    body = """### Actual Behavior
+I ran `pytest` and saw:
+
+~~~text
+### Error detail
+something went wrong
+~~~
+
+### Acceptance Criteria
+- [ ] The bug is fixed
+"""
+    sections = extract_sections(body)
+    assert "error detail" not in sections
+    assert "pytest" in sections["actual behavior"]
+    assert evaluate_readiness(body, ["bug"]).ready
+
+
+def test_unclosed_fence_does_not_swallow_later_sections():
+    """One stray marker in a log paste must not reject an otherwise-ready report."""
+    body = """### Actual Behavior
+I ran `python repro.py` and saw the crash below.
+
+### Relevant Logs
+```shell
+Traceback (most recent call last):
+  the paste was cut off before the closing fence
+
+### Acceptance Criteria
+- [ ] The bug is fixed
+"""
+    sections = extract_sections(body)
+    assert {"actual behavior", "relevant logs", "acceptance criteria"} <= set(sections)
+    assert evaluate_readiness(body, ["bug"]).ready
+
+
+def _run_main(monkeypatch, argv: list[str]) -> int:
+    monkeypatch.setattr("sys.argv", ["check_issue_readiness.py", *argv])
+    return main()
+
+
+def test_main_json_ready(tmp_path, capsys, monkeypatch):
+    body_file = tmp_path / "issue.md"
+    body_file.write_text(BUG_READY)
+    exit_code = _run_main(
+        monkeypatch, ["--body-file", str(body_file), "--labels", "bug", "--json"]
+    )
+    assert exit_code == 0
+    data = json.loads(capsys.readouterr().out)
+    assert data["ready"] is True
+    assert data["reasons"] == []
+
+
+def test_main_json_not_ready_stays_machine_readable(tmp_path, capsys, monkeypatch):
+    """Not-ready JSON exits 1 but still prints parseable JSON on stdout.
+
+    The workflow absorbs the exit code with `|| true` so `set -euo pipefail`
+    does not abort label/comment handling.
+    """
+    body_file = tmp_path / "issue.md"
+    body_file.write_text("### Actual Behavior\n\nIt broke.\n")
+    exit_code = _run_main(
+        monkeypatch, ["--body-file", str(body_file), "--labels", "bug", "--json"]
+    )
+    assert exit_code == 1
+    data = json.loads(capsys.readouterr().out)
+    assert data["ready"] is False
+    assert len(data["reasons"]) > 0
+
+
+def test_main_text_ready(tmp_path, capsys, monkeypatch):
+    body_file = tmp_path / "issue.md"
+    body_file.write_text(BUG_READY)
+    exit_code = _run_main(
+        monkeypatch, ["--body-file", str(body_file), "--labels", "bug"]
+    )
+    assert exit_code == 0
+    assert "Issue meets ready-for-dev criteria." in capsys.readouterr().out
+
+
+def test_main_text_not_ready(tmp_path, capsys, monkeypatch):
+    body_file = tmp_path / "issue.md"
+    body_file.write_text("### Actual Behavior\n\nIt broke.\n")
+    exit_code = _run_main(
+        monkeypatch, ["--body-file", str(body_file), "--labels", "bug"]
+    )
+    assert exit_code == 1
+    assert "Issue does not meet ready-for-dev criteria:" in capsys.readouterr().out
+
+
+def test_main_event_path_json_ready(tmp_path, capsys, monkeypatch):
+    event_file = tmp_path / "event.json"
+    event_file.write_text(
+        json.dumps({"issue": {"body": BUG_READY, "labels": [{"name": "bug"}]}})
+    )
+    exit_code = _run_main(monkeypatch, ["--event-path", str(event_file), "--json"])
+    assert exit_code == 0
+    data = json.loads(capsys.readouterr().out)
+    assert data["ready"] is True
diff --git a/tests/cross/test_check_pr_description.py b/tests/cross/test_check_pr_description.py
index 91220c9fda..2d4a836590 100644
--- a/tests/cross/test_check_pr_description.py
+++ b/tests/cross/test_check_pr_description.py
@@ -6,10 +6,10 @@
 from pathlib import Path
 
 
-def _load_prod_module():
-    repo_root = Path(__file__).resolve().parents[2]
-    script_path = repo_root / ".github" / "scripts" / "check_pr_description.py"
-    name = "check_pr_description"
+def _load(name: str, script_name: str):
+    script_path = (
+        Path(__file__).resolve().parents[2] / ".github" / "scripts" / script_name
+    )
     spec = importlib.util.spec_from_file_location(name, script_path)
     assert spec and spec.loader
     mod = importlib.util.module_from_spec(spec)
@@ -18,9 +18,13 @@ def _load_prod_module():
     return mod
 
 
-_prod = _load_prod_module()
+# Import markdown_sections first so check_pr_description can resolve its
+# `from markdown_sections import ...` against the module we loaded above.
+_load("markdown_sections", "markdown_sections.py")
+_prod = _load("check_pr_description", "check_pr_description.py")
 validate_pr_body = _prod.validate_pr_body
 body_from_event = _prod.body_from_event
+extract_human_note = _prod.extract_human_note
 extract_linked_issue_numbers = _prod.extract_linked_issue_numbers
 validate_linked_issue_ready = _prod.validate_linked_issue_ready
 fetch_issue_details = _prod.fetch_issue_details
@@ -105,6 +109,44 @@ def test_optional_template_sections_may_be_removed():
     assert validate_pr_body(body) == []
 
 
+FENCED_TEMPLATE_BODY = """HUMAN:
+
+Quoting the template I was asked to fill in:
+
+```
+HUMAN:
+I really did test this thoroughly by hand, twice, on my own machine.
+AGENT:
+```
+"""
+
+
+def test_fenced_human_marker_does_not_supply_the_note():
+    assert extract_human_note(FENCED_TEMPLATE_BODY) == ""
+
+
+def test_fenced_agent_marker_does_not_satisfy_validation():
+    errors = validate_pr_body(FENCED_TEMPLATE_BODY)
+    assert "Add a short human-written note between `HUMAN:` and `AGENT:`." in errors
+    assert "Keep the `AGENT:` marker from the PR template." in errors
+
+
+def test_real_markers_still_supply_the_note():
+    body = """HUMAN:
+
+I ran the checker suite and reproduced the fenced-heading case by hand.
+
+AGENT:
+
+## Why
+x
+"""
+    assert "reproduced the fenced-heading case" in extract_human_note(body)
+    errors = validate_pr_body(body)
+    assert "Keep the `AGENT:` marker from the PR template." not in errors
+    assert "Add a short human-written note between `HUMAN:` and `AGENT:`." not in errors
+
+
 def test_body_from_event_reads_pull_request_body(tmp_path: Path):
     event_path = tmp_path / "event.json"
     event_path.write_text(
diff --git a/tests/cross/test_refresh_linked_pr_checks.py b/tests/cross/test_refresh_linked_pr_checks.py
index d8754c4598..77a15d9c8b 100644
--- a/tests/cross/test_refresh_linked_pr_checks.py
+++ b/tests/cross/test_refresh_linked_pr_checks.py
@@ -18,8 +18,9 @@ def _load(name: str, script_name: str):
     return mod
 
 
-# Import check_pr_description first so refresh_linked_pr_checks can resolve its
-# `from check_pr_description import ...` against the module we loaded above.
+# Import markdown_sections and check_pr_description first so the scripts can
+# resolve their imports against the modules we loaded above.
+_load("markdown_sections", "markdown_sections.py")
 _load("check_pr_description", "check_pr_description.py")
 _prod = _load("refresh_linked_pr_checks", "refresh_linked_pr_checks.py")
 
@@ -94,6 +95,25 @@ def test_reruns_linked_pr_check_when_readiness_label_changes(monkeypatch, tmp_pa
     assert seen == [("org/repo", "abc123")]
 
 
+def test_reruns_linked_pr_check_when_readiness_label_removed(monkeypatch, tmp_path):
+    """Losing `ready-for-dev` must also refresh linked PR gates (unlabeled)."""
+    _write_event(monkeypatch, _event(action="unlabeled"), tmp_path)
+    monkeypatch.setattr(
+        _prod,
+        "_linked_open_prs",
+        lambda repo, num: [{"number": 7, "headRefOid": "abc123"}],
+    )
+    monkeypatch.setattr(_prod, "_run", lambda args: _FakeProc("Fixes #12"))
+    seen = []
+    monkeypatch.setattr(
+        _prod,
+        "_rerun_pr_description_check",
+        lambda repo, sha: (seen.append((repo, sha)) or True),
+    )
+    assert _prod.main() == 0
+    assert seen == [("org/repo", "abc123")]
+
+
 def test_skips_cross_referenced_pr_that_does_not_link_issue(monkeypatch, tmp_path):
     _write_event(monkeypatch, _event(), tmp_path)
     monkeypatch.setattr(