[nightshift] Parallel sub-agents, automerge, and reviewer selection#4391
[nightshift] Parallel sub-agents, automerge, and reviewer selection#4391
Conversation
Rewrite nightshift_cleanup.py to spawn one scout agent per subproject in parallel git worktrees, then merge results into a single PR. Add rebase on origin/main, gh pr merge --auto --squash, and git-log-based reviewer selection to both cleanup and doc-drift workflows. Bump cleanup timeout to 360 minutes.
|
Claude finished @rjpower's task in 1m 37s —— View job PR Review: Parallel sub-agents, automerge, and reviewer selection
SummaryGood architectural change — splitting into parallel scout agents per subproject with a merge agent is a solid design that should improve coverage and reduce wasted turns. The doc-drift automerge/reviewer additions are clean. I left 7 inline comments on Issues to address
Design observations
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| cwd=repo_root, | ||
| ) | ||
|
|
||
| result_file = tempfile.mktemp(suffix=".json", prefix=f"nightshift-{worktree_name}-") |
There was a problem hiding this comment.
tempfile.mktemp is deprecated and has a TOCTOU race — the path could be claimed between creation and the scout writing to it. Use tempfile.NamedTemporaryFile(delete=False) instead:
| result_file = tempfile.mktemp(suffix=".json", prefix=f"nightshift-{worktree_name}-") | |
| with tempfile.NamedTemporaryFile(suffix=".json", prefix=f"nightshift-{worktree_name}-", delete=False) as f: | |
| result_file = f.name |
| subprocess.run(["git", "branch", "-D", branch_name], capture_output=True, cwd=repo_root) | ||
|
|
||
| # Create worktree on a fresh branch from origin/main | ||
| subprocess.run(["git", "fetch", "origin", "main"], check=True, cwd=repo_root, capture_output=True) |
There was a problem hiding this comment.
Each of the 4 scouts runs git fetch origin main independently. Since they execute in parallel via ThreadPoolExecutor, this creates 4 concurrent fetches of the same ref. The fetch already happens once in main() at line 236 before scouts are launched — this one could be removed (or guarded with a note that the worktree creation depends on origin/main being fresh).
| futures = {pool.submit(run_scout, sp, date, repo_root): sp for sp in SUBPROJECTS} | ||
| for future in as_completed(futures): | ||
| subproject, result, worktree_path = future.result() |
There was a problem hiding this comment.
If any scout raises an exception (e.g. the worktree git checkout fails with check=True), future.result() will re-raise it here and kill the entire run — including any scouts still in-flight. Consider wrapping with try/except to let other scouts finish:
for future in as_completed(futures):
try:
subproject, result, worktree_path = future.result()
except Exception:
sp = futures[future]
logger.exception("Scout %s failed", sp)
scout_results.append({"subproject": sp, "status": "error", "summary": "Scout raised an exception"})
continue
...
| ) | ||
|
|
||
| result = {"subproject": subproject, "status": "error", "summary": "Scout did not produce a result file"} | ||
| if Path(result_file).exists(): |
There was a problem hiding this comment.
Malformed JSON from the scout will crash with an unhandled json.JSONDecodeError. Worth wrapping:
| if Path(result_file).exists(): | |
| if Path(result_file).exists(): | |
| try: | |
| result = json.loads(Path(result_file).read_text()) | |
| except json.JSONDecodeError: | |
| result = {"subproject": subproject, "status": "error", "summary": "Scout produced invalid JSON"} | |
| Path(result_file).unlink() |
| def run_scout(subproject: str, date: str, repo_root: Path) -> tuple[str, dict, str]: | ||
| """Run a single scout agent in a git worktree. Returns (subproject, result_dict, worktree_path).""" | ||
| worktree_name = f"nightshift-scout-{subproject.replace('/', '-')}" | ||
| worktree_path = repo_root / ".nightshift-worktrees" / worktree_name |
There was a problem hiding this comment.
.nightshift-worktrees is not in .gitignore. If a scout or the cleanup function fails partway through, this directory (or its git metadata) could end up tracked. Consider adding it to .gitignore.
| def cleanup_worktrees(repo_root: Path) -> None: | ||
| """Remove all nightshift scout worktrees.""" | ||
| worktrees_dir = repo_root / ".nightshift-worktrees" | ||
| if worktrees_dir.exists(): | ||
| for child in worktrees_dir.iterdir(): | ||
| subprocess.run(["git", "worktree", "remove", "--force", str(child)], capture_output=True, cwd=repo_root) | ||
| worktrees_dir.rmdir() |
There was a problem hiding this comment.
If any child directory inside .nightshift-worktrees was not a worktree (or was already removed), worktrees_dir.rmdir() will silently succeed. But if git worktree remove fails for one child, rmdir() will raise OSError because the directory isn't empty. Consider using shutil.rmtree as a fallback, or at least logging failures from the worktree-remove loop.
| 6. Pick a reviewer by finding who recently touched the changed files: | ||
| ``` | ||
| git log --format='%ae' -20 -- <changed_files> | sort | uniq -c | sort -rn | head -5 | ||
| ``` |
There was a problem hiding this comment.
The reviewer selection uses --format='%ae' (email), but gh pr edit --add-reviewer expects a GitHub username, not an email. This will work only if the email happens to match the username. You'd need to map emails → GitHub usernames (e.g. via gh api or by using --format='%aN' and matching against known contributors).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aad18d3c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| 6. Pick a reviewer by finding who recently touched the changed files: | ||
| ``` | ||
| git log --format='%ae' -20 -- <changed_files> | sort | uniq -c | sort -rn | head -5 |
There was a problem hiding this comment.
This workflow instructs the agent to select reviewers from git log --format='%ae', which yields email addresses, but gh pr edit --add-reviewer accepts reviewer logins (per gh pr edit help: --add-reviewer <login>). In practice this causes reviewer assignment to fail (or pick invalid identities), so the new auto-reviewer step in cleanup can break PR automation when the selected value is an email.
Useful? React with 👍 / 👎.
|
|
||
| result = {"subproject": subproject, "status": "error", "summary": "Scout did not produce a result file"} | ||
| if Path(result_file).exists(): | ||
| result = json.loads(Path(result_file).read_text()) |
There was a problem hiding this comment.
run_scout is intended to tolerate scout failures (check=False on the Claude call), but it unconditionally parses the result file with json.loads(...) and does not catch decode/read errors. If any scout writes partial/invalid JSON, future.result() raises and aborts the whole cleanup run before merge, which defeats the fault-tolerant parallel design.
Useful? React with 👍 / 👎.
Fix tempfile.mktemp TOCTOU race with NamedTemporaryFile(delete=False). Remove redundant git fetch from scouts (main() fetches once). Wrap future.result() to isolate scout failures. Handle malformed scout JSON. Use shutil.rmtree as fallback in cleanup_worktrees. Switch reviewer selection from git log emails to gh api GitHub logins. Add .nightshift-worktrees to .gitignore.
…4391) Rewrite nightshift_cleanup.py to spawn one scout agent per subproject in parallel git worktrees, then merge results into a single PR. Each scout gets 400 turns to find one meaty improvement in its assigned subproject, then a merge agent cherry-picks all results, runs lint+tests, and opens one unified PR. Also add to both cleanup and doc-drift workflows: rebase on origin/main before pushing, enable automerge via gh pr merge --auto --squash, and select a reviewer from git log on changed files. Cleanup workflow timeout bumped from 240 to 360 minutes for parallel agent overhead.
1 participant
Rewrite nightshift_cleanup.py to spawn one scout agent per subproject in
parallel git worktrees, then merge results into a single PR. Each scout
gets 400 turns to find one meaty improvement in its assigned subproject,
then a merge agent cherry-picks all results, runs lint+tests, and opens
one unified PR.
Also add to both cleanup and doc-drift workflows: rebase on origin/main
before pushing, enable automerge via gh pr merge --auto --squash, and
select a reviewer from git log on changed files. Cleanup workflow timeout
bumped from 240 to 360 minutes for parallel agent overhead.