feat(prv): --allow-approve gate posts APPROVE on high-confidence panel#153
Conversation
Ports the /reviewer skill's auto-approve gate into `cube prv`. Default behaviour is unchanged (review event = COMMENT); pass `--allow-approve` to opt in to APPROVE when every condition holds: - All judges APPROVED with zero blocker_issues (panel-side) - No MISSED or PARTIAL acceptance_criteria from PO/QA persona - No `blocker`-severity ux_findings - `would_accept_as_customer` is not "no" - CI is fully green (no FAILURE / TIMED_OUT / CANCELLED on the head SHA) - PR author is not the cube identity (gh-CLI authenticated user) — no self-approve - No unresolved CHANGES_REQUESTED reviews from other humans on the current head Any missed condition → drops to COMMENT and lists the unmet gate items in the review body so the human can see exactly why cube didn't approve. Auto-merge is deliberately NOT in scope; the reviewer skill never merges, and Reza's babysit procedure flags "do not merge automatically" explicitly. Implementation: new `commands/auto_approve.py` with the gate logic, threaded through `peer_review_command` -> `_run_pr_review`. Replaces the hardcoded `post_action = "COMMENT"` with `compute_review_event(...)`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughAdds an opt-in auto-approve gate that evaluates judge decisions and PR state to choose COMMENT/REQUEST_CHANGES/APPROVE, threads GitHub App installation tokens into gh subprocesses for posting reviews, exposes CLI flags and a github-app command, and adds repo-level persona wiring docs with a verification smoke test. ChangesPR Auto-Approve Gate with Custom Personas
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cube/commands/auto_approve.py`:
- Around line 201-205: The code currently lets approvals proceed when CI checks
are pending; change the logic so that if the pending boolean is true the outcome
is forced to COMMENT (not APPROVE) regardless of the --allow-approve opt-in.
Locate the branches that inspect pending (the block with "elif pending:" and the
separate branch at the later lines 247-248) and replace the pass/approve
behavior with an explicit assignment or return that sets the event/result to
COMMENT and surfaces the pending state in the body; ensure both occurrences use
the same rule so pending CI never leads to APPROVE.
- Around line 83-96: The current code uses sets (blockers, cleared) and
subtracts them which fails when a reviewer changes their last review on the same
head; instead, build a per-author latest-review map by iterating
data.get("reviews") and for each review (only consider reviews for the current
head and ignore cube_login) compare timestamps (use review.get("submittedAt") or
review.get("createdAt"), falling back to the review list index) to keep the most
recent state per login, then compute blockers as logins whose latest state ==
"CHANGES_REQUESTED" (and treat "APPROVED"/"DISMISSED" as non-blocking); return
sorted(blockers) so the decision is based on each author’s latest review rather
than set subtraction.
In `@python/cube/commands/peer_review.py`:
- Around line 372-379: compute_review_event is being called without providing
inline_comment_count so the inline-comment gate is effectively disabled; update
the call site that invokes compute_review_event (the block using
parse_all_decisions and variables pr_number, status, allow_approve,
PROJECT_ROOT) to compute and pass the raw inline comment count from the parsed
decisions (e.g., derive inline_count = count of inline comments in decisions
returned by parse_all_decisions or use an existing field on decisions) and
supply inline_comment_count=inline_count in the compute_review_event(...)
arguments so the inline-comment gate is enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96bf34a2-8963-446b-8597-a775d7cbd0de
⛔ Files ignored due to path filters (1)
.claude/scheduled_tasks.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
cube-personas-v2-prompt.mdpython/cube/cli.pypython/cube/commands/auto_approve.pypython/cube/commands/peer_review.py
📜 Review details
🧰 Additional context used
🪛 LanguageTool
cube-personas-v2-prompt.md
[uncategorized] ~9-~9: Possible missing article found.
Context: ...>` for inheritance. The loader resolves parent body + concatenates the delta, so we on...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~86-~86: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...d or update cube.yaml at the repo root so the five judges reference our personas ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~122-~122: Possible missing comma found.
Context: ...s/code-review.zip` references. Same for Ella citations against her github skills. #...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~122-~122: The official name of this software platform is spelled with a capital “H”.
Context: ...es. Same for Ella citations against her github skills. ## Acceptance criteria (Produc...
(GITHUB)
[uncategorized] ~127-~127: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...2**: Each file uses extends: correctly so the parent body is inherited. The resol...
(COMMA_COMPOUND_SENTENCE_2)
🪛 Ruff (0.15.12)
python/cube/commands/auto_approve.py
[error] 31-31: subprocess call: check for execution of untrusted input
(S603)
[error] 32-32: Starting a process with a partial executable path
(S607)
[warning] 43-43: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (1)
cube-personas-v2-prompt.md (1)
109-117: ⚡ Quick winThe verification command is syntactically valid as written; no quoting fix is required.
The nested
python -c "..."inside the outerpython -c "..."string is valid bash. Command substitution$(...)properly handles nested quoting, so the shell parses this without error. The bash syntax check (bash -n) confirms this.Whilst the proposed heredoc approach is more readable, the original command is not broken and does not need correction.
> Likely an incorrect or invalid review comment.
| elif pending: | ||
| # Pending is allowed-with-note per the /reviewer skill, but only | ||
| # when the user explicitly opted in by passing --allow-approve. We | ||
| # still surface it in the body. | ||
| pass |
There was a problem hiding this comment.
Do not auto-approve while CI checks are pending.
Lines 202-205 and 247-248 currently permit APPROVE with pending checks. For a high-confidence gate, pending CI should keep the event at COMMENT.
Proposed fix
- elif pending:
- # Pending is allowed-with-note per the /reviewer skill, but only
- # when the user explicitly opted in by passing --allow-approve. We
- # still surface it in the body.
- pass
+ elif pending:
+ reasons.append(f"CI pending: {', '.join(pending[:3])}")
@@
- pending_note = ""
- if pending:
- pending_note = f"\n\n_Approving with {len(pending)} CI check(s) still pending: {', '.join(pending[:3])}._"
+ pending_note = ""Also applies to: 247-248
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/cube/commands/auto_approve.py` around lines 201 - 205, The code
currently lets approvals proceed when CI checks are pending; change the logic so
that if the pending boolean is true the outcome is forced to COMMENT (not
APPROVE) regardless of the --allow-approve opt-in. Locate the branches that
inspect pending (the block with "elif pending:" and the separate branch at the
later lines 247-248) and replace the pass/approve behavior with an explicit
assignment or return that sets the event/result to COMMENT and surfaces the
pending state in the body; ensure both occurrences use the same rule so pending
CI never leads to APPROVE.
…prove
GitHub forbids approving a PR you opened. When `cube auto` opens the PR
as the human's gh-CLI identity, `cube prv --allow-approve` from the same
identity returns 422 "Can not approve your own pull request". Solution:
configure a GitHub App. When `~/.cube/github-app.json` is present, cube
mints a short-lived installation token (RS256 JWT → token exchange,
cached at `~/.cube/github-app-token.json` until 5 min before expiry)
and passes it as `GH_TOKEN` to every `gh` subprocess that posts a
review. Reviews then appear as `<app-name>[bot]`.
When no App is configured, cube falls back to the user's gh-CLI auth
unchanged — backward-compatible.
New components:
- `python/cube/github/app_auth.py` — JWT mint + token exchange + cache,
stdlib HTTP (urllib) + `cryptography` for RS256 signing.
- `cube github-app --setup` interactive helper; `--status` / `--test`
for verifying the config without exposing the token value.
- `post_review` and its inline-comment fallback path thread the
token-aware env through every write. Read paths keep the user's
identity (they don't need bot permissions).
- `cryptography` added to runtime deps.
Setup steps:
1. Register an App at github.com/settings/apps/new with repo perms
Contents (Read), Pull requests (Write)
2. Generate a private key, install the app on the target org/repos
3. Run `cube github-app --setup` and paste the app id + installation
id + key path
Also fixes pre-existing test broken by the cross-cutting persona header
introduced in #152: `test_inline_judge_persona_is_preserved` now checks
the inline text is contained in the resolved persona body rather than
exact-equal, matching the new contract where every persona inherits the
comment-style + failure-mode discipline header.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![the-agent-cube[bot]](https://avatars.githubusercontent.com/u/156566864?s=60&v=4){kind=small}
| # Inline personas inherit the cross-cutting review-craft header | ||
| # (comment-style + failure-mode discipline) from the persona resolver, | ||
| # then the original text is appended verbatim. | ||
| persona = config.judges["judge_1"].persona |
There was a problem hiding this comment.
[WARNING] The PR test plan claims committed unit smoke coverage for compute_review_event() and App auth edge cases, but the diff does not include tests for either module. Either add the regression tests or update the PR description to say these checks were only run locally.
— Agent Cube / Judge PO/QA, Judge Principal 🤖
| @@ -0,0 +1 @@ | |||
| {"sessionId":"9d24c4e2-9a7e-43f0-a639-5471ae1b287e","pid":29706,"procStart":"Fri May 15 07:40:21 2026","acquiredAt":1779015852104} No newline at end of file | |||
There was a problem hiding this comment.
[WARNING] This is a local runtime lock file containing session and process metadata. It should be removed from the PR and .claude/ or .claude/*.lock should be ignored so it does not get recommitted.
— Agent Cube / Judge PO/QA, Judge Principal, Judge Backend 🤖
| @@ -0,0 +1,141 @@ | |||
| # Wire repo-level cube judge personas into aetheron-connect-v2 | |||
There was a problem hiding this comment.
[WARNING] This is a cross-repo task brief for wiring personas into aetheronhq/aetheron-connect-v2, not part of the --allow-approve or GitHub App work in this PR. Please drop it from this PR or move it to a separate planning change.
— Agent Cube / Judge PO/QA, Judge Principal 🤖
| Caches across cube invocations; mints a new one when within 5 min of expiry. | ||
| """ | ||
| cfg = cfg or GitHubAppConfig.load() | ||
| if cfg is None: |
There was a problem hiding this comment.
[NITPICK] get_installation_token() returns None both when no App config exists and when a present config is broken, such as an unreadable private key. That makes --allow-approve silently fall back to gh-CLI auth and fail later; warn clearly when config is present but token minting fails.
— Agent Cube / Judge PO/QA 🤖
| cube_login = _cube_login(cwd=cwd) | ||
| try: | ||
| author = _pr_author(pr_number, cwd=cwd) | ||
| if cube_login and author == cube_login.lower(): |
There was a problem hiding this comment.
[CRITICAL] The self-approval guard compares the PR author to the gh user even when review posting uses the GitHub App token. Clarify whether this is meant to block the operating human or only the posting identity, and avoid preventing the documented App-bot approval path when the human opened the PR.
— Agent Cube / Judge Principal, Judge Backend 🤖
|
|
||
| # Human CHANGES_REQUESTED guard | ||
| try: | ||
| crs = _human_changes_requested(pr_number, cube_login, cwd=cwd) |
There was a problem hiding this comment.
[WARNING] _human_changes_requested() ignores the gh user but not the GitHub App bot identity. A prior bot-posted CHANGES_REQUESTED review on the current head could be counted as a human blocker and prevent a later bot approval after the issue is fixed.
— Agent Cube / Judge Principal 🤖
|
|
||
| now = int(time.time()) | ||
| header = {"alg": "RS256", "typ": "JWT"} | ||
| payload = {"iat": now - 60, "exp": now + 540, "iss": cfg.app_id} |
There was a problem hiding this comment.
[NITPICK] The docstring says the JWT uses about 9 minutes of validity for clock-skew headroom, but the current iat/exp values produce a 10-minute lifetime. Either reduce exp for real headroom or correct the comment.
— Agent Cube / Judge Principal 🤖
|
|
||
| # If the panel itself isn't unanimously approved, this isn't an auto-approve | ||
| # candidate at all — caller decides REQUEST_CHANGES vs COMMENT. | ||
| if not panel_status.get("approved"): |
There was a problem hiding this comment.
[CRITICAL] The auto-approve path appears to trust panel_status["approved"] alone, but get_peer_review_status() can still include remaining_issues or blocker_issues. Require those issue lists to be empty before treating the panel as clean enough to approve.
— Agent Cube / Judge Backend 🤖
| # candidate at all — caller decides REQUEST_CHANGES vs COMMENT. | ||
| if not panel_status.get("approved"): | ||
| body = f"⚠️ Cube panel: {approvals}/{total} approved.\n\n" f"Per-judge:\n{judge_block}" | ||
| return GateResult(event="REQUEST_CHANGES" if approvals == 0 else "COMMENT", body=body, reasons=[]) |
There was a problem hiding this comment.
[WARNING] This path can post REQUEST_CHANGES when zero judges approve, even without --allow-approve. The PR contract says default behavior remains comment-only unless approval is explicitly opted in, so preserve COMMENT by default or add an explicit opt-in for blocking review events.
— Agent Cube / Judge Backend 🤖
|
|
||
| def _write_cached_token(cfg: GitHubAppConfig, token: str, expires_at: float) -> None: | ||
| TOKEN_CACHE_PATH.parent.mkdir(parents=True, exist_ok=True) | ||
| TOKEN_CACHE_PATH.write_text( |
There was a problem hiding this comment.
[WARNING] The token cache is written before chmod(0o600), leaving a window where default umask permissions may expose a pull_requests:write installation token. Create the file with mode 0o600 via os.open(..., O_CREAT, 0o600); apply the same pattern to the App config writer.
— Agent Cube / Judge Security 🤖
| if not key_path_str: | ||
| return None | ||
| key_path = Path(os.path.expanduser(key_path_str)) | ||
| if not key_path.exists(): |
There was a problem hiding this comment.
[NITPICK] The private key file is used without checking owner-only permissions. Consider warning or refusing when private_key_path.stat().st_mode & 0o077 is nonzero so a world-readable GitHub App key is caught early.
— Agent Cube / Judge Security 🤖
| Returns `(token, epoch_expires_at)`. | ||
| """ | ||
| jwt = _mint_jwt(cfg) | ||
| url = f"https://api.github.com/app/installations/{cfg.installation_id}/access_tokens" |
There was a problem hiding this comment.
[NITPICK] installation_id is interpolated into the token URL from local config. A simple digits-only validation at config load would fail closed with a clear error instead of producing a later token-exchange 404.
— Agent Cube / Judge Security 🤖
| data = json.loads(TOKEN_CACHE_PATH.read_text()) | ||
| except (OSError, json.JSONDecodeError): | ||
| return None | ||
| if data.get("installation_id") != cfg.installation_id: |
There was a problem hiding this comment.
[NITPICK] The cached-token validity check matches only installation_id. Include app_id in the cache record and match so a reconfigured App cannot accidentally reuse a token from another App with the same installation id.
— Agent Cube / Judge Security 🤖
|
— Agent Cube / Judge Principal 🤖 |
The cube panel reviewed PR #153 itself and flagged real gaps. Fixing the substantive ones: 1. inline_comment_count was dead — declared in compute_review_event's signature but never passed by _run_pr_review. Result: the /reviewer skill's "zero inline findings worth posting" condition never blocked auto-approve. Moved the gate evaluation to AFTER dedupe in _run_pr_review and pass len(dedupe_result.inline_comments) so the gate fires correctly. 2. Human CHANGES_REQUESTED detection wasn't order-agnostic. The old set-subtraction logic ignored review ordering: a reviewer who APPROVED then later left CHANGES_REQUESTED on the same head was treated as cleared, when in fact the most recent state is what matters. Rewrote to walk reviews in order and take each author's LAST APPROVED/CHANGES_REQUESTED/DISMISSED state on the head SHA. COMMENTED reviews don't transition state. 3. Console printed `Decision: APPROVED` even when the gate would post COMMENT — display_decision was computed from panel status only, ignoring the gate. Now prints `Posting as: <event>` so the log matches what actually goes to GitHub. 4. Token cache only matched installation_id, not app_id. A reconfigured App could reuse a stale token from a different app sharing the same installation id. Now matches both. (Storing app_id in the cache file too.) 5. installation_id/app_id were not validated at config load. A typo in ~/.cube/github-app.json surfaced as an opaque 404 against the token exchange endpoint. Now fails closed at load when either id contains non-digits. All four addressable findings have unit-test coverage in this commit's smoke test: - inline_comment_count > 0 → gate blocks with reason listed - order-agnostic CR: APPROVED-then-CR blocks; CR-then-APPROVED clears - non-numeric app_id rejected by GitHubAppConfig.load() - (token cache key change is straightforward, tested by integration) The branch-review path (_run_branch_review) keeps display_decision since it doesn't post to GitHub and doesn't run through the auto-approve gate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cube/commands/auto_approve.py`:
- Around line 204-215: Initialize the variables returned by _ci_failures before
the try so they exist if an exception is raised: declare failed and pending
(e.g., failed = [] and pending = []) immediately before calling _ci_failures in
auto_approve.py, then keep the existing try/except that appends to reasons; this
prevents NameError when the except path runs and later code references pending
or failed.
- Around line 42-43: The except block that catches json.JSONDecodeError should
chain the original exception to preserve traceback: update the raise in the
except (the line that raises RuntimeError(f"gh {' '.join(args)} returned
non-JSON: {e}")) to use "raise ... from e" so the JSONDecodeError is attached
(refer to the except json.JSONDecodeError as e block and the use of args/e in
auto_approve.py).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43ecdd24-1f56-47eb-a23c-0600d3d54f4b
📒 Files selected for processing (3)
python/cube/commands/auto_approve.pypython/cube/commands/peer_review.pypython/cube/github/app_auth.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/cube/commands/peer_review.py
📜 Review details
🧰 Additional context used
🪛 Ruff (0.15.12)
python/cube/commands/auto_approve.py
[error] 31-31: subprocess call: check for execution of untrusted input
(S603)
[error] 32-32: Starting a process with a partial executable path
(S607)
[warning] 43-43: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
python/cube/github/app_auth.py
[error] 132-140: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 142-142: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 148-149: try-except-pass detected, consider logging the exception
(S110)
[warning] 148-148: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (16)
python/cube/commands/auto_approve.py (7)
209-213: Pending CI checks still permitAPPROVE.This concern was raised in a previous review and remains unaddressed. The
passstatement allows the gate to proceed toAPPROVEwith pending CI. For a high-confidence gate, pending CI should add a reason and keep the event atCOMMENT.
163-165:REQUEST_CHANGESposted without--allow-approveopt-in.This concern was raised in a previous review and remains unaddressed. When zero judges approve, this returns
REQUEST_CHANGESregardless of the--allow-approveflag. Per the PR contract, the default behaviour should remain comment-only; posting blocking review events should require explicit opt-in.
1-28: LGTM!
46-67: LGTM!
70-104: LGTM!
107-121: LGTM!
145-159: LGTM!Also applies to: 167-202, 234-262
python/cube/github/app_auth.py (9)
200-216: Token cache file briefly world-readable beforechmod.This concern was raised in a previous review and remains unaddressed. The token is written with default umask permissions before
chmod(0o600)is applied, creating a brief window where a sensitivepull_requests:writetoken may be readable by other users. Useos.open(..., os.O_CREAT | os.O_WRONLY, 0o600)withos.fdopento create the file with restricted permissions atomically.
1-44: LGTM!
46-88: LGTM!
90-122: LGTM!
125-163: LGTM!
165-173: LGTM!
176-197: LGTM!
219-232: LGTM!
235-255: LGTM!
| except json.JSONDecodeError as e: | ||
| raise RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}") |
There was a problem hiding this comment.
Chain the exception for proper traceback context.
The raise statement inside the except block should use raise ... from e to preserve the original exception context.
Proposed fix
except json.JSONDecodeError as e:
- raise RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}")
+ raise RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}") from e📝 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.
| except json.JSONDecodeError as e: | |
| raise RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}") | |
| except json.JSONDecodeError as e: | |
| raise RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}") from e |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 43-43: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/cube/commands/auto_approve.py` around lines 42 - 43, The except block
that catches json.JSONDecodeError should chain the original exception to
preserve traceback: update the raise in the except (the line that raises
RuntimeError(f"gh {' '.join(args)} returned non-JSON: {e}")) to use "raise ...
from e" so the JSONDecodeError is attached (refer to the except
json.JSONDecodeError as e block and the use of args/e in auto_approve.py).
| # CI gate | ||
| try: | ||
| failed, pending = _ci_failures(pr_number, cwd=cwd) | ||
| if failed: | ||
| reasons.append(f"CI failing: {', '.join(failed[:3])}") | ||
| elif pending: | ||
| # Pending is allowed-with-note per the /reviewer skill, but only | ||
| # when the user explicitly opted in by passing --allow-approve. We | ||
| # still surface it in the body. | ||
| pass | ||
| except RuntimeError as e: | ||
| reasons.append(f"could not read CI checks ({e})") |
There was a problem hiding this comment.
pending variable undefined when _ci_failures raises, causing NameError.
If _ci_failures() raises a RuntimeError (e.g., network failure, gh not available), the exception is caught but pending is never assigned. The reference to pending at line 255 will then raise NameError: name 'pending' is not defined.
Proposed fix — initialise before try block
# CI gate
+ failed: list[str] = []
+ pending: list[str] = []
try:
failed, pending = _ci_failures(pr_number, cwd=cwd)
if failed:
reasons.append(f"CI failing: {', '.join(failed[:3])}")
elif pending:
# Pending is allowed-with-note per the /reviewer skill, but only
# when the user explicitly opted in by passing --allow-approve. We
# still surface it in the body.
pass
except RuntimeError as e:
reasons.append(f"could not read CI checks ({e})")Also applies to: 253-256
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/cube/commands/auto_approve.py` around lines 204 - 215, Initialize the
variables returned by _ci_failures before the try so they exist if an exception
is raised: declare failed and pending (e.g., failed = [] and pending = [])
immediately before calling _ci_failures in auto_approve.py, then keep the
existing try/except that appends to reasons; this prevents NameError when the
except path runs and later code references pending or failed.
When a separate 'cube' Claude Code account exists, cube's writer / judge /
prompter subprocesses should run under that identity instead of the
user's terminal session. Adds a CLAUDE_CONFIG_DIR override that's
injected only into spawn env, not into the user's shell.
Resolution order (in ClaudeAdapter.run):
1. $CUBE_CLAUDE_CONFIG_DIR env var (explicit override)
2. ~/.cube/claude-config/ if it exists (default convention)
3. No override → claude uses keychain OAuth as today
Setup: user runs `claude /login` once with CLAUDE_CONFIG_DIR pointed at
the cube identity's config dir, e.g.
CLAUDE_CONFIG_DIR=~/.cube/claude-config claude /login
After that, every cube-spawned claude subprocess uses cube's
credentials; the interactive `claude` in the user's terminal is
untouched.
check_logged_in() also respects the same resolver so the readiness
check looks at the same dir cube subprocesses will actually use.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cube/core/adapters/claude.py`:
- Around line 23-25: In check_logged_in(), replace uses of Path.exists() when
validating directory inputs with Path.is_dir() so regular files won't pass the
check and cause NotADirectoryError when later calling iterdir(); specifically
update the explicit-path branch that currently does "path =
Path(os.path.expanduser(explicit))" and its return check, and the other similar
check around the CUBE_CLAUDE_CONFIG_DIR handling (the occurrences referenced at
lines 23-25 and 52-53) to use path.is_dir() instead of path.exists().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0a5faa4-d571-47b4-843b-78e365ad52d7
📒 Files selected for processing (1)
python/cube/core/adapters/claude.py
📜 Review details
🔇 Additional comments (1)
python/cube/core/adapters/claude.py (1)
10-22: LGTM!Also applies to: 26-29, 47-51, 64-74
| if explicit: | ||
| path = Path(os.path.expanduser(explicit)) | ||
| return path if path.exists() else None |
There was a problem hiding this comment.
Guard against file paths in CUBE_CLAUDE_CONFIG_DIR to prevent NotADirectoryError.
exists() accepts regular files. If the env var points to a file, check_logged_in() can crash on iterdir(). Use is_dir() for directory-only checks.
Suggested patch
def _resolve_cube_claude_config_dir() -> Optional[Path]:
@@
explicit = os.environ.get("CUBE_CLAUDE_CONFIG_DIR")
if explicit:
path = Path(os.path.expanduser(explicit))
- return path if path.exists() else None
- if _DEFAULT_CUBE_CLAUDE_CONFIG_DIR.exists():
+ return path if path.is_dir() else None
+ if _DEFAULT_CUBE_CLAUDE_CONFIG_DIR.is_dir():
return _DEFAULT_CUBE_CLAUDE_CONFIG_DIR
return None
@@
def check_logged_in(self) -> bool:
@@
config_dir = _resolve_cube_claude_config_dir() or (Path.home() / ".claude")
- return config_dir.exists() and any(config_dir.iterdir())
+ return config_dir.is_dir() and any(config_dir.iterdir())Also applies to: 52-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/cube/core/adapters/claude.py` around lines 23 - 25, In
check_logged_in(), replace uses of Path.exists() when validating directory
inputs with Path.is_dir() so regular files won't pass the check and cause
NotADirectoryError when later calling iterdir(); specifically update the
explicit-path branch that currently does "path =
Path(os.path.expanduser(explicit))" and its return check, and the other similar
check around the CUBE_CLAUDE_CONFIG_DIR handling (the occurrences referenced at
lines 23-25 and 52-53) to use path.is_dir() instead of path.exists().
1 participant
Summary
Closes the approve half of the autonomous-PR loop with two changes:
--allow-approvegate ported from Jacob's/reviewerskill intocube prv. When the panel is unanimously clean AND every human-side condition holds, postsevent: APPROVE. Otherwise drops toCOMMENTand lists the unmet gate conditions inline so the human sees exactly what blocked.GitHub App identity for review posting. GitHub forbids approving a PR you opened, so when
cube autoopens the PR as the human's gh-CLI identity,cube prv --allow-approvefrom the same identity returns422 "Can not approve your own pull request". Solution: configure a GitHub App, cube mints a short-lived installation token, everyghsubprocess that posts a review uses it viaGH_TOKENenv. Reviews appear as<app-name>[bot]. No App configured → falls back to default gh-CLI auth (backward-compatible).The gate (
--allow-approve)COMMENTunless all of these are true:APPROVEDwith zeroblocker_issuesMISSEDorPARTIALacceptance criteria fromproduct-owner-qablocker-severityux_findingswould_accept_as_customeris not"no"FAILURE/TIMED_OUT/CANCELLEDon the head SHA)CHANGES_REQUESTEDreviews from other humans on the current headAuto-merge is deliberately not in scope — the reviewer skill never merges, Reza's babysit procedure flags it explicitly.
GitHub App setup
Register at https://github.com/settings/apps/new with repo perms
Contents (Read)+Pull requests (Write), generate a private key, install on target org/repos, then:The interactive prompt asks for App ID + Installation ID + path to the
.pemkey file, writes~/.cube/github-app.json(chmod 600), and verifies by minting a token immediately. Token cached at~/.cube/github-app-token.jsonuntil 5 min before expiry.cube github-app --statusand--testverify the config without exposing the token value.Implementation
python/cube/commands/auto_approve.py—compute_review_event()pure-logic gate, mockable.python/cube/github/app_auth.py— JWT mint + installation-token exchange + on-disk cache. Stdlib HTTP (urllib) +cryptographyfor RS256 signing.python/cube/github/reviews.py—post_reviewand its inline-comment fallback thread the token-aware env through every write. Read paths keep the user's identity.python/cube/cli.py—--allow-approveflag oncube prv+ newcube github-appcommand.allow_approvethroughpeer_review_command → _run_pr_review. Replaces the hardcodedpost_action = "COMMENT"withcompute_review_event(...).Pre-existing test
test_inline_judge_persona_is_preserved(broken by the cross-cutting persona header in #152) updated to match the new contract.Test plan
compute_review_event(): partial panel, no--allow-approve, clean gate APPROVE, MISSED AC, self-approve guard, CI failing — all pass.pytest tests/core/ -q— 87 passed.agent-cube, runcube prv <a clean PR> --allow-approveand verify the APPROVE lands asagent-cube[bot].🤖 Generated with Claude Code
Overview
Adds an opt-in auto-approve gate to
cube prvvia --allow-approve and optional GitHub App authentication for posting reviews. When the judge panel is unanimously clean and all human/CI gates pass,cube prv --allow-approveposts an APPROVE; otherwise it posts COMMENT (or REQUEST_CHANGES where appropriate) and enumerates unmet gate conditions. Auto-merge is out of scope.Key changes
--allow-approveto pr-review and a newcube github-appcommand (--setup/--status/--test) to configure/verify a GitHub App.compute_review_event()returnsGateResult(event, body, reasons)and evaluates the high-confidence gate conditions (including inline comment counting after dedupe and CHANGES_REQUESTED detection tied to the PR head SHA).allow_approvethrough the PR flow, uses gate result to choose posted event, logs up to three gate reasons or success, and appends the gate body to the human summary. Console output reflects the actual posted event.env_with_app_token()andis_configured(). Falls back to gh CLI auth if not configured.env_with_app_token()toghsubprocesses so reviews post under the App installation identity when configured;_post_review_with_commentsaccepts an optional env.cryptography>=41.0.0.Gate conditions (all must pass to auto-APPROVE)
would_accept_as_customer≠ "no".If any condition fails (or
--allow-approvenot supplied), the command posts COMMENT (or REQUEST_CHANGES where appropriate) and lists unmet gate reasons.Tests & verification
Review/Risk notes