Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/nightshift-cleanup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ concurrency:
jobs:
cleanup:
runs-on: ubuntu-latest
timeout-minutes: 240
timeout-minutes: 360
permissions:
contents: write
pull-requests: write
Expand Down
223 changes: 203 additions & 20 deletions infra/scripts/nightshift_cleanup.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
# Copyright The Marin Authors
# SPDX-License-Identifier: Apache-2.0

"""Nightshift cleanup: single agent picks a subproject, finds something meaty, opens a PR."""
"""Nightshift cleanup: spawns parallel sub-agents per subproject, merges results into one PR."""

import datetime
import json
import logging
import secrets
import subprocess
import tempfile
from concurrent.futures import ThreadPoolExecutor, as_completed
from pathlib import Path

logger = logging.getLogger(__name__)

SUBPROJECTS = ["lib/marin/src/marin", "lib/iris/src/iris", "lib/zephyr/src/zephyr", "lib/levanter/src/levanter"]

CLEANUP_PROMPT = """\
You are the Nightshift Cleanup Agent.
SCOUT_PROMPT = """\
You are a Nightshift Scout Agent assigned to: {subproject}

Your random seed is: {haiku_seed}
Use this seed to compose a haiku about code maintenance. Include it
as the epigraph of your PR description.

## Your Mission

Pick one of these subprojects to focus on: {subprojects}

Browse the subproject and find something **meaty** to improve — not cosmetic
Browse `{subproject}` and find something **meaty** to improve — not cosmetic
lint, not renaming, but a genuine code quality win. Look for things like:

- Dead code: unused functions, stale TODO/FIXME (>90 days via `git blame`),
Expand All @@ -45,24 +48,157 @@
that test modules you changed.
- If you find issues but the fix is non-trivial, file a GitHub issue instead
of making a risky change.
- Keep the PR focused: one coherent improvement.
- Keep changes focused: one coherent improvement per subproject.

## Output

Create a branch named `nightshift/cleanup-{date}` and open a PR with:
- Title: `[nightshift] <concise description>`
- Body: your haiku, then a summary of what was cleaned and why
- Labels: `agent-generated`, `nightshift`
Do NOT create a PR or push to a remote. Work locally only.

If you made improvements, commit them to the current branch with a message like:
`[nightshift] <subproject>: <concise description>`

If you find nothing worth changing, exit cleanly — no branch, no PR.
Then write a JSON summary to `{result_file}`:
```json
{{
"subproject": "{subproject}",
"status": "changed",
"summary": "<one-paragraph description of what was cleaned and why>",
"files_changed": ["<list of files changed>"]
}}
```

If you find nothing worth changing, write:
```json
{{
"subproject": "{subproject}",
"status": "no_change",
"summary": "<brief note on what you checked>"
}}
```
"""

MERGE_PROMPT = """\
You are the Nightshift Merge Agent.

