Skip to content

Commit f42c8d9

Browse files
TMHSDigitalclaude
andcommitted
fix(drift-check): honor --gh-token and harden sticky-issue upsert
The token passed via --gh-token or DRIFT_CHECK_TOKEN is now exported as GH_TOKEN for every gh call instead of being dropped. Issue lookup asks gh for each candidate's body and only accepts an exact marker match, so a full-text near miss can no longer be edited. Bodies go through --body-file - on stdin and are truncated under GitHub's size limit, and gh calls have a timeout. Closes #107 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: TMHSDigital <154358121+TMHSDigital@users.noreply.github.com>
1 parent 57ffcb1 commit f42c8d9

3 files changed

Lines changed: 139 additions & 34 deletions

File tree

‎scripts/drift_check/cli.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,13 +430,14 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
430430
findings,
431431
meta_commit=args.meta_commit,
432432
repo=args.sticky_issue_repo,
433+
gh_token=gh_token,
433434
)
434435
except RuntimeError as exc:
435436
print(f"error: sticky-issue upsert failed: {exc}", file=sys.stderr)
436437
return 2
437438
sys.stdout.write(
438439
f"Sticky issue: {action}"
439-
+ (f" — {url}" if url else "")
440+
+ (f" - {url}" if url else "")
440441
+ "\n"
441442
)
442443

