fix(ci): resolve component to manifest path in rc-release workflow#2231
Conversation
The Determine RC version step read .release-please-manifest.json with the component name (e.g. "wren-core-py"), but the manifest is keyed by package path (e.g. "core/wren-core-py"), causing KeyError. Map the component to its path via release-please-config.json before lookup.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRC version lookup now resolves a component's package path from ChangesRC Version Resolution Update
Release-Please Job Outputs Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/rc-release.yml (1)
64-66: ⚡ Quick winConsider guarding
manifest[path]for a consistent error experience.The missing-component case (line 62–63) exits with a clear human-readable message via
sys.exit(...). But ifpathis found inrelease-please-config.jsonyet absent from.release-please-manifest.json(e.g., after a manual edit to either file), line 66 raises an unhandledKeyErrorwith a raw Python traceback instead.♻️ Proposed fix: guard the manifest lookup
- print(manifest[path]) + version = manifest.get(path) + if version is None: + sys.exit(f"path {path!r} not found in .release-please-manifest.json (component '{COMPONENT}')") + print(version)Note:
{COMPONENT}here is a Python variable name — move the shell expansion to a separate Python variable or just keep the path in the message, which is already unambiguous:sys.exit(f"path {path!r} not found in .release-please-manifest.json")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/rc-release.yml around lines 64 - 66, The manifest lookup currently does manifest[path] which can raise an uncaught KeyError; update the code around the manifest usage to guard the lookup (use if path not in manifest) and call sys.exit with a clear message like f"path {path!r} not found in .release-please-manifest.json" if missing, rather than letting a traceback surface; ensure you still print manifest[path] when present. Use the existing variables manifest, path and sys.exit to implement this check so the error experience is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/rc-release.yml:
- Around line 64-66: The manifest lookup currently does manifest[path] which can
raise an uncaught KeyError; update the code around the manifest usage to guard
the lookup (use if path not in manifest) and call sys.exit with a clear message
like f"path {path!r} not found in .release-please-manifest.json" if missing,
rather than letting a traceback surface; ensure you still print manifest[path]
when present. Use the existing variables manifest, path and sys.exit to
implement this check so the error experience is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d17a41bc-2008-4894-89ea-e59a14ef9f33
📒 Files selected for processing (1)
.github/workflows/rc-release.yml
release-please-action prefixes its per-package outputs with the manifest path (e.g. core/wren-core-py--release_created), not the component name. The publish-* jobs therefore never triggered because their gating outputs were always empty.
Summary
rc-release.ymlfailed withKeyError: 'wren-core-py'(failing run)..release-please-manifest.jsonis keyed by package path (e.g.core/wren-core-py), but the workflow looked it up by the component name (wren-core-py).release-please-config.jsonbefore reading the manifest, with a clear error if the component isn't configured.Test plan
release-please-config.json+.release-please-manifest.jsonforwren-core-py→0.4.0,wren→0.3.0,wren-core-wasm→0.1.0, and an unknown component (fails with a readable message).component=wren-core-pyand confirm the Determine RC version step succeeds.🤖 Generated with Claude Code
Summary by CodeRabbit