Skip to content

Commit 0dbd3a2

Browse files
authored
Fix release draft breaking change notes (#3440)
1 parent da23bfc commit 0dbd3a2

3 files changed

Lines changed: 328 additions & 57 deletions

File tree

.github/workflows/release-draft.yaml

Lines changed: 28 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,27 @@ jobs:
4040
run: |
4141
set -euo pipefail
4242
43+
# Keep trusted verifier inputs outside Claude's writable workspace.
4344
diff_dir="$RUNNER_TEMP/release-draft-pr"
45+
claude_diff_dir="$(mktemp -d "${GITHUB_WORKSPACE:?}/.release-draft-pr.XXXXXX")"
4446
mkdir -p "$diff_dir"
47+
chmod 700 "$diff_dir"
48+
chmod 700 "$claude_diff_dir"
4549
4650
diff_path="$diff_dir/pr.diff"
4751
diffstat_path="$diff_dir/pr.diffstat"
4852
files_path="$diff_dir/pr.files"
4953
deleted_lines_path="$diff_dir/deleted-lines.txt"
5054
context_path="$diff_dir/analysis-context.md"
55+
claude_diff_path="$claude_diff_dir/pr.diff"
56+
claude_files_path="$claude_diff_dir/pr.files"
57+
claude_deleted_lines_path="$claude_diff_dir/deleted-lines.txt"
58+
claude_context_path="$claude_diff_dir/analysis-context.md"
5159
5260
write_pull_request_diff() {
5361
gh pr diff "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" > "$diff_path"
5462
gh pr diff "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --name-only > "$files_path"
55-
sed -n '/^-[^-]/ s/^-//p' "$diff_path" > "$deleted_lines_path"
63+
awk '/^--- / { next } /^-/ { print substr($0, 2) }' "$diff_path" > "$deleted_lines_path"
5664
printf '%s lines\n' "$(wc -l < "$diff_path")" > "$diffstat_path"
5765
}
5866
@@ -67,12 +75,16 @@ jobs:
6775
git diff --name-status "$base_sha" "$MERGE_SHA" > "$files_path"
6876
git diff --no-ext-diff --find-renames --unified=80 "$base_sha" "$MERGE_SHA" > "$diff_path"
6977
git diff --no-ext-diff --find-renames --unified=0 "$base_sha" "$MERGE_SHA" \
70-
| awk '/^-[^-]/ { print substr($0, 2) }' > "$deleted_lines_path"
78+
| awk '/^--- / { next } /^-/ { print substr($0, 2) }' > "$deleted_lines_path"
7179
else
7280
base_sha=""
7381
write_pull_request_diff
7482
fi
7583
84+
cp "$diff_path" "$claude_diff_path"
85+
cp "$files_path" "$claude_files_path"
86+
cp "$deleted_lines_path" "$claude_deleted_lines_path"
87+
7688
{
7789
printf '# Release Draft Analysis Context\n\n'
7890
printf 'PR: #%s\n' "$PR_NUMBER"
@@ -81,20 +93,23 @@ jobs:
8193
printf 'Base SHA: %s\n\n' "${base_sha:-unknown}"
8294
printf 'Use only these prepared artifacts when deciding whether this PR has breaking changes.\n'
8395
printf 'Do not infer changes from unrelated refs, branches, or commits.\n\n'
84-
printf 'Diff path: `%s`\n' "$diff_path"
85-
printf 'Changed files path: `%s`\n' "$files_path"
86-
printf 'Deleted lines path: `%s`\n\n' "$deleted_lines_path"
96+
printf 'Diff path: `%s`\n' "$claude_diff_path"
97+
printf 'Changed files path: `%s`\n' "$claude_files_path"
98+
printf 'Deleted lines path: `%s`\n\n' "$claude_deleted_lines_path"
8799
printf '## Diffstat\n\n```text\n'
88100
cat "$diffstat_path"
89101
printf '\n```\n\n## Changed Files\n\n```text\n'
90102
cat "$files_path"
91103
printf '\n```\n'
92104
} > "$context_path"
105+
cp "$context_path" "$claude_context_path"
106+
chmod 600 "$claude_diff_path" "$claude_files_path" "$claude_deleted_lines_path" "$claude_context_path"
93107
94108
{
95-
echo "diff_path=$diff_path"
96-
echo "deleted_lines_path=$deleted_lines_path"
97-
echo "analysis_context_path=$context_path"
109+
echo "diff_path=$claude_diff_path"
110+
echo "deleted_lines_path=$claude_deleted_lines_path"
111+
echo "analysis_context_path=$claude_context_path"
112+
echo "trusted_deleted_lines_path=$deleted_lines_path"
98113
} >> "$GITHUB_OUTPUT"
99114
100115
- name: Run Claude Code Analysis
@@ -148,64 +163,20 @@ jobs:
148163
claude_args: |
149164
--model claude-opus-4-8
150165
--max-turns 50
166+
--allowedTools Read
151167
--json-schema '{"type":"object","properties":{"has_breaking_changes":{"type":"boolean","description":"Whether this PR contains breaking changes"},"breaking_changes_content":{"type":"string","description":"Formatted breaking changes section content (without ## Breaking Changes header), or empty string if none"},"reasoning":{"type":"string","description":"Brief explanation of why this is or is not a breaking change"}},"required":["has_breaking_changes","breaking_changes_content","reasoning"]}'
152168
153169
- name: Parse Claude output
154170
id: parse
155171
env:
156172
CLAUDE_OUTPUT: ${{ steps.claude.outputs.structured_output }}
157173
ANALYSIS_PATH: ${{ runner.temp }}/release-draft-analysis.json
158-
DELETED_LINES_PATH: ${{ steps.pr-diff.outputs.deleted_lines_path }}
174+
DELETED_LINES_PATH: ${{ steps.pr-diff.outputs.trusted_deleted_lines_path }}
159175
run: |
160176
set -euo pipefail
161-
# Parse structured output from Claude using env var to avoid shell injection
162-
HAS_BC=$(printf '%s\n' "$CLAUDE_OUTPUT" | jq -r '.has_breaking_changes // false')
163-
BC_CONTENT=$(printf '%s\n' "$CLAUDE_OUTPUT" | jq -r '.breaking_changes_content // ""')
164-
REASONING=$(printf '%s\n' "$CLAUDE_OUTPUT" | jq -r '.reasoning // ""')
165-
166-
# Guard against release-note hallucinations caused by comparing against
167-
# unrelated branches: if the analysis claims a backticked option/API was
168-
# removed, that token must be present in the actual deleted lines.
169-
if [ "$HAS_BC" = "true" ] && printf '%s\n' "$BC_CONTENT" | grep -Eiq '(^|\b)(removed|removal|deleted|no longer)\b'; then
170-
if [ ! -s "$DELETED_LINES_PATH" ]; then
171-
{
172-
echo "Release draft analysis claimed removals, but the exact PR diff has no deleted lines:"
173-
printf '%s\n' "$BC_CONTENT"
174-
} >&2
175-
exit 1
176-
fi
177-
178-
invalid_claims=""
179-
while IFS= read -r line; do
180-
if ! printf '%s\n' "$line" | grep -Eiq '(^|\b)(removed|removal|deleted|no longer)\b'; then
181-
continue
182-
fi
183-
while IFS= read -r token; do
184-
if ! printf '%s\n' "$token" | grep -Eq '^[[:alnum:]_.-]+(\(\))?$'; then
185-
continue
186-
fi
187-
normalized_token="${token%()}"
188-
if ! grep -F -- "$normalized_token" "$DELETED_LINES_PATH" >/dev/null; then
189-
invalid_claims="${invalid_claims}"$'\n'"${line}"$'\n'" Missing deleted token: ${normalized_token}"
190-
fi
191-
done < <(printf '%s\n' "$line" | grep -oE '`[^`]+`' | tr -d '`' || true)
192-
done < <(printf '%s\n' "$BC_CONTENT")
193-
194-
if [ -n "$invalid_claims" ]; then
195-
{
196-
echo "Release draft analysis claimed removals that are not present in the exact PR diff:"
197-
printf '%s\n' "$invalid_claims"
198-
} >&2
199-
exit 1
200-
fi
201-
fi
202-
203-
jq -n \
204-
--argjson has_breaking_changes "$HAS_BC" \
205-
--arg breaking_changes_content "$BC_CONTENT" \
206-
--arg reasoning "$REASONING" \
207-
'{has_breaking_changes: $has_breaking_changes, breaking_changes_content: $breaking_changes_content, reasoning: $reasoning}' \
208-
> "$ANALYSIS_PATH"
177+
python3 scripts/validate_release_draft_analysis.py \
178+
--analysis-path "$ANALYSIS_PATH" \
179+
--deleted-lines-path "$DELETED_LINES_PATH"
209180
210181
- name: Upload analysis artifact
211182
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
"""Validate Claude release draft analysis before updating the draft release."""
2+
3+
from __future__ import annotations
4+
5+
import argparse
6+
import json
7+
import os
8+
import re
9+
import sys
10+
from pathlib import Path
11+
from typing import Any
12+
13+
REMOVAL_PATTERNS = [
14+
re.compile(r"\b(?:removed|deleted|dropped)\b(?:(?!`).){0,80}`(?P<token>[^`\n]+)`", re.IGNORECASE),
15+
re.compile(r"`(?P<token>[^`\n]+)`(?:(?!`).){0,80}\b(?:removed|deleted|dropped)\b", re.IGNORECASE),
16+
re.compile(
17+
r"\bno longer\s+(?:supports?|accepts?|recognizes?|exports?|provides?|includes?|allows?|has)\b"
18+
r"(?:(?!`).){0,80}`(?P<token>[^`\n]+)`",
19+
re.IGNORECASE,
20+
),
21+
re.compile(
22+
r"`(?P<token>[^`\n]+)`(?:(?!`).){0,80}\bno longer\s+"
23+
r"(?:available|exists|supported|accepted|recognized|exported|provided|included|allowed)\b",
24+
re.IGNORECASE,
25+
),
26+
]
27+
UNREADABLE_DIFF_RE = re.compile(
28+
r"\b(?:unable|could not|can't|cannot)\b.{0,160}\b(?:read|access)\b.{0,160}"
29+
r"\b(?:prepared diff|analysis artifact|diff file|diff)\b",
30+
re.IGNORECASE | re.DOTALL,
31+
)
32+
TOKEN_BOUNDARY_CHARS = r"A-Za-z0-9_.:/-"
33+
34+
35+
def _parse_args() -> argparse.Namespace:
36+
"""Parse validator CLI arguments."""
37+
parser = argparse.ArgumentParser()
38+
parser.add_argument("--analysis-path", required=True, type=Path)
39+
parser.add_argument("--deleted-lines-path", required=True, type=Path)
40+
return parser.parse_args()
41+
42+
43+
def _parse_claude_output(raw_output: str) -> dict[str, Any]:
44+
"""Parse Claude's structured output as a JSON object."""
45+
if not raw_output:
46+
message = "Claude structured output is empty"
47+
raise SystemExit(message)
48+
try:
49+
parsed = json.loads(raw_output)
50+
except json.JSONDecodeError as exc:
51+
message = f"Invalid Claude structured output JSON: {exc}"
52+
raise SystemExit(message) from exc
53+
if isinstance(parsed, dict):
54+
return parsed
55+
message = "Claude structured output must be a JSON object"
56+
raise SystemExit(message)
57+
58+
59+
def _as_bool(value: Any) -> bool:
60+
"""Normalize Claude's boolean-like field values."""
61+
result = False
62+
match value:
63+
case bool():
64+
result = value
65+
case str() if value.lower() == "true":
66+
result = True
67+
case _:
68+
result = False
69+
return result
70+
71+
72+
def _as_string(value: Any) -> str:
73+
"""Normalize Claude's string-like field values."""
74+
return value if isinstance(value, str) else ""
75+
76+
77+
def _normalize_token(token: str) -> str:
78+
"""Normalize a claimed removed token for comparison."""
79+
stripped = token.strip()
80+
normalized = stripped
81+
match stripped:
82+
case value if value.endswith("()"):
83+
normalized = value[:-2]
84+
case value:
85+
normalized = value
86+
return normalized
87+
88+
89+
def _iter_non_fenced_lines(text: str) -> list[str]:
90+
"""Return lines outside Markdown code fences."""
91+
lines: list[str] = []
92+
in_fence = False
93+
for line in text.splitlines():
94+
stripped = line.strip()
95+
if stripped.startswith(("```", "~~~")):
96+
in_fence = not in_fence
97+
continue
98+
if in_fence:
99+
continue
100+
lines.append(line)
101+
return lines
102+
103+
104+
def _claimed_removed_tokens(line: str) -> set[str]:
105+
"""Extract explicit removed-token claims from one non-fenced line."""
106+
tokens: set[str] = set()
107+
for pattern in REMOVAL_PATTERNS:
108+
for match in pattern.finditer(line):
109+
if normalized := _normalize_token(match.group("token")):
110+
tokens.add(normalized)
111+
return tokens
112+
113+
114+
def _token_present(token: str, deleted_lines: str) -> bool:
115+
"""Return whether token appears with non-token boundaries in deleted lines."""
116+
if not (normalized := _normalize_token(token)):
117+
return False
118+
pattern = re.compile(rf"(?<![{TOKEN_BOUNDARY_CHARS}]){re.escape(normalized)}(?![{TOKEN_BOUNDARY_CHARS}])")
119+
return bool(pattern.search(deleted_lines))
120+
121+
122+
def _validate_removal_claims(breaking_changes_content: str, deleted_lines_path: Path) -> None:
123+
"""Validate explicit removal claims against trusted deleted diff lines."""
124+
claims = [
125+
(line, token)
126+
for line in _iter_non_fenced_lines(breaking_changes_content)
127+
for token in _claimed_removed_tokens(line)
128+
]
129+
if not claims:
130+
return
131+
132+
deleted_lines = deleted_lines_path.read_text(encoding="utf-8") if deleted_lines_path.exists() else ""
133+
if not deleted_lines.strip():
134+
print(
135+
"Release draft analysis claimed removals, but the exact PR diff has no deleted lines:",
136+
file=sys.stderr,
137+
)
138+
print(breaking_changes_content, file=sys.stderr)
139+
raise SystemExit(1)
140+
141+
invalid_claims = [
142+
f"{line}\n Missing deleted token: {token}"
143+
for line, token in claims
144+
if not _token_present(token, deleted_lines)
145+
]
146+
if not invalid_claims:
147+
return
148+
149+
print("Release draft analysis claimed removals that are not present in the exact PR diff:", file=sys.stderr)
150+
print("\n".join(invalid_claims), file=sys.stderr)
151+
raise SystemExit(1)
152+
153+
154+
def _validate_diff_was_read(reasoning: str) -> None:
155+
"""Fail if Claude says it could not read the prepared diff."""
156+
if not (reasoning and UNREADABLE_DIFF_RE.search(reasoning)):
157+
return
158+
print("Release draft analysis could not read the prepared diff; refusing to update the draft.", file=sys.stderr)
159+
print(reasoning, file=sys.stderr)
160+
raise SystemExit(1)
161+
162+
163+
def main() -> int:
164+
"""Validate Claude output and persist the normalized analysis JSON artifact."""
165+
args = _parse_args()
166+
claude_output = _parse_claude_output(os.environ.get("CLAUDE_OUTPUT", ""))
167+
has_breaking_changes = _as_bool(claude_output.get("has_breaking_changes", False))
168+
breaking_changes_content = _as_string(claude_output.get("breaking_changes_content", ""))
169+
reasoning = _as_string(claude_output.get("reasoning", ""))
170+
171+
_validate_diff_was_read(reasoning)
172+
if has_breaking_changes:
173+
_validate_removal_claims(breaking_changes_content, args.deleted_lines_path)
174+
175+
args.analysis_path.write_text(
176+
json.dumps(
177+
{
178+
"has_breaking_changes": has_breaking_changes,
179+
"breaking_changes_content": breaking_changes_content,
180+
"reasoning": reasoning,
181+
},
182+
ensure_ascii=False,
183+
)
184+
+ "\n",
185+
encoding="utf-8",
186+
)
187+
return 0
188+
189+
190+
if __name__ == "__main__":
191+
raise SystemExit(main())

0 commit comments

Comments
 (0)