From ca2bfb31dcddeabf6b63692f3974adfd3a7f6d96 Mon Sep 17 00:00:00 2001 From: sdairs Date: Wed, 22 Jul 2026 15:10:06 +0100 Subject: [PATCH 1/3] Stream drift issue body to gh via stdin and truncate to GitHub's limit The scheduled drift check crashed with OSError Errno 7 (Argument list too long) when 155 findings produced an issue body larger than Linux's per-argument exec limit, so no drift issue was filed. Pass the body to `gh issue create` through stdin with `--body-file -`, and truncate oversized bodies at a line boundary (re-closing any open code fence or
block) to fit GitHub's 65,536-character issue body cap, with a notice pointing at the --dry-run command for the full report. Fixes #304 Co-Authored-By: Claude Fable 5 --- scripts/check-openapi-drift.py | 41 ++++++++++++++++++++++- scripts/tests/test_check_openapi_drift.py | 33 ++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/scripts/check-openapi-drift.py b/scripts/check-openapi-drift.py index a3dc327..f6edf1c 100755 --- a/scripts/check-openapi-drift.py +++ b/scripts/check-openapi-drift.py @@ -27,6 +27,13 @@ "CLICKHOUSE_OPENAPI_SPEC_URL", "https://api.clickhouse.cloud/v1" ) ISSUE_LABEL = "openapi-drift" +# GitHub rejects issue bodies over 65,536 characters. +MAX_ISSUE_BODY_CHARS = 65536 +TRUNCATION_NOTICE = ( + "\n---\n\n" + "**Report truncated to fit GitHub's issue body limit.** " + "Run `python3 scripts/check-openapi-drift.py --dry-run` locally for the full report.\n" +) def fetch_live_spec() -> dict | None: @@ -156,9 +163,41 @@ def open_drift_issues() -> list[dict]: return json.loads(result.stdout) +def truncate_issue_body(body: str) -> str: + """Cut an oversized body at a line boundary, keeping the markdown well-formed.""" + if len(body) <= MAX_ISSUE_BODY_CHARS: + return body + closers = "```\n
\n" + budget = MAX_ISSUE_BODY_CHARS - len(TRUNCATION_NOTICE) - len(closers) + kept = [] + used = 0 + in_fence = False + in_details = False + for line in body.splitlines(): + if used + len(line) + 1 > budget: + break + kept.append(line) + used += len(line) + 1 + stripped = line.strip() + if stripped.startswith("```"): + in_fence = not in_fence + elif stripped == "
": + in_details = True + elif stripped == "
": + in_details = False + if in_fence: + kept.append("```") + if in_details: + kept.append("") + return "\n".join(kept) + TRUNCATION_NOTICE + + def create_issue(title: str, body: str): + # The body can exceed the kernel's per-argument size limit; feed it via stdin. subprocess.run( - ["gh", "issue", "create", "--title", title, "--body", body, "--label", ISSUE_LABEL], + ["gh", "issue", "create", "--title", title, "--body-file", "-", "--label", ISSUE_LABEL], + input=truncate_issue_body(body), + text=True, check=True, ) diff --git a/scripts/tests/test_check_openapi_drift.py b/scripts/tests/test_check_openapi_drift.py index 8761a94..df8079a 100644 --- a/scripts/tests/test_check_openapi_drift.py +++ b/scripts/tests/test_check_openapi_drift.py @@ -68,6 +68,39 @@ 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_truncate_issue_body_keeps_short_bodies_intact(self): + body = "short body" + self.assertEqual(drift.truncate_issue_body(body), body) + + def test_truncate_issue_body_fits_github_limit_and_closes_markdown(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) + + truncated = drift.truncate_issue_body(body) + self.assertLessEqual(len(truncated), drift.MAX_ISSUE_BODY_CHARS) + self.assertTrue(truncated.endswith(drift.TRUNCATION_NOTICE)) + # The cut lands inside the fenced block; both must be re-closed. + before_notice = truncated[: -len(drift.TRUNCATION_NOTICE)] + self.assertTrue(before_notice.endswith("```\n")) + + @mock.patch.object(drift.subprocess, "run") + def test_create_issue_streams_body_over_stdin(self, run): + body = "x" * (drift.MAX_ISSUE_BODY_CHARS * 3) + drift.create_issue("title", body) + + run.assert_called_once() + args, kwargs = run.call_args + command = args[0] + self.assertIn("--body-file", command) + self.assertIn("-", command) + self.assertNotIn("--body", command) + self.assertNotIn(body, command) + self.assertLessEqual(len(kwargs["input"]), drift.MAX_ISSUE_BODY_CHARS) + self.assertTrue(kwargs["input"].endswith(drift.TRUNCATION_NOTICE)) + @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") From dafd7127488a5a0874b316a6bf436993aaf46f72 Mon Sep 17 00:00:00 2001 From: sdairs Date: Wed, 22 Jul 2026 16:37:48 +0100 Subject: [PATCH 2/3] Overflow oversized drift issue bodies into comments instead of truncating Truncating at 65,536 characters silently dropped the tail of large drift reports. Split the body into GitHub-sized chunks instead, posting the first as the issue body and the rest as comments so nothing is lost. Chunk boundaries prefer the last line outside any code fence or
block so a block never straddles the issue/comment boundary; a single block larger than the limit is still cut mid-block and re-closed as a fallback. Fixes #304 Co-Authored-By: Claude Fable 5 --- scripts/check-openapi-drift.py | 101 +++++++++++++++------- scripts/tests/test_check_openapi_drift.py | 86 +++++++++++++----- 2 files changed, 135 insertions(+), 52 deletions(-) diff --git a/scripts/check-openapi-drift.py b/scripts/check-openapi-drift.py index f6edf1c..9fe383d 100755 --- a/scripts/check-openapi-drift.py +++ b/scripts/check-openapi-drift.py @@ -27,13 +27,10 @@ "CLICKHOUSE_OPENAPI_SPEC_URL", "https://api.clickhouse.cloud/v1" ) ISSUE_LABEL = "openapi-drift" -# GitHub rejects issue bodies over 65,536 characters. +# GitHub rejects issue bodies and comments over 65,536 characters. MAX_ISSUE_BODY_CHARS = 65536 -TRUNCATION_NOTICE = ( - "\n---\n\n" - "**Report truncated to fit GitHub's issue body limit.** " - "Run `python3 scripts/check-openapi-drift.py --dry-run` locally for the full report.\n" -) +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: @@ -163,43 +160,83 @@ def open_drift_issues() -> list[dict]: return json.loads(result.stdout) -def truncate_issue_body(body: str) -> str: - """Cut an oversized body at a line boundary, keeping the markdown well-formed.""" +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 + return [body] closers = "```\n
\n" - budget = MAX_ISSUE_BODY_CHARS - len(TRUNCATION_NOTICE) - len(closers) - kept = [] - used = 0 - in_fence = False - in_details = False - for line in body.splitlines(): - if used + len(line) + 1 > budget: + 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 - kept.append(line) - used += len(line) + 1 - stripped = line.strip() - if stripped.startswith("```"): - in_fence = not in_fence - elif stripped == "
": - in_details = True - elif stripped == "
": - in_details = False - if in_fence: - kept.append("```") - if in_details: - kept.append("
") - return "\n".join(kept) + TRUNCATION_NOTICE + if clean_cut is not None and clean_cut > start: + kept = lines[start:clean_cut] + start = clean_cut + else: + # A single block exceeds the budget; cut mid-block and re-close. + end = max(end, start + 1) + kept = lines[start:end] + if in_fence: + kept.append("```") + if in_details: + kept.append("") + start = end + chunks.append(header + "\n".join(kept) + CONTINUATION_NOTICE) + return chunks def create_issue(title: str, body: str): + chunks = split_issue_body(body) # The body can exceed the kernel's per-argument size limit; feed it via stdin. - subprocess.run( + result = subprocess.run( ["gh", "issue", "create", "--title", title, "--body-file", "-", "--label", ISSUE_LABEL], - input=truncate_issue_body(body), + 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:]: + subprocess.run( + ["gh", "issue", "comment", issue_url, "--body-file", "-"], + input=chunk, + text=True, + check=True, + ) 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 df8079a..93dbc1a 100644 --- a/scripts/tests/test_check_openapi_drift.py +++ b/scripts/tests/test_check_openapi_drift.py @@ -68,38 +68,84 @@ 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_truncate_issue_body_keeps_short_bodies_intact(self): + def test_split_issue_body_keeps_short_bodies_intact(self): body = "short body" - self.assertEqual(drift.truncate_issue_body(body), body) + self.assertEqual(drift.split_issue_body(body), [body]) - def test_truncate_issue_body_fits_github_limit_and_closes_markdown(self): + 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) - truncated = drift.truncate_issue_body(body) - self.assertLessEqual(len(truncated), drift.MAX_ISSUE_BODY_CHARS) - self.assertTrue(truncated.endswith(drift.TRUNCATION_NOTICE)) - # The cut lands inside the fenced block; both must be re-closed. - before_notice = truncated[: -len(drift.TRUNCATION_NOTICE)] - self.assertTrue(before_notice.endswith("```\n
")) + 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) @mock.patch.object(drift.subprocess, "run") - def test_create_issue_streams_body_over_stdin(self, run): - body = "x" * (drift.MAX_ISSUE_BODY_CHARS * 3) + 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) - run.assert_called_once() - args, kwargs = run.call_args - command = args[0] - self.assertIn("--body-file", command) - self.assertIn("-", command) - self.assertNotIn("--body", command) - self.assertNotIn(body, command) - self.assertLessEqual(len(kwargs["input"]), drift.MAX_ISSUE_BODY_CHARS) - self.assertTrue(kwargs["input"].endswith(drift.TRUNCATION_NOTICE)) + 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_analyzer_subprocess_failure_is_fatal(self, run): From 8a0b462e279d08acfb6a95306d483df8567e159e Mon Sep 17 00:00:00 2001 From: sdairs Date: Wed, 22 Jul 2026 17:12:40 +0100 Subject: [PATCH 3/3] Address PR review: cap oversized lines, retry failed continuation comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - split_issue_body now hard-truncates a single line that alone exceeds the chunk budget (visible "… [line truncated]" marker), so every chunk is guaranteed <= MAX_ISSUE_BODY_CHARS for any input. - The continuation notice is only appended when lines actually remain, fixing a false "report continues" footer when the forced branch consumes the final line. - create_issue retries a failed gh issue comment once; on double failure it posts a best-effort "report incomplete" comment on the issue, logs the error, and re-raises so CI still fails loudly instead of leaving a silently partial drift report that blocks future runs. Co-Authored-By: Claude Fable 5 --- scripts/check-openapi-drift.py | 66 +++++++++++++--- scripts/tests/test_check_openapi_drift.py | 94 +++++++++++++++++++++++ 2 files changed, 151 insertions(+), 9 deletions(-) diff --git a/scripts/check-openapi-drift.py b/scripts/check-openapi-drift.py index 9fe383d..02c2b2d 100755 --- a/scripts/check-openapi-drift.py +++ b/scripts/check-openapi-drift.py @@ -205,16 +205,29 @@ def split_issue_body(body: str) -> list[str]: if clean_cut is not None and clean_cut > start: kept = lines[start:clean_cut] start = clean_cut - else: + elif end > start: # A single block exceeds the budget; cut mid-block and re-close. - end = max(end, start + 1) kept = lines[start:end] if in_fence: kept.append("```") if in_details: kept.append("") start = end - chunks.append(header + "\n".join(kept) + CONTINUATION_NOTICE) + 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 @@ -231,12 +244,47 @@ def create_issue(title: str, body: str): issue_url = result.stdout.strip().splitlines()[-1] print(issue_url, file=sys.stderr) for chunk in chunks[1:]: - subprocess.run( - ["gh", "issue", "comment", issue_url, "--body-file", "-"], - input=chunk, - text=True, - check=True, - ) + 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 93dbc1a..a051e63 100644 --- a/scripts/tests/test_check_openapi_drift.py +++ b/scripts/tests/test_check_openapi_drift.py @@ -116,6 +116,33 @@ def test_split_issue_body_recloses_oversized_single_block(self): 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( @@ -147,6 +174,73 @@ def test_create_issue_streams_body_and_overflows_into_comments(self, run): 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")