def main() -> None:
date = datetime.date.today().strftime("%Y%m%d")
prompt = CLEANUP_PROMPT.format(
haiku_seed=secrets.token_hex(4),
subprojects=", ".join(SUBPROJECTS),
Your random seed is: {haiku_seed}
Use this seed to compose a haiku about code maintenance. Include it
as the epigraph of your PR description.

## Context

Parallel scout agents searched these subprojects for cleanup opportunities.
Each scout worked in its own git worktree and committed changes independently.
Their results:

{scout_results}

## Worktrees

The scout worktrees with their commits are at these paths:
{worktree_info}

## Your Mission

1. Cherry-pick the scout commits from each worktree that had `status: "changed"`
into the current branch (`nightshift/cleanup-{date}`). Skip any that conflict
or fail tests after merging.

2. Run `./infra/pre-commit.py --all-files --fix` and `uv run pytest -x` on all
affected test files to verify the combined changes are clean.

3. Before pushing, rebase on origin/main:
```
git fetch origin main
git rebase origin/main
```

4. Open a PR:
- Title: `[nightshift] {date} multi-cleanup`
- Body: your haiku as epigraph, then a combined summary of all scout findings
- Labels: `agent-generated`, `nightshift`

5. Enable automerge:
```
gh pr merge --auto --squash <PR_NUMBER>
```

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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use GitHub logins for reviewer assignment

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 👍 / 👎.

```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Pick the top contributor who is NOT `github-actions[bot]` or `noreply`.
```
gh pr edit <PR_NUMBER> --add-reviewer <username>
```

If no scouts produced changes, exit cleanly — no branch, no PR.
"""


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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.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.

branch_name = f"nightshift/scout-{subproject.replace('/', '-')}-{date}"

# Clean up any stale worktree
subprocess.run(["git", "worktree", "remove", "--force", str(worktree_path)], capture_output=True, cwd=repo_root)
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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

subprocess.run(
["git", "worktree", "add", "-b", branch_name, str(worktree_path), "origin/main"],
check=True,
cwd=repo_root,
)

result_file = tempfile.mktemp(suffix=".json", prefix=f"nightshift-{worktree_name}-")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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

haiku_seed = secrets.token_hex(4)

prompt = SCOUT_PROMPT.format(
subproject=subproject,
haiku_seed=haiku_seed,
result_file=result_file,
)

logger.info("Starting scout for %s in %s", subproject, worktree_path)
subprocess.run(
[
"claude",
"--model=opus",
"--print",
"--dangerously-skip-permissions",
"--tools=Read,Write,Edit,Glob,Grep,Bash",
"--max-turns",
"400",
"--cwd",
str(worktree_path),
"--",
prompt,
],
check=False, # Don't fail the whole run if one scout errors
)

result = {"subproject": subproject, "status": "error", "summary": "Scout did not produce a result file"}
if Path(result_file).exists():
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Malformed JSON from the scout will crash with an unhandled json.JSONDecodeError. Worth wrapping:

Suggested change
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()

result = json.loads(Path(result_file).read_text())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle malformed scout JSON instead of crashing merge run

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 👍 / 👎.

Path(result_file).unlink()

return subproject, result, str(worktree_path)


def run_merge(date: str, haiku_seed: str, scout_results: list[dict], worktree_info: list[tuple[str, str]]) -> None:
"""Run the merge agent to combine scout results into a single PR."""
results_text = "\n".join(
f"### {r['subproject']}\n- Status: {r['status']}\n- Summary: {r.get('summary', 'N/A')}" for r in scout_results
)
worktree_text = "\n".join(f"- `{subproject}`: `{path}`" for subproject, path in worktree_info)

prompt = MERGE_PROMPT.format(
haiku_seed=haiku_seed,
scout_results=results_text,
worktree_info=worktree_text,
date=date,
)

Expand All @@ -74,13 +210,60 @@ def main() -> None:
"--dangerously-skip-permissions",
"--tools=Read,Write,Edit,Glob,Grep,Bash",
"--max-turns",
"800",
"200",
"--",
prompt,
],
check=True,
)


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()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.



def main() -> None:
logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")
date = datetime.date.today().strftime("%Y%m%d")
repo_root = Path(subprocess.check_output(["git", "rev-parse", "--show-toplevel"], text=True).strip())

# Create the merge branch
subprocess.run(["git", "fetch", "origin", "main"], check=True, cwd=repo_root, capture_output=True)
merge_branch = f"nightshift/cleanup-{date}"
subprocess.run(["git", "branch", "-D", merge_branch], capture_output=True, cwd=repo_root)
subprocess.run(["git", "checkout", "-b", merge_branch, "origin/main"], check=True, cwd=repo_root)

# Run scouts in parallel
scout_results: list[dict] = []
worktree_info: list[tuple[str, str]] = []

with ThreadPoolExecutor(max_workers=len(SUBPROJECTS)) as pool:
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()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
    ...

scout_results.append(result)
if result.get("status") == "changed":
worktree_info.append((subproject, worktree_path))
logger.info("Scout %s: %s", subproject, result.get("status"))

changed = [r for r in scout_results if r.get("status") == "changed"]
if not changed:
logger.info("No scouts found changes. Exiting cleanly.")
cleanup_worktrees(repo_root)
return

# Run merge agent to cherry-pick, verify, and open PR
haiku_seed = secrets.token_hex(4)
try:
run_merge(date, haiku_seed, scout_results, worktree_info)
finally:
cleanup_worktrees(repo_root)


if __name__ == "__main__":
main()
26 changes: 23 additions & 3 deletions infra/scripts/nightshift_doc_drift.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,29 @@
- Fix straightforward issues (broken links, wrong imports in examples).
- For larger issues, file a GitHub issue with labels `documentation`,
`agent-generated`, and `nightshift`. Begin the body with your haiku.
- If you made fixes, open a PR titled `[nightshift] fix documentation drift`
with labels `agent-generated` and `nightshift`. Begin the PR body with
your haiku.
- If you made fixes, open a PR following these steps:
1. Before pushing, ensure your branch is up to date with origin/main:
```
git fetch origin main
git rebase origin/main
```
2. Push and open the PR:
- Title: `[nightshift] fix documentation drift`
- Labels: `agent-generated`, `nightshift`
- Begin the PR body with your haiku.
3. Enable automerge on the PR:
```
gh pr merge --auto --squash <PR_NUMBER>
```
4. Pick a reviewer by finding who has recently touched the files you changed:
```
git log --format='%ae' -20 -- <changed_files> | sort | uniq -c | sort -rn | head -5
```
Pick the top contributor who is NOT `github-actions[bot]` or `noreply`.
Request their review:
```
gh pr edit <PR_NUMBER> --add-reviewer <username>
```

If nothing is found, exit cleanly.
"""
Expand Down
Loading