Repository navigation
chore: add opt-in /passci skill for CI-verified pull requests - #9546
danieljbruce wants to merge 5 commits into
Conversation
Adds the public opt-in `/passci` skill under `.agents/skills/passci/` to guide Jetski through opening a draft PR immediately after solving a task and pushing `[Style Maintence]`, `[Independent review follow-ups]`, and `[Address CI errors]` commits with 95% unit test confidence.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the /passci opt-in workflow skill for googleapis/google-cloud-node, adding a detailed markdown guide and a helper Python script to automate style audits, review checks, and unit test verification. The code review identified several critical robustness issues in the helper script, including potential crashes from unhandled exceptions during file reading and JSON parsing of API responses or configuration files. Additionally, a typo in the style maintenance commit prefix was flagged for correction across the codebase and documentation.
| issue_comments: Sequence[Mapping[str, Any]] = ( | ||
| json.loads(issue_proc.stdout) if issue_proc.returncode == 0 else [] | ||
| ) | ||
| pull_comments: Sequence[Mapping[str, Any]] = ( | ||
| json.loads(pull_proc.stdout) if pull_proc.returncode == 0 else [] | ||
| ) |
There was a problem hiding this comment.
If the GitHub API returns an error or unexpected non-list response (e.g., {"message": "Not Found"}), json.loads will return a dictionary. Iterating over issue_comments or pull_comments directly will then iterate over the dictionary keys (strings), leading to an AttributeError when calling .get() on lines 677 or 686. This will crash the script. Validate that the parsed JSON is indeed a list before using it.
| issue_comments: Sequence[Mapping[str, Any]] = ( | |
| json.loads(issue_proc.stdout) if issue_proc.returncode == 0 else [] | |
| ) | |
| pull_comments: Sequence[Mapping[str, Any]] = ( | |
| json.loads(pull_proc.stdout) if pull_proc.returncode == 0 else [] | |
| ) | |
| issue_json = json.loads(issue_proc.stdout) if issue_proc.returncode == 0 else [] | |
| issue_comments: Sequence[Mapping[str, Any]] = ( | |
| issue_json if isinstance(issue_json, list) else [] | |
| ) | |
| pull_json = json.loads(pull_proc.stdout) if pull_proc.returncode == 0 else [] | |
| pull_comments: Sequence[Mapping[str, Any]] = ( | |
| pull_json if isinstance(pull_json, list) else [] | |
| ) |
There was a problem hiding this comment.
Addressed in 5aab6f7: validated that issue_json and pull_json are list instances (isinstance(..., list)) and that each element is a Mapping before iterating.
| if isinstance(raw_ignore, dict): | ||
| ignored_packages = [str(x) for x in raw_ignore.get("ignored", [])] |
There was a problem hiding this comment.
If ignore.json contains "ignored": null, raw_ignore.get("ignored", []) will return None. Iterating over None in the list comprehension will raise a TypeError and crash the script. Safely check that the value is a list before processing.
| if isinstance(raw_ignore, dict): | |
| ignored_packages = [str(x) for x in raw_ignore.get("ignored", [])] | |
| if isinstance(raw_ignore, dict): | |
| ignored_val = raw_ignore.get("ignored") | |
| ignored_packages = ( | |
| [str(x) for x in ignored_val] if isinstance(ignored_val, list) else [] | |
| ) |
There was a problem hiding this comment.
Addressed in 5aab6f7: checked isinstance(ignored_val, list) on raw_ignore.get("ignored") before iterating so a null value in ignore.json safely defaults to [].
| try: | ||
| content = full_path.read_text(encoding="utf-8") | ||
| except OSError: | ||
| continue |
There was a problem hiding this comment.
Reading files with read_text(encoding="utf-8") can raise a UnicodeDecodeError if a file contains invalid UTF-8 sequences (e.g., binary files or legacy fixtures). Since UnicodeDecodeError inherits from ValueError, it will not be caught by except OSError:, causing the audit to crash. Catch ValueError as well to make the file reading robust.
| try: | |
| content = full_path.read_text(encoding="utf-8") | |
| except OSError: | |
| continue | |
| try: | |
| content = full_path.read_text(encoding="utf-8") | |
| except (OSError, ValueError): | |
| continue |
There was a problem hiding this comment.
Addressed in 5aab6f7: updated the exception handler around full_path.read_text(encoding="utf-8") to except (OSError, ValueError): so UnicodeDecodeError is caught cleanly.
| MAX_GEMINI_REVIEW_ROUNDS = 3 | ||
| GEMINI_REVIEW_TRIGGER_COMMAND = "/gemini review" | ||
|
|
||
| STYLE_MAINTENANCE_PREFIX = "[Style Maintence]" |
There was a problem hiding this comment.
There is a typo in the prefix name: "[Style Maintence]" should be "[Style Maintenance]". Correcting this typo will improve readability and maintainability of the commit prefixes. Please also update the corresponding references in SKILL.md and the script's help text.
| STYLE_MAINTENANCE_PREFIX = "[Style Maintence]" | |
| STYLE_MAINTENANCE_PREFIX = "[Style Maintenance]" |
- Fix `[Style Maintenance]` commit prefix spelling across `passci.py` and `SKILL.md` - Validate that `gh api` JSON responses are lists before iterating in `_fetch_pr_gemini_reviews` - Guard against `"ignored": null` in `ignore.json` when reading ignored packages - Catch `ValueError` (including `UnicodeDecodeError`) alongside `OSError` when reading changed files
- Conduct 2 independent Gemini reviews per round locally within Jetski (up to 3 rounds or until no high-priority issues come up) instead of repeatedly triggering `/gemini review` on the PR - Enforce a strict 2-push budget (Push #1 when opening the draft PR, Push #2 after all local `[Style Maintenance]`, `[Independent review follow-ups]`, and local `[Address CI errors]` commits are complete) so GitHub Actions unit tests do not reach their quota - Run 1 final `/gemini review` on the PR at the very end to confirm no outstanding major issues remain
…Style Maintenance] prefix
This branch has not been deployed
2 participants
Description
Adds an opt-in
/passciagent skill under.agents/skills/passci/that guides coding agents to open a draft pull request immediately after solving a task (Push #1 of 2), create local[Style Maintenance],[Independent review follow-ups], and[Address CI errors]commits, push all follow-up commits in a single batch (Push #2 of 2) to preserve GitHub Actions quota, and run 1 final"/gemini review"on the PR to confirm no major issues remain.Impact
Ensures that pull requests created with
/passcisurface the initial solution in a draft PR right away while systematically enforcing repository style guidelines, two independent Gemini reviews per round within Jetski (up to 5 rounds or until no high-priority issues come up), and 95% unit test confidence (including packages skipped by conditional CI triggers) while requiring only 2git pushinvocations so GitHub unit tests do not reach their quota.Changes
.agents/skills/passci/SKILL.mddefining the 5-stage opt-in/passciworkflow with a strict 2-push budget:CONTRIBUTING.md,core/packages/gax/CONTRIBUTING.md, the Google TypeScript and JavaScript Style Guides,gts,.eslintrc.json,.prettierrc.cjs, andbin/linter.mjs) and create local style commits prefixed with[Style Maintenance].[Independent review follow-ups]up to five times or until no high-priority issues come up, whatever comes first.ci/run_conditional_tests.sh), resolve any local CI/test failures with commits prefixed with[Address CI errors], perform Push Add transactional query, get, put and del #2 of 2 to push all follow-up commits in a single batch, and run 1 final"/gemini review"on the PR to confirm no outstanding major issues remain..agents/skills/passci/scripts/passci.pyto deterministically auditCONTRIBUTING.mdstyle compliance, evaluate independent Gemini review rounds for high-priority issues, enforce the 2-push budget, identify CI blind spots, compute 95% confidence unit test plans, and validate commit prefix ordering.Testing
passci.pyunit tests covering 95% confidence sample size calculation (CONTRIBUTING.mdstyle auditing, and commit prefix ordering..agents/skills/passci/scripts/passci.py --repo-root . --mode auditagainst thegoogle-cloud-noderepository.Alternatives
/gemini reviewon the PR and pushing after every intermediate commit was considered, but conducting the iterative independent Gemini reviews locally within Jetski (2 reviewers per round, up to 5 rounds) and batching all follow-up commits into a single second push avoids exhausting GitHub Actions unit test quota while still running 1 final/gemini reviewon the PR to confirm no major issues remain./passciworkflow ingoogleapis/google-cloud-node.