@@ -472,7 +473,7 @@ def _apply_fixes_live(
472473
473474
Fixable = ``version-signal`` error findings (missing or malformed
474475
signal, or MAJOR.MINOR/patch drift). Pragma-skipped and tool_newer
475-
findings are left alone — those require human review.
476+
findings are left alone; those require human review.
476477
"""
477478
fm = scripts_dir / "add_frontmatter.py"
478479
cm = scripts_dir / "add_comment_marker.py"

‎scripts/drift_check/report/issue.py‎

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from __future__ import annotations
2222

2323
import json
24+
import os
2425
import subprocess
2526
import sys
2627
from datetime import datetime, timezone
@@ -39,23 +40,65 @@
3940
IssueAction = Literal["created", "updated", "reopened", "closed", "no_op"]
4041

4142

42-
# Runner type: takes a list of args, returns (rc, stdout, stderr).
43-
GhRunner = Callable[[Sequence[str]], "tuple[int, str, str]"]
43+
# GitHub rejects issue bodies over 65,536 characters. Leave headroom for
44+
# the truncation notice.
45+
MAX_BODY_CHARS = 60_000
46+
GH_TIMEOUT_SECONDS = 120
47+
48+
# Runner type: takes a list of args plus an optional ``stdin`` keyword
49+
# (used for ``--body-file -``), returns (rc, stdout, stderr).
50+
GhRunner = Callable[..., "tuple[int, str, str]"]
51+
52+
53+
def make_gh_runner(gh_token: str | None = None) -> GhRunner:
54+
"""Build the default subprocess runner. When *gh_token* is given it is
55+
exported as ``GH_TOKEN`` for every ``gh`` call, so the token the caller
56+
supplied is the one used for writes (not whatever ambient login or
57+
``GITHUB_TOKEN`` happens to exist)."""
58+
env = {**os.environ}
59+
if gh_token:
60+
env["GH_TOKEN"] = gh_token
61+
62+
def _run(args: Sequence[str], stdin: str | None = None) -> "tuple[int, str, str]":
63+
try:
64+
result = subprocess.run(
65+
["gh", *args],
66+
input=stdin,
67+
capture_output=True,
68+
text=True,
69+
env=env,
70+
timeout=GH_TIMEOUT_SECONDS,
71+
)
72+
except subprocess.TimeoutExpired:
73+
return 124, "", f"gh {' '.join(args[:2])} timed out after {GH_TIMEOUT_SECONDS}s"
74+
return result.returncode, result.stdout, result.stderr
75+
76+
return _run
4477

4578

46-
def _default_gh_runner(args: Sequence[str]) -> "tuple[int, str, str]":
47-
"""Default subprocess runner: invokes ``gh`` with the given args."""
48-
result = subprocess.run(
49-
["gh", *args],
50-
capture_output=True,
51-
text=True,
52-
)
53-
return result.returncode, result.stdout, result.stderr
79+
def _default_gh_runner(args: Sequence[str], stdin: str | None = None) -> "tuple[int, str, str]":
80+
"""Default runner using the ambient ``gh`` authentication."""
81+
return make_gh_runner()(args, stdin=stdin)
82+
83+
84+
def _truncate_body(body: str) -> str:
85+
if len(body) <= MAX_BODY_CHARS:
86+
return body
87+
notice = "\n".join([
88+
"",
89+
"",
90+
"---",
91+
"",
92+
"_Report truncated to fit GitHub's issue size limit. "
93+
"The full report is in the drift-check workflow run summary._",
94+
"",
95+
])
96+
return body[: MAX_BODY_CHARS - len(notice)] + notice
5497

5598

5699
def _has_blocking_findings(findings: Iterable[Finding]) -> bool:
57100
"""Anything that should make the sticky issue exist (open). ``info`` is
58-
intentionally excluded — info findings do not justify a sticky issue."""
101+
intentionally excluded; info findings do not justify a sticky issue."""
59102
return any(f.severity in ("error", "warn") for f in findings)
60103

61104

@@ -95,23 +138,28 @@ def _find_sticky_issue(
95138
repo: str,
96139
runner: GhRunner,
97140
) -> Optional[dict]:
98-
"""Search by marker. Returns the first matching issue (open OR
99-
closed), or None.
141+
"""Search by marker. Returns the first issue (open OR closed) whose
142+
body actually contains the marker, or None.
100143
101-
The ``gh issue list --search`` matches the body, which is what we
102-
want — title may be edited by humans, marker is structural."""
144+
``gh issue list --search`` is a tokenized full-text search, so it can
145+
return near-misses; each candidate's body is checked for the exact
146+
marker. Title is not used because humans may edit it; the marker is
147+
structural."""
103148
rc, out, err = runner([
104149
"issue", "list",
105150
"--repo", repo,
106151
"--state", "all",
107152
"--search", STICKY_MARKER,
108-
"--json", "number,state,title,url",
109-
"--limit", "5",
153+
"--json", "number,state,title,url,body",
154+
"--limit", "20",
110155
])
111156
if rc != 0:
112157
raise RuntimeError(f"gh issue list failed: {err.strip() or out.strip()}")
113158
items = json.loads(out or "[]")
114-
return items[0] if items else None
159+
for item in items:
160+
if STICKY_MARKER in (item.get("body") or ""):
161+
return item
162+
return None
115163

116164

117165
def upsert_sticky_issue(
@@ -122,12 +170,14 @@ def upsert_sticky_issue(
122170
repo: str = "TMHSDigital/Developer-Tools-Directory",
123171
runner: GhRunner | None = None,
124172
now: datetime | None = None,
173+
gh_token: str | None = None,
125174
) -> "tuple[IssueAction, Optional[str]]":
126175
"""Run the sticky-issue state machine.
127176
128177
Returns ``(action, issue_url)``. ``issue_url`` is None for ``no_op``.
178+
*gh_token*, when given, authenticates the default runner.
129179
"""
130-
runner = runner or _default_gh_runner
180+
runner = runner or make_gh_runner(gh_token)
131181
findings = list(findings)
132182
snapshots = list(snapshots)
133183

@@ -144,7 +194,7 @@ def upsert_sticky_issue(
144194
return "closed", existing["url"]
145195
return "no_op", existing["url"] if existing else None
146196

147-
body = _build_issue_body(snapshots, findings, meta_commit, now=now)
197+
body = _truncate_body(_build_issue_body(snapshots, findings, meta_commit, now=now))
148198

149199
if existing is None:
150200
url = _gh_create(repo, STICKY_TITLE, body, runner)
@@ -171,8 +221,8 @@ def _gh_create(repo: str, title: str, body: str, runner: GhRunner) -> str:
171221
"issue", "create",
172222
"--repo", repo,
173223
"--title", title,
174-
"--body", body,
175-
])
224+
"--body-file", "-",
225+
], stdin=body)
176226
if rc != 0:
177227
raise RuntimeError(f"gh issue create failed: {err.strip() or out.strip()}")
178228
# gh prints the issue URL on stdout.
@@ -183,8 +233,8 @@ def _gh_edit(repo: str, number: int, body: str, runner: GhRunner) -> None:
183233
rc, out, err = runner([
184234
"issue", "edit", str(number),
185235
"--repo", repo,
186-
"--body", body,
187-
])
236+
"--body-file", "-",
237+
], stdin=body)
188238
if rc != 0:
189239
raise RuntimeError(f"gh issue edit failed: {err.strip() or out.strip()}")
190240

@@ -211,7 +261,7 @@ def _gh_comment(repo: str, number: int, body: str, runner: GhRunner) -> None:
211261
rc, out, err = runner([
212262
"issue", "comment", str(number),
213263
"--repo", repo,
214-
"--body", body,
215-
])
264+
"--body-file", "-",
265+
], stdin=body)
216266
if rc != 0:
217267
raise RuntimeError(f"gh issue comment failed: {err.strip() or out.strip()}")

‎tests/test_issue_upsert.py‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,12 @@ def __init__(self, queue=None, handlers=None):
5555
self.queue = list(queue or [])
5656
self.handlers = handlers or {}
5757
self.calls = []
58+
self.stdins = []
5859

