chore(workspace): propagate unified make and release automation#5
chore(workspace): propagate unified make and release automation#5marlon-costa-dc merged 9 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/github/pr_manager.py">
<violation number="1" location="scripts/github/pr_manager.py:156">
P2: The `--merge-method` argument has no `choices` validation, and `_merge_pr` silently falls back to `--squash` for any unrecognized value. A typo (e.g., `--merge-method=sqaush`) would silently squash-merge instead of raising an error. Add `choices=["merge", "rebase", "squash"]` to the argument definition, consistent with how `--action` is validated.</violation>
</file>
<file name="scripts/release/notes.py">
<violation number="1" location="scripts/release/notes.py:74">
P2: Likely off-by-one: `len(projects) + 1` now counts "root" as a packaged project, whereas the old code (`len(projects) + 2`) explicitly excluded "root" from the packaged count (the +2 accounted for the two hardcoded project names, not "root"). With those hardcoded names removed, the count should be `len(projects)` to preserve the same convention.</violation>
</file>
<file name="scripts/maintenance/enforce_python_version.py">
<violation number="1" location="scripts/maintenance/enforce_python_version.py:102">
P2: `_has_guard` only checks for the marker string presence, not whether the guard enforces the current `REQUIRED_MINOR`. In `--check` mode, if `REQUIRED_MINOR` changes, stale guards enforcing the old Python version would incorrectly pass validation. Consider also verifying the version number inside the guard block.</violation>
</file>
<file name="scripts/core/stub_supply_chain.py">
<violation number="1" location="scripts/core/stub_supply_chain.py:22">
P1: Missing module: `libs.selection` does not exist in the repository. This import will fail with `ModuleNotFoundError` at runtime, breaking the entire script. If this module is expected to come from another PR or branch, consider adding it in this PR or reverting to the previous self-contained `discover_projects` implementation until the dependency is available.</violation>
</file>
<file name="scripts/core/skill_validate.py">
<violation number="1" location="scripts/core/skill_validate.py:18">
P0: Import will fail at runtime: `libs.discovery` module does not exist in the repository. This is an unconditional top-level import with no fallback, so the entire `skill_validate.py` script will be broken with an `ImportError`. The `libs/discovery.py` (or `libs/discovery/__init__.py`) file appears to be missing from this PR.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if str(Path(__file__).resolve().parents[2]) not in sys.path: | ||
| sys.path.insert(0, str(Path(__file__).resolve().parents[2])) | ||
|
|
||
| from libs.discovery import discover_projects as ssot_discover_projects |
There was a problem hiding this comment.
P0: Import will fail at runtime: libs.discovery module does not exist in the repository. This is an unconditional top-level import with no fallback, so the entire skill_validate.py script will be broken with an ImportError. The libs/discovery.py (or libs/discovery/__init__.py) file appears to be missing from this PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/core/skill_validate.py, line 18:
<comment>Import will fail at runtime: `libs.discovery` module does not exist in the repository. This is an unconditional top-level import with no fallback, so the entire `skill_validate.py` script will be broken with an `ImportError`. The `libs/discovery.py` (or `libs/discovery/__init__.py`) file appears to be missing from this PR.</comment>
<file context>
@@ -12,6 +12,11 @@
+if str(Path(__file__).resolve().parents[2]) not in sys.path:
+ sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
+
+from libs.discovery import discover_projects as ssot_discover_projects
+
try:
</file context>
| if str(Path(__file__).resolve().parents[2]) not in sys.path: | ||
| sys.path.insert(0, str(Path(__file__).resolve().parents[2])) | ||
|
|
||
| from libs.selection import resolve_projects |
There was a problem hiding this comment.
P1: Missing module: libs.selection does not exist in the repository. This import will fail with ModuleNotFoundError at runtime, breaking the entire script. If this module is expected to come from another PR or branch, consider adding it in this PR or reverting to the previous self-contained discover_projects implementation until the dependency is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/core/stub_supply_chain.py, line 22:
<comment>Missing module: `libs.selection` does not exist in the repository. This import will fail with `ModuleNotFoundError` at runtime, breaking the entire script. If this module is expected to come from another PR or branch, consider adding it in this PR or reverting to the previous self-contained `discover_projects` implementation until the dependency is available.</comment>
<file context>
@@ -16,6 +16,11 @@
+if str(Path(__file__).resolve().parents[2]) not in sys.path:
+ sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
+
+from libs.selection import resolve_projects
+
MISSING_IMPORT_RE = re.compile(r"Cannot find module `([^`]+)` \[missing-import\]")
</file context>
| _ = parser.add_argument("--title", default="") | ||
| _ = parser.add_argument("--body", default="") | ||
| _ = parser.add_argument("--draft", type=int, default=0) | ||
| _ = parser.add_argument("--merge-method", default="squash") |
There was a problem hiding this comment.
P2: The --merge-method argument has no choices validation, and _merge_pr silently falls back to --squash for any unrecognized value. A typo (e.g., --merge-method=sqaush) would silently squash-merge instead of raising an error. Add choices=["merge", "rebase", "squash"] to the argument definition, consistent with how --action is validated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/github/pr_manager.py, line 156:
<comment>The `--merge-method` argument has no `choices` validation, and `_merge_pr` silently falls back to `--squash` for any unrecognized value. A typo (e.g., `--merge-method=sqaush`) would silently squash-merge instead of raising an error. Add `choices=["merge", "rebase", "squash"]` to the argument definition, consistent with how `--action` is validated.</comment>
<file context>
@@ -0,0 +1,204 @@
+ _ = parser.add_argument("--title", default="")
+ _ = parser.add_argument("--body", default="")
+ _ = parser.add_argument("--draft", type=int, default=0)
+ _ = parser.add_argument("--merge-method", default="squash")
+ _ = parser.add_argument("--auto", type=int, default=0)
+ _ = parser.add_argument("--delete-branch", type=int, default=0)
</file context>
| "", | ||
| f"- Workspace release version: {version}", | ||
| f"- Projects packaged: {len(projects) + 2}", | ||
| f"- Projects packaged: {len(projects) + 1}", |
There was a problem hiding this comment.
P2: Likely off-by-one: len(projects) + 1 now counts "root" as a packaged project, whereas the old code (len(projects) + 2) explicitly excluded "root" from the packaged count (the +2 accounted for the two hardcoded project names, not "root"). With those hardcoded names removed, the count should be len(projects) to preserve the same convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/release/notes.py, line 74:
<comment>Likely off-by-one: `len(projects) + 1` now counts "root" as a packaged project, whereas the old code (`len(projects) + 2`) explicitly excluded "root" from the packaged count (the +2 accounted for the two hardcoded project names, not "root"). With those hardcoded names removed, the count should be `len(projects)` to preserve the same convention.</comment>
<file context>
@@ -70,7 +71,7 @@ def main() -> int:
"",
f"- Workspace release version: {version}",
- f"- Projects packaged: {len(projects) + 2}",
+ f"- Projects packaged: {len(projects) + 1}",
"",
"## Projects impacted",
</file context>
|
|
||
| def _has_guard(content: str) -> bool: | ||
| """Check if conftest.py already has the version guard.""" | ||
| return GUARD_MARKER in content |
There was a problem hiding this comment.
P2: _has_guard only checks for the marker string presence, not whether the guard enforces the current REQUIRED_MINOR. In --check mode, if REQUIRED_MINOR changes, stale guards enforcing the old Python version would incorrectly pass validation. Consider also verifying the version number inside the guard block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/maintenance/enforce_python_version.py, line 102:
<comment>`_has_guard` only checks for the marker string presence, not whether the guard enforces the current `REQUIRED_MINOR`. In `--check` mode, if `REQUIRED_MINOR` changes, stale guards enforcing the old Python version would incorrectly pass validation. Consider also verifying the version number inside the guard block.</comment>
<file context>
@@ -0,0 +1,249 @@
+
+def _has_guard(content: str) -> bool:
+ """Check if conftest.py already has the version guard."""
+ return GUARD_MARKER in content
+
+
</file context>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/github/pr_manager.py">
<violation number="1" location="scripts/github/pr_manager.py:104">
P2: `gh release view` existence check leaks verbose output to stdout. `_run_stream` does not capture output, so the full release details are printed to the terminal. Since this is only an existence check, the output should be suppressed to keep the structured `key=value` output consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if tag is None: | ||
| return | ||
|
|
||
| if _run_stream(["gh", "release", "view", tag], repo_root) == 0: |
There was a problem hiding this comment.
P2: gh release view existence check leaks verbose output to stdout. _run_stream does not capture output, so the full release details are printed to the terminal. Since this is only an existence check, the output should be suppressed to keep the structured key=value output consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/github/pr_manager.py, line 104:
<comment>`gh release view` existence check leaks verbose output to stdout. `_run_stream` does not capture output, so the full release details are printed to the terminal. Since this is only an existence check, the output should be suppressed to keep the structured `key=value` output consistent.</comment>
<file context>
@@ -79,6 +80,41 @@ def _selector(pr_number: str, head: str) -> str:
+ if tag is None:
+ return
+
+ if _run_stream(["gh", "release", "view", tag], repo_root) == 0:
+ print(f"status=release-exists tag={tag}")
+ return
</file context>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/release/run.py">
<violation number="1">
P2: Moving tag creation outside the `if push:` block can cause a stale tag to be pushed. If the publish phase is first run with `push=False` (tag created locally at commit A), then additional commits are made, and the phase is re-run with `push=True`, the existing tag still points to commit A. The `tag_exists` check prevents re-creation, so `git push origin tag` pushes the stale tag.
Consider re-creating the tag if it doesn't point to the current HEAD, e.g. by checking `git rev-parse` for the tag vs HEAD, or by keeping the tag creation adjacent to the push so it always reflects the pushed commit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
marlon-costa-dc
force-pushed
the
0.11.0-dev
branch
from
429b207 to
1b34d82
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/github/pr_workspace.py">
<violation number="1" location="scripts/github/pr_workspace.py:120">
P1: The script path `scripts/github/pr_manager.py` is relative but the `subprocess.run` call has no `cwd` set. If the working directory differs from `workspace_root` (e.g., `--workspace-root` points elsewhere), the script won't be found. Use an absolute path derived from `repo_root` (which equals `workspace_root` in this branch).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if repo_root == workspace_root: | ||
| command = [ | ||
| "python", | ||
| "scripts/github/pr_manager.py", |
There was a problem hiding this comment.
P1: The script path scripts/github/pr_manager.py is relative but the subprocess.run call has no cwd set. If the working directory differs from workspace_root (e.g., --workspace-root points elsewhere), the script won't be found. Use an absolute path derived from repo_root (which equals workspace_root in this branch).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/github/pr_workspace.py, line 120:
<comment>The script path `scripts/github/pr_manager.py` is relative but the `subprocess.run` call has no `cwd` set. If the working directory differs from `workspace_root` (e.g., `--workspace-root` points elsewhere), the script won't be found. Use an absolute path derived from `repo_root` (which equals `workspace_root` in this branch).</comment>
<file context>
@@ -114,28 +114,60 @@ def _run_pr(repo_root: Path, workspace_root: Path, args: argparse.Namespace) ->
+ if repo_root == workspace_root:
+ command = [
+ "python",
+ "scripts/github/pr_manager.py",
+ "--repo-root",
+ str(repo_root),
</file context>
| "scripts/github/pr_manager.py", | |
| str(repo_root / "scripts" / "github" / "pr_manager.py"), |
1 participant
Summary
0.11.0-devValidation
Summary by cubic
Unifies workspace make/release automation with consistent project discovery and a workspace PR runner. Adds auto-rebase merges, optional release dispatch on merge, and enforces Python 3.13 to avoid mismatched venvs.
New Features
Refactors
Written for commit a48c59c. Summary will update on new commits.