Post-release changes#543
Conversation
📝 WalkthroughWalkthroughThis pull request adds three new Python build scripts under Buildscripts/: gh-download-artifacts.py for downloading GitHub Actions workflow artifacts, gh-release-summary.py for generating commit and pull request markdown between the last two git tags, and release-summary.py for producing a Claude-generated release summary from those markdown files. Documentation/releasing.md is updated to describe the revised release process incorporating these scripts, and version.txt is bumped from 0.7.0 to 0.8.0-dev. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GhDownload as gh-download-artifacts.py
participant GhSummary as gh-release-summary.py
participant ReleaseSummary as release-summary.py
participant Claude as claude CLI
User->>GhDownload: run with run_id, token, repo
GhDownload->>GhDownload: download & extract artifacts
User->>ReleaseSummary: run with repo, out
ReleaseSummary->>GhSummary: invoke as subprocess
GhSummary-->>ReleaseSummary: release-commits.md, release-pull-requests.md
ReleaseSummary->>Claude: send prompt + commits/PRs
Claude-->>ReleaseSummary: summary text
ReleaseSummary-->>User: release-summary.md
Related issues: None referenced in the provided information. Related PRs: None referenced in the provided information. Suggested labels: tooling, documentation, build-scripts Suggested reviewers: None determinable from the provided information. Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
Buildscripts/gh-download-artifacts.py (1)
77-95: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueArtifact
nameis used unsanitized to build a filesystem path.
out_dir / f"{name}.zip"(Line 91) derives directly from the API response'sartifact["name"]. If a workflow artifact were ever named with path separators (e.g. via a compromised job), this could write outsideout_dir. Given artifacts come from the target repo's own Actions run, sanitizing the name defensively is cheap insurance.Source: Linters/SAST tools
Buildscripts/gh-release-summary.py (1)
76-89: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePR number extraction can mistake issue references for PR numbers.
PR_REF_REmatches any#<digits>in a commit subject, so a commit message referencing an issue (e.g.Fixes#42``) would be listed under "Pull Requests" even though it isn't one. Given this only feeds documentation/summary generation (not a blocking release gate), it's a minor cosmetic gap.Buildscripts/release-summary.py (2)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTypo: "don't like pull request" → "don't link the pull request".
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
run_gh_release_summarypropagates raw errors on missinggh-release-summary.pyexit.Consistent with the comment on
run_claudeabove:subprocess.run(cmd, check=True)will raise an unhandledCalledProcessErrortraceback rather than a clean message if the child script fails. Minor, since the child script's ownsys.exitmessage would still surface in stderr, but the outer traceback adds noise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe2cb9d4-11fe-4f0a-a133-630a2c150c66
📒 Files selected for processing (5)
Buildscripts/gh-download-artifacts.pyBuildscripts/gh-release-summary.pyBuildscripts/release-summary.pyDocumentation/releasing.mdversion.txt
| url = f"{API_URL}/repos/{repo}/actions/runs/{run_id}/artifacts" | ||
| artifacts = [] | ||
| while url: | ||
| resp = requests.get(url, headers=headers, params={"per_page": 100}) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add timeouts to requests.get calls.
Neither the artifact-list request nor the artifact-download request specifies a timeout, so a hung GitHub API/CDN response will block the script indefinitely.
🌐 Proposed fix
- resp = requests.get(url, headers=headers, params={"per_page": 100})
+ resp = requests.get(url, headers=headers, params={"per_page": 100}, timeout=30)- resp = requests.get(download_url, headers=headers, stream=True)
+ resp = requests.get(download_url, headers=headers, stream=True, timeout=30)Also applies to: 88-88
🧰 Tools
🪛 Ruff (0.15.20)
[error] 67-67: Probable use of requests call without timeout
(S113)
Source: Linters/SAST tools
| if name.endswith(".bin"): | ||
| with zipfile.ZipFile(zip_path) as zf: | ||
| zf.extractall(out_dir) | ||
| zip_path.unlink() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate zip member paths before extracting (Zip Slip).
zf.extractall(out_dir) trusts every entry name in the downloaded archive. A malicious or compromised artifact containing a path like ../../evil could write outside out_dir. Since the artifact content originates from the GitHub Actions API for the target run, this is a real (if lower-likelihood) supply-chain risk worth guarding against.
🔒 Proposed fix
if name.endswith(".bin"):
with zipfile.ZipFile(zip_path) as zf:
- zf.extractall(out_dir)
+ for member in zf.namelist():
+ member_path = (out_dir / member).resolve()
+ if not member_path.is_relative_to(out_dir.resolve()):
+ sys.exit(f"Unsafe path in archive: {member}")
+ zf.extractall(out_dir)
zip_path.unlink()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if name.endswith(".bin"): | |
| with zipfile.ZipFile(zip_path) as zf: | |
| zf.extractall(out_dir) | |
| zip_path.unlink() | |
| if name.endswith(".bin"): | |
| with zipfile.ZipFile(zip_path) as zf: | |
| for member in zf.namelist(): | |
| member_path = (out_dir / member).resolve() | |
| if not member_path.is_relative_to(out_dir.resolve()): | |
| sys.exit(f"Unsafe path in archive: {member}") | |
| zf.extractall(out_dir) | |
| zip_path.unlink() |
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 97-97: Calling extractall() on a zipfile.ZipFile or tarfile archive without validating member paths lets a crafted entry (e.g. "../../etc/passwd") write outside the destination directory (Zip Slip). Validate each member resolves inside the target directory, or pass a safe filter (tarfile: filter="data" / tarfile.data_filter).
Context: zf.extractall(out_dir)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(archive-extractall-path-traversal-python)
Source: Linters/SAST tools
| def build_prompt() -> str: | ||
| section_list = "\n".join(f"- {s}" for s in SECTIONS) | ||
| return ( | ||
| "You are given a list of commits (with GitHub commit links) and a list of pull " | ||
| "request links from a software release. Write a release summary in Markdown, " | ||
| "organizing changes into these sections (omit a section if it has no relevant " | ||
| "changes):\n\n" | ||
| f"{section_list}\n\n" | ||
| "Use concise bullet points. Base every bullet strictly on the given commits and " | ||
| "pull requests, referencing the pull request link where applicable. Output only " | ||
| "the Markdown, no commentary before or after it." | ||
| "If a pull request or commit has multiple changes add them as separate entries." | ||
| "Ignore irrelevant changes: documentation, minor fixes, non-descript fixes, scripts, github workflows." | ||
| "For 'New Devices': only mention the device name, don't like pull request." | ||
| "For 'Improvements & Fixes': Avoid non-descript entries, write specifics." | ||
| "For 'TactilitySDK': Avoid generic changes, be specific. Mention API changes." | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing spaces/separators between concatenated prompt strings.
The adjacent string literals in build_prompt are implicitly concatenated by Python without a separating space, so the emitted prompt runs sentences together, e.g. "...after it.If a pull request...". This degrades prompt readability/quality sent to Claude.
✏️ Proposed fix
"Use concise bullet points. Base every bullet strictly on the given commits and "
"pull requests, referencing the pull request link where applicable. Output only "
- "the Markdown, no commentary before or after it."
- "If a pull request or commit has multiple changes add them as separate entries."
- "Ignore irrelevant changes: documentation, minor fixes, non-descript fixes, scripts, github workflows."
- "For 'New Devices': only mention the device name, don't like pull request."
- "For 'Improvements & Fixes': Avoid non-descript entries, write specifics."
- "For 'TactilitySDK': Avoid generic changes, be specific. Mention API changes."
+ "the Markdown, no commentary before or after it. "
+ "If a pull request or commit has multiple changes add them as separate entries. "
+ "Ignore irrelevant changes: documentation, minor fixes, non-descript fixes, scripts, github workflows. "
+ "For 'New Devices': only mention the device name, don't link the pull request. "
+ "For 'Improvements & Fixes': Avoid non-descript entries, write specifics. "
+ "For 'TactilitySDK': Avoid generic changes, be specific. Mention API changes."
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def build_prompt() -> str: | |
| section_list = "\n".join(f"- {s}" for s in SECTIONS) | |
| return ( | |
| "You are given a list of commits (with GitHub commit links) and a list of pull " | |
| "request links from a software release. Write a release summary in Markdown, " | |
| "organizing changes into these sections (omit a section if it has no relevant " | |
| "changes):\n\n" | |
| f"{section_list}\n\n" | |
| "Use concise bullet points. Base every bullet strictly on the given commits and " | |
| "pull requests, referencing the pull request link where applicable. Output only " | |
| "the Markdown, no commentary before or after it." | |
| "If a pull request or commit has multiple changes add them as separate entries." | |
| "Ignore irrelevant changes: documentation, minor fixes, non-descript fixes, scripts, github workflows." | |
| "For 'New Devices': only mention the device name, don't like pull request." | |
| "For 'Improvements & Fixes': Avoid non-descript entries, write specifics." | |
| "For 'TactilitySDK': Avoid generic changes, be specific. Mention API changes." | |
| ) | |
| def build_prompt() -> str: | |
| section_list = "\n".join(f"- {s}" for s in SECTIONS) | |
| return ( | |
| "You are given a list of commits (with GitHub commit links) and a list of pull " | |
| "request links from a software release. Write a release summary in Markdown, " | |
| "organizing changes into these sections (omit a section if it has no relevant " | |
| "changes):\n\n" | |
| f"{section_list}\n\n" | |
| "Use concise bullet points. Base every bullet strictly on the given commits and " | |
| "pull requests, referencing the pull request link where applicable. Output only " | |
| "the Markdown, no commentary before or after it. " | |
| "If a pull request or commit has multiple changes add them as separate entries. " | |
| "Ignore irrelevant changes: documentation, minor fixes, non-descript fixes, scripts, github workflows. " | |
| "For 'New Devices': only mention the device name, don't link the pull request. " | |
| "For 'Improvements & Fixes': Avoid non-descript entries, write specifics. " | |
| "For 'TactilitySDK': Avoid generic changes, be specific. Mention API changes." | |
| ) |
| def run_claude(commits_md: str, prs_md: str) -> str: | ||
| prompt = build_prompt() | ||
| stdin_content = f"## Commits\n\n{commits_md}\n\n## Pull Requests\n\n{prs_md}" | ||
| result = subprocess.run( | ||
| ["claude", "-p", prompt], | ||
| input=stdin_content, | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| ) | ||
| return result.stdout |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No timeout and no friendly error handling around the claude subprocess.
Unlike get_token/get_repo in the sibling scripts, which catch FileNotFoundError/CalledProcessError and print an actionable message, run_claude lets an unhandled CalledProcessError/FileNotFoundError (e.g. claude CLI missing) propagate as a raw traceback. It also has no timeout, so a hung/rate-limited claude call can block the release pipeline indefinitely.
🛠️ Proposed fix
def run_claude(commits_md: str, prs_md: str) -> str:
prompt = build_prompt()
stdin_content = f"## Commits\n\n{commits_md}\n\n## Pull Requests\n\n{prs_md}"
- result = subprocess.run(
- ["claude", "-p", prompt],
- input=stdin_content,
- capture_output=True,
- text=True,
- check=True,
- )
- return result.stdout
+ try:
+ result = subprocess.run(
+ ["claude", "-p", prompt],
+ input=stdin_content,
+ capture_output=True,
+ text=True,
+ check=True,
+ timeout=300,
+ )
+ except FileNotFoundError:
+ sys.exit("`claude` CLI not found. Install it and ensure it's on PATH.")
+ except subprocess.TimeoutExpired:
+ sys.exit("`claude` call timed out.")
+ except subprocess.CalledProcessError as e:
+ sys.exit(f"`claude` failed: {e.stderr}")
+ return result.stdout📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def run_claude(commits_md: str, prs_md: str) -> str: | |
| prompt = build_prompt() | |
| stdin_content = f"## Commits\n\n{commits_md}\n\n## Pull Requests\n\n{prs_md}" | |
| result = subprocess.run( | |
| ["claude", "-p", prompt], | |
| input=stdin_content, | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| ) | |
| return result.stdout | |
| def run_claude(commits_md: str, prs_md: str) -> str: | |
| prompt = build_prompt() | |
| stdin_content = f"## Commits\n\n{commits_md}\n\n## Pull Requests\n\n{prs_md}" | |
| try: | |
| result = subprocess.run( | |
| ["claude", "-p", prompt], | |
| input=stdin_content, | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| timeout=300, | |
| ) | |
| except FileNotFoundError: | |
| sys.exit("`claude` CLI not found. Install it and ensure it's on PATH.") | |
| except subprocess.TimeoutExpired: | |
| sys.exit("`claude` call timed out.") | |
| except subprocess.CalledProcessError as e: | |
| sys.exit(f"`claude` failed: {e.stderr}") | |
| return result.stdout |
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 50-56: Command coming from incoming request
Context: subprocess.run(
["claude", "-p", prompt],
input=stdin_content,
capture_output=True,
text=True,
check=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.20)
[error] 51-51: subprocess call: check for execution of untrusted input
(S603)
[error] 52-52: Starting a process with a partial executable path
(S607)
| 4. Wait for the SDK and firmwares to be published on the CDN. | ||
| 5. Test the firmwares on all devices with the web installer | ||
| 6. Run `Buildscripts/gh-download-artifacts.py [actionId] --token [token]` and let it run in the background. | ||
| Get a token from [here](https://github.com/settings/personal-access-tokens) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the generic link text.
[here] is opaque and triggers the markdownlint MD059 warning. Use descriptive text so the step is self-explanatory.
💡 Suggested fix
- Get a token from [here](https://github.com/settings/personal-access-tokens)
+ Get a token from [GitHub personal access tokens](https://github.com/settings/personal-access-tokens)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Get a token from [here](https://github.com/settings/personal-access-tokens) | |
| Get a token from [GitHub personal access tokens](https://github.com/settings/personal-access-tokens) |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 11-11: Link text should be descriptive
(MD059, descriptive-link-text)
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores
0.8.0-dev.