59-
def __call__(self, args: Sequence[str]):
60+
def __call__(self, args: Sequence[str], stdin=None):
6061
args = list(args)
6162
self.calls.append(args)
63+
self.stdins.append(stdin)
6264
for key, handler in self.handlers.items():
6365
if args[: len(key)] == list(key):
6466
return handler(args)
@@ -92,8 +94,8 @@ def test_create_when_no_existing_and_drift():
9294
create_args = runner.calls[1]
9395
assert create_args[0] == "issue"
9496
assert create_args[1] == "create"
95-
body_idx = create_args.index("--body") + 1
96-
body = create_args[body_idx]
97+
assert create_args[create_args.index("--body-file") + 1] == "-"
98+
body = runner.stdins[1]
9799
assert STICKY_MARKER in body
98100
assert "abc123" in body # meta commit appears in body
99101
assert STICKY_TITLE in create_args
@@ -107,6 +109,7 @@ def test_update_when_existing_open_and_drift():
107109
"number": 7,
108110
"state": "OPEN",
109111
"title": STICKY_TITLE,
112+
"body": STICKY_MARKER,
110113
"url": "https://github.com/.../issues/7",
111114
}])
112115
runner = _ScriptedRunner(queue=[
@@ -130,6 +133,7 @@ def test_reopen_when_existing_closed_and_drift():
130133
"number": 11,
131134
"state": "CLOSED",
132135
"title": STICKY_TITLE,
136+
"body": STICKY_MARKER,
133137
"url": "https://github.com/.../issues/11",
134138
}])
135139
runner = _ScriptedRunner(queue=[
@@ -149,8 +153,8 @@ def test_reopen_when_existing_closed_and_drift():
149153
# Verify comment includes "re-detected"
150154
comment_calls = [c for c in runner.calls if c[:3] == ["issue", "comment", "11"]]
151155
assert len(comment_calls) == 1
152-
body_idx = comment_calls[0].index("--body") + 1
153-
assert "re-detected" in comment_calls[0][body_idx]
156+
comment_idx = runner.calls.index(comment_calls[0])
157+
assert "re-detected" in runner.stdins[comment_idx]
154158

155159

156160
# ------------------------------- close ------------------------------------
@@ -161,6 +165,7 @@ def test_close_when_existing_open_and_clean():
161165
"number": 9,
162166
"state": "OPEN",
163167
"title": STICKY_TITLE,
168+
"body": STICKY_MARKER,
164169
"url": "https://github.com/.../issues/9",
165170
}])
166171
runner = _ScriptedRunner(queue=[
@@ -197,6 +202,7 @@ def test_no_op_when_clean_and_existing_closed():
197202
"number": 9,
198203
"state": "CLOSED",
199204
"title": STICKY_TITLE,
205+
"body": STICKY_MARKER,
200206
"url": "https://github.com/.../issues/9",
201207
}])
202208
runner = _ScriptedRunner(queue=[(0, existing, "")])
@@ -253,8 +259,56 @@ def test_body_includes_marker_and_metadata():
253259
[_snap()], [_err()], meta_commit="cafef00d",
254260
runner=runner, now=fixed,
255261
)
256-
body = runner.calls[1][runner.calls[1].index("--body") + 1]
262+
body = runner.stdins[1]
257263
assert STICKY_MARKER in body
258264
assert "cafef00d" in body
259265
assert "2026-04-24T22:00:00+00:00" in body
260266
assert "1 errors, 0 warnings" in body
267+
268+
269+
# ------------------------------- lookup / auth / size ----------------------
270+
271+
272+
def test_search_hit_without_exact_marker_is_ignored():
273+
near_miss = json.dumps([{
274+
"number": 3,
275+
"state": "OPEN",
276+
"title": "Unrelated issue mentioning drift-check",
277+
"body": "drift check sticky issue v1 discussion",
278+
"url": "https://github.com/.../issues/3",
279+
}])
280+
runner = _ScriptedRunner(queue=[(0, near_miss, ""), (0, "https://x/issues/50", "")])
281+
action, url = upsert_sticky_issue(
282+
[_snap()], [_err()], meta_commit="abc", runner=runner,
283+
)
284+
assert action == "created"
285+
assert runner.calls[1][:2] == ["issue", "create"]
286+
assert "body" in runner.calls[0][runner.calls[0].index("--json") + 1]
287+
288+
289+
def test_oversized_body_is_truncated():
290+
runner = _ScriptedRunner(queue=[(0, "[]", ""), (0, "u", "")])
291+
many = [_err(msg="x" * 500) for _ in range(400)]
292+
upsert_sticky_issue([_snap()], many, meta_commit="abc", runner=runner)
293+
body = runner.stdins[1]
294+
assert len(body) <= issue.MAX_BODY_CHARS
295+
assert "truncated" in body
296+
297+
298+
def test_gh_token_is_exported_to_default_runner(monkeypatch):
299+
seen = {}
300+
301+
class _Result:
302+
returncode = 0
303+
stdout = "[]"
304+
stderr = ""
305+
306+
def fake_run(args, **kwargs):
307+
seen["env"] = kwargs["env"]
308+
seen["timeout"] = kwargs.get("timeout")
309+
return _Result()
310+
311+
monkeypatch.setattr(issue.subprocess, "run", fake_run)
312+
upsert_sticky_issue([_snap()], [], meta_commit="abc", gh_token="tok_123")
313+
assert seen["env"]["GH_TOKEN"] == "tok_123"
314+
assert seen["timeout"]

0 commit comments

Comments
 (0)