diff --git a/scripts/check-openapi-drift.py b/scripts/check-openapi-drift.py index a3dc327..02c2b2d 100755 --- a/scripts/check-openapi-drift.py +++ b/scripts/check-openapi-drift.py @@ -27,6 +27,10 @@ "CLICKHOUSE_OPENAPI_SPEC_URL", "https://api.clickhouse.cloud/v1" ) ISSUE_LABEL = "openapi-drift" +# GitHub rejects issue bodies and comments over 65,536 characters. +MAX_ISSUE_BODY_CHARS = 65536 +CONTINUATION_NOTICE = "\n---\n\n**Report continues in the next comment.**\n" +CONTINUATION_HEADER = "**Drift report, continued from above.**\n\n---\n\n" def fetch_live_spec() -> dict | None: @@ -156,11 +160,131 @@ def open_drift_issues() -> list[dict]: return json.loads(result.stdout) +def split_issue_body(body: str) -> list[str]: + """Split an oversized body into GitHub-sized chunks; overflow goes to comments. + + Chunks break at line boundaries, preferring the last point that is outside + any code fence or
block so no block straddles a chunk boundary. + If a single block exceeds the budget, it is cut mid-block and re-closed to + keep the chunk's markdown well-formed. + """ + if len(body) <= MAX_ISSUE_BODY_CHARS: + return [body] + closers = "```\n
\n" + lines = body.splitlines() + chunks = [] + start = 0 + while start < len(lines): + header = CONTINUATION_HEADER if chunks else "" + budget = ( + MAX_ISSUE_BODY_CHARS - len(header) - len(CONTINUATION_NOTICE) - len(closers) + ) + used = 0 + in_fence = False + in_details = False + clean_cut = None + end = start + while end < len(lines): + line = lines[end] + if used + len(line) + 1 > budget: + break + used += len(line) + 1 + end += 1 + stripped = line.strip() + if stripped.startswith("```"): + in_fence = not in_fence + elif stripped == "
": + in_details = True + elif stripped == "
": + in_details = False + if not in_fence and not in_details: + clean_cut = end + if end == len(lines): + chunks.append(header + "\n".join(lines[start:])) + break + if clean_cut is not None and clean_cut > start: + kept = lines[start:clean_cut] + start = clean_cut + elif end > start: + # A single block exceeds the budget; cut mid-block and re-close. + kept = lines[start:end] + if in_fence: + kept.append("```") + if in_details: + kept.append("") + start = end + else: + # A single line alone exceeds the budget; hard-truncate it so the + # emitted chunk is guaranteed to fit, leaving a visible marker. + marker = "… [line truncated]" + cut = max(budget - len(marker) - len(closers), 0) + kept = [lines[start][:cut] + marker] + if in_fence: + kept.append("```") + if in_details: + kept.append("") + start += 1 + # Only advertise a continuation when more lines actually remain; the + # forced branches above can consume the final line of the body. + notice = CONTINUATION_NOTICE if start < len(lines) else "" + chunks.append(header + "\n".join(kept) + notice) + return chunks + + def create_issue(title: str, body: str): - subprocess.run( - ["gh", "issue", "create", "--title", title, "--body", body, "--label", ISSUE_LABEL], + chunks = split_issue_body(body) + # The body can exceed the kernel's per-argument size limit; feed it via stdin. + result = subprocess.run( + ["gh", "issue", "create", "--title", title, "--body-file", "-", "--label", ISSUE_LABEL], + input=chunks[0], + text=True, check=True, + capture_output=True, ) + issue_url = result.stdout.strip().splitlines()[-1] + print(issue_url, file=sys.stderr) + for chunk in chunks[1:]: + try: + subprocess.run( + ["gh", "issue", "comment", issue_url, "--body-file", "-"], + input=chunk, + text=True, + check=True, + ) + except subprocess.CalledProcessError: + # Retry once; transient GitHub failures are common. + try: + subprocess.run( + ["gh", "issue", "comment", issue_url, "--body-file", "-"], + input=chunk, + text=True, + check=True, + ) + except subprocess.CalledProcessError: + # A comment failed twice. Never leave an issue that silently + # looks complete: best-effort flag it as truncated, then fail + # loudly so the CI run still reports the problem. + fallback = ( + "**This drift report is incomplete.** A continuation comment " + "failed to post after a retry; check the workflow logs and " + "re-run the drift check to regenerate the full report.\n" + ) + try: + subprocess.run( + ["gh", "issue", "comment", issue_url, "--body-file", "-"], + input=fallback, + text=True, + check=False, + capture_output=True, + ) + except Exception: # noqa: BLE001 - the fallback must never mask the failure + pass + print( + f"ERROR: failed to post a continuation comment to {issue_url}; " + "the drift report is incomplete.", + file=sys.stderr, + ) + raise def build_issue_body(report: dict, live_spec: dict) -> str: diff --git a/scripts/tests/test_check_openapi_drift.py b/scripts/tests/test_check_openapi_drift.py index 8761a94..a051e63 100644 --- a/scripts/tests/test_check_openapi_drift.py +++ b/scripts/tests/test_check_openapi_drift.py @@ -68,6 +68,179 @@ def test_groups_findings_and_renders_spec_snippets(self): self.assertIn("## Acknowledged Unsupported Enum Constraints", body) self.assertIn("## Enum VALUES Const Mismatches", body) + def test_split_issue_body_keeps_short_bodies_intact(self): + body = "short body" + self.assertEqual(drift.split_issue_body(body), [body]) + + def test_split_issue_body_breaks_between_blocks_and_loses_nothing(self): + # Many small self-contained sections: every chunk boundary should land + # between blocks, never inside a fence or
block. + section = "\n".join( + ["## Section", "", "
", "```json"] + + ['{"filler": "%s"}' % ("x" * 60) for _ in range(50)] + + ["```", "
", ""] + ) + body = "\n".join(section for _ in range(60)) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + chunks = drift.split_issue_body(body) + self.assertGreater(len(chunks), 1) + for chunk in chunks: + self.assertLessEqual(len(chunk), drift.MAX_ISSUE_BODY_CHARS) + self.assertEqual(chunk.count("```") % 2, 0) + self.assertEqual(chunk.count("
"), chunk.count("
")) + for chunk in chunks[:-1]: + self.assertTrue(chunk.endswith(drift.CONTINUATION_NOTICE)) + for chunk in chunks[1:]: + self.assertTrue(chunk.startswith(drift.CONTINUATION_HEADER)) + # No content is dropped: every original line survives in some chunk. + rejoined = "\n".join(chunks) + for line in body.splitlines(): + self.assertIn(line, rejoined) + + def test_split_issue_body_recloses_oversized_single_block(self): + lines = ["
", "```json"] + lines += ['{"filler": %d}' % i for i in range(10_000)] + lines += ["```", "
"] + body = "\n".join(lines) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + chunks = drift.split_issue_body(body) + self.assertGreater(len(chunks), 1) + for chunk in chunks: + self.assertLessEqual(len(chunk), drift.MAX_ISSUE_BODY_CHARS) + # The first cut lands inside the fenced block; both must be re-closed. + first = chunks[0][: -len(drift.CONTINUATION_NOTICE)] + self.assertTrue(first.endswith("```\n
")) + rejoined = "\n".join(chunks) + for line in lines: + self.assertIn(line, rejoined) + + def test_split_issue_body_truncates_oversized_single_line(self): + # A single line longer than the whole budget cannot break at a line + # boundary, so it must be hard-truncated to keep every chunk in bounds. + huge = "x" * (drift.MAX_ISSUE_BODY_CHARS + 5_000) + body = "\n".join(["intro line", huge, "tail line"]) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + chunks = drift.split_issue_body(body) + self.assertGreater(len(chunks), 1) + for chunk in chunks: + self.assertLessEqual(len(chunk), drift.MAX_ISSUE_BODY_CHARS) + self.assertIn("… [line truncated]", "\n".join(chunks)) + + def test_split_issue_body_no_notice_when_final_line_is_oversized(self): + # The forced truncation branch consumes the last line here; the final + # chunk must not falsely advertise that the report continues. + huge = "y" * (drift.MAX_ISSUE_BODY_CHARS + 5_000) + body = "\n".join(["intro line", huge]) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + chunks = drift.split_issue_body(body) + self.assertGreater(len(chunks), 1) + for chunk in chunks: + self.assertLessEqual(len(chunk), drift.MAX_ISSUE_BODY_CHARS) + self.assertIn("… [line truncated]", chunks[-1]) + self.assertFalse(chunks[-1].endswith(drift.CONTINUATION_NOTICE)) + + @mock.patch.object(drift.subprocess, "run") + def test_create_issue_streams_body_and_overflows_into_comments(self, run): + run.return_value = SimpleNamespace( + returncode=0, + stdout="https://github.com/ClickHouse/clickhousectl/issues/999\n", + stderr="", + ) + body = "\n".join("line %d" % i for i in range(20_000)) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + drift.create_issue("title", body) + + self.assertGreater(run.call_count, 1) + create_args, create_kwargs = run.call_args_list[0] + self.assertEqual(create_args[0][:3], ["gh", "issue", "create"]) + self.assertIn("--body-file", create_args[0]) + self.assertNotIn("--body", create_args[0]) + self.assertLessEqual(len(create_kwargs["input"]), drift.MAX_ISSUE_BODY_CHARS) + + seen = create_kwargs["input"] + for comment_args, comment_kwargs in run.call_args_list[1:]: + self.assertEqual(comment_args[0][:3], ["gh", "issue", "comment"]) + self.assertIn( + "https://github.com/ClickHouse/clickhousectl/issues/999", + comment_args[0], + ) + self.assertLessEqual(len(comment_kwargs["input"]), drift.MAX_ISSUE_BODY_CHARS) + seen += comment_kwargs["input"] + # Nothing was dropped across the issue body and its comments. + for line in body.splitlines(): + self.assertIn(line, seen) + + @mock.patch.object(drift.subprocess, "run") + def test_create_issue_raises_and_flags_when_comment_fails_twice(self, run): + def fake_run(cmd, *args, **kwargs): + if cmd[:3] == ["gh", "issue", "create"]: + return SimpleNamespace( + returncode=0, + stdout="https://github.com/ClickHouse/clickhousectl/issues/999\n", + stderr="", + ) + # The fallback truncation comment must succeed (best effort). + if "incomplete" in kwargs.get("input", ""): + return SimpleNamespace(returncode=0, stdout="", stderr="") + # Every real continuation chunk fails, including the retry. + raise drift.subprocess.CalledProcessError(1, cmd) + + run.side_effect = fake_run + body = "\n".join("line %d" % i for i in range(20_000)) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + with self.assertRaises(drift.subprocess.CalledProcessError): + drift.create_issue("title", body) + + comment_inputs = [ + c.kwargs.get("input", "") + for c in run.call_args_list + if c.args[0][:3] == ["gh", "issue", "comment"] + ] + non_fallback = [i for i in comment_inputs if "incomplete" not in i] + fallback = [i for i in comment_inputs if "incomplete" in i] + # The failing chunk was attempted twice (initial + one retry)... + self.assertEqual(len(non_fallback), 2) + self.assertEqual(non_fallback[0], non_fallback[1]) + # ...then a best-effort truncation notice was posted before raising. + self.assertEqual(len(fallback), 1) + + @mock.patch.object(drift.subprocess, "run") + def test_create_issue_recovers_from_transient_comment_failure(self, run): + failed_once = set() + + def fake_run(cmd, *args, **kwargs): + if cmd[:3] == ["gh", "issue", "create"]: + return SimpleNamespace( + returncode=0, + stdout="https://github.com/ClickHouse/clickhousectl/issues/999\n", + stderr="", + ) + chunk = kwargs.get("input", "") + # Fail each chunk's first attempt, then succeed on the retry. + if chunk not in failed_once: + failed_once.add(chunk) + raise drift.subprocess.CalledProcessError(1, cmd) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + run.side_effect = fake_run + body = "\n".join("line %d" % i for i in range(20_000)) + self.assertGreater(len(body), drift.MAX_ISSUE_BODY_CHARS) + + # Transient failures must not raise or leave a truncation notice. + drift.create_issue("title", body) + comment_inputs = [ + c.kwargs.get("input", "") + for c in run.call_args_list + if c.args[0][:3] == ["gh", "issue", "comment"] + ] + self.assertTrue(comment_inputs) + self.assertFalse(any("incomplete" in i for i in comment_inputs)) + @mock.patch.object(drift.subprocess, "run") def test_analyzer_subprocess_failure_is_fatal(self, run): run.return_value = SimpleNamespace(returncode=1, stdout="", stderr="bad source")