Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
40 changes: 33 additions & 7 deletions .claude/commands/drain.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,17 +189,43 @@ for PR in $OPEN_PRS; do
done
```

### Fix branch-specific CI failures
### Phase 3b: Parallel per-PR agents (MANDATORY for remaining failures)

For every still-failing PR after Phase 3, spawn ONE Agent per PR IN PARALLEL using `isolation: "worktree"` and `mode: "bypassPermissions"`. Send all spawn calls in a single message (multiple Agent tool calls in one turn) so they run concurrently.

Each agent's prompt must include:
1. The PR number, title, head branch
2. Current failing checks (name + log URL from `gh pr checks $PR`)
3. Any unresolved bot/human review comments (fetch via `gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate`)

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 PR-level comments missed by review-comments endpoint

gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate fetches only inline review comments (line-level). General PR-level bot review summaries (e.g. the top-level CodeRabbit or Greptile summary comment) are posted as issue comments and are only accessible via gh api repos/{owner}/{repo}/issues/$PR/comments --paginate. Agents instructed to "address each unaddressed CodeRabbit / Greptile comment" will silently miss all summary-level feedback unless both endpoints are fetched.

Suggested change
3. Any unresolved bot/human review comments (fetch via `gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate`)
3. Any unresolved bot/human review comments (fetch both endpoints):
- `gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate` (inline review comments)
- `gh api repos/{owner}/{repo}/issues/$PR/comments --paginate` (PR-level / summary comments)

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does GitHub REST endpoint GET /repos/{owner}/{repo}/pulls/{pull_number}/comments include review-thread resolved/unresolved state, and what is the recommended API (GraphQL reviewThreads?) to fetch only unresolved PR review comments?

💡 Result:

No, the GitHub REST endpoint GET /repos/{owner}/{repo}/pulls/{pull_number}/comments does not include review-thread resolved/unresolved state. The response returns an array of PullRequestReviewComment objects, which include fields like id, url, body, path, position, etc., but no field indicating the resolved state of the review thread the comment belongs to. The recommended API to fetch only unresolved PR review comments is the GitHub GraphQL API. Query the pullRequest's reviewThreads connection, filtering or post-filtering for those where isResolved: false, then access the comments within those threads. Example GraphQL query: { repository(owner: "OWNER", name: "REPO") { pullRequest(number: PULL_NUMBER) { reviewThreads(first: 100, unresolved: true) { nodes { isResolved comments(first: 100) { nodes { id body author { login } path line } } } } } } } Note: The reviewThreads connection supports an 'unresolved: Boolean' argument to fetch only unresolved threads.

Citations:


REST API endpoint cannot reliably fetch unresolved review comments—use GraphQL instead

The gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate endpoint does not return thread resolution state, so it cannot distinguish between resolved and unresolved comments. Agents using this will re-process already-resolved feedback.

Use GitHub's GraphQL API with reviewThreads(unresolved: true) to fetch only unresolved threads:

repository(owner: "OWNER", name: "REPO") {
  pullRequest(number: PULL_NUMBER) {
    reviewThreads(first: 100, unresolved: true) {
      nodes {
        isResolved
        comments(first: 100) {
          nodes { id body author { login } path line }
        }
      }
    }
  }
}
Suggested doc fix
-3. Any unresolved bot/human review comments (fetch via `gh api repos/{owner}/{repo}/pulls/$PR/comments --paginate`)
+3. Any unresolved bot/human review comments (fetch via GraphQL reviewThreads with unresolved: true filter to avoid re-processing resolved feedback)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/drain.md at line 199, Replace the REST call "gh api
repos/{owner}/{repo}/pulls/$PR/comments --paginate" with a GraphQL query that
calls repository { pullRequest(number: PULL_NUMBER) { reviewThreads(unresolved:
true) { nodes { isResolved comments(first: 100) { nodes { id body author { login
} path line } } } } } } so the agent fetches only unresolved review threads;
update any code that parses the old REST response to instead iterate
reviewThreads.nodes and their comments, preserving the existing logic that used
comment id/body/author/path/line for downstream processing.

4. Instructions:
- Check out the PR's head branch in the worktree
- Rebase onto latest `main`; resolve conflicts preserving PR intent
- Run `pnpm --filter=@jovie/web exec tsc --noEmit`, `pnpm biome check apps/web`, and the failing test files locally
- Fix root causes (not symptoms); update tests only if they're wrong
- Address each unaddressed CodeRabbit / Greptile comment (fix or reply explaining why declined)
- Commit with conventional messages, push to the PR head branch
- Enable auto-merge via `gh pr merge $PR --auto`
- Report DONE / DONE_WITH_CONCERNS / BLOCKED with the specific reason

Sample Agent tool call (repeat per PR, all in one message):

For each PR that still has failures after the rebase:
1. Check out the branch
2. Identify the failure (typecheck, lint, test, etc.)
3. Fix it, commit, and push
4. Wait for CI to go green, then enable auto-merge
```

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.

⚠️ Potential issue | 🟡 Minor

Add a language tag to the fenced code block

Line 212 violates markdownlint MD040 (missing fenced code language). Add a language identifier (e.g., text or javascript) to keep docs lint-clean.

Suggested doc fix
-```
+```javascript
 Agent({
   subagent_type: "general-purpose",
   isolation: "worktree",
   mode: "bypassPermissions",
   description: "Fix PR #<NUMBER>",
   prompt: "<self-contained prompt with failures + comments + instructions>"
 })
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **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.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 212-212: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/drain.md at line 212, The fenced code block containing the
Agent({...}) example is missing a language tag; update the opening fence from
``` to include a language identifier (for example change it to ```javascript or
```text) so the block starting with Agent({ subagent_type: "general-purpose",
isolation: "worktree", mode: "bypassPermissions", description: "Fix PR
#<NUMBER>", prompt: "<self-contained prompt with failures + comments +
instructions>" }) complies with markdownlint MD040.

Agent({
subagent_type: "general-purpose",
isolation: "worktree",
mode: "bypassPermissions",
description: "Fix PR #<NUMBER>",
prompt: "<self-contained prompt with failures + comments + instructions>"
})
Comment on lines +213 to +219

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 mode is not a valid Agent tool parameter

The Agent tool schema only accepts subagent_type, description, prompt, run_in_background, and isolation. There is no mode parameter. The PR description itself acknowledges that bypassPermissions "doesn't work as documented" inside worktrees. As written, the mode: "bypassPermissions" key will be silently ignored (or raise a schema error depending on the harness version), and sub-agents will run with standard sandbox restrictions, causing the Bash commands inside the worktree to fail. This known gap should be documented directly in the drain skill (not only in the PR description) so operators know the fan-out step is not fully functional until the harness issue is resolved.

```

Do NOT wait for one agent before spawning the next. Do NOT run them sequentially. The point of this phase is fan-out.

After all agents return, re-run Phase 3 auto-merge enablement and report.
Comment on lines +192 to +224

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 No fan-out guard for large PR queues

The instruction says to spawn one Agent per still-failing PR in a single turn with no upper bound. If the queue has 20+ failing PRs, this creates 20+ concurrent worktree agents simultaneously consuming disk I/O, memory, and GitHub API quota. Consider adding a practical cap (e.g. process at most 10 PRs in parallel, then iterate) and a note that gh pr merge --auto requires auto-merge to be enabled on the repository settings.


### Close stale or stuck PRs

PRs that can't be fixed automatically:
PRs that can't be fixed automatically (agent returns BLOCKED):
- Close with a comment explaining why
- Label as `needs-human` if human intervention is required

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ jobs:
needs: [ci-build-public, ci-path-changes, neon-db]
# Runs Lighthouse CI against the public-route build artifact on PRs.
# Blocks PRs when triggered (path-gated to public-route changes).
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && needs.ci-build-public.outputs.has_artifact == 'true' && needs.ci-path-changes.outputs.run_public_lighthouse == 'true' }}
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-build-public.outputs.has_artifact == 'true' && needs.ci-path-changes.outputs.run_public_lighthouse == '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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In GitHub Actions pull_request workflows, what is the behavioral difference between github.actorandgithub.event.pull_request.user.login, especially for manual re-runs, and which one is stable for identifying Dependabot-authored PRs?

💡 Result:

In GitHub Actions pull_request workflows, github.actor identifies the user or app that triggered the specific workflow run (the "executing actor"), while github.event.pull_request.user.login identifies the author of the pull request itself from the event payload. The key behavioral difference, especially for manual re-runs, is that since July 2022, re-runs use the privileges and actor of the initial triggering run for consistency and security. github.actor remains the original triggerer (e.g., 'dependabot[bot]' for initial Dependabot PR runs), even if manually re-run by a human user. The re-run triggerer is separately available as github.triggering_actor (displayed in UI), but github.actor is stable to the original. For identifying Dependabot-authored PRs reliably (including across re-runs, pushes to the branch, or other triggers), use github.event.pull_request.user.login == 'dependabot[bot]'. Official docs and examples recommend this for Dependabot checks (e.g., in pull_request_target for security restrictions). github.actor may vary if the workflow trigger differs from PR creation (e.g., workflow_run, other bots), though stable on re-runs. Examples: - Initial Dependabot PR open/synchronize: both == 'dependabot[bot]' - Manual re-run of that workflow: github.actor stays 'dependabot[bot]', PR author unchanged. - Push to Dependabot branch by user: new run has github.actor as user, but PR author still 'dependabot[bot]'. - workflow_run triggered by Dependabot test completion: actor may be Dependabot, but use PR author for safety.

Citations:


🏁 Script executed:

# Check the workflow file at the specified lines
cd .github/workflows && wc -l ci.yml

Repository: JovieInc/Jovie

Length of output: 69


🏁 Script executed:

# Read the specific lines mentioned in the review
sed -n '780,785p' .github/workflows/ci.yml
sed -n '1033,1038p' .github/workflows/ci.yml
sed -n '1167,1172p' .github/workflows/ci.yml

Repository: JovieInc/Jovie

Length of output: 1318


🏁 Script executed:

# Get more context around each line to understand the job structure
sed -n '770,790p' .github/workflows/ci.yml
sed -n '1025,1045p' .github/workflows/ci.yml
sed -n '1160,1180p' .github/workflows/ci.yml

Repository: JovieInc/Jovie

Length of output: 3210


Use PR author context for Dependabot gating, not triggering actor.

Lines 781, 1034, and 1168 use github.actor != 'dependabot[bot]' to skip Neon-dependent jobs for Dependabot PRs. However, if someone pushes to a Dependabot PR branch, github.actor becomes that user while the PR author remains 'dependabot[bot]'. This can cause these Neon-dependent jobs to execute unexpectedly and access secrets they should not. Use github.event.pull_request.user.login instead, which reliably identifies the PR author across all scenarios.

🔧 Suggested fix
- if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-build-public.outputs.has_artifact == 'true' && needs.ci-path-changes.outputs.run_public_lighthouse == 'true' }}
+ if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.event.pull_request.user.login != 'dependabot[bot]' && needs.ci-build-public.outputs.has_artifact == 'true' && needs.ci-path-changes.outputs.run_public_lighthouse == 'true' }}

- if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_onboarding_lighthouse == 'true' }}
+ if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.event.pull_request.user.login != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_onboarding_lighthouse == 'true' }}

- if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_admin_lighthouse == 'true' }}
+ if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.event.pull_request.user.login != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_admin_lighthouse == 'true' }}

Also applies to: 1034-1034, 1168-1168

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml at line 781, Replace checks that compare
github.actor to 'dependabot[bot]' with checks against the PR author using
github.event.pull_request.user.login; specifically update occurrences of the
condition snippet "github.actor != 'dependabot[bot]'" inside the CI job
if-expressions to "github.event.pull_request.user.login != 'dependabot[bot]'" so
the gating reliably uses the PR author (e.g., where the workflow contains the
existing if for run_public_lighthouse and the similar Neon-dependent job
if-expressions).

runs-on: ubuntu-latest
timeout-minutes: 30
steps:
Expand Down Expand Up @@ -1031,7 +1031,7 @@ jobs:
ci-lighthouse-onboarding-pr:
name: Lighthouse (onboarding PR)
needs: [ci-fast, ci-path-changes, neon-db]
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && needs.ci-path-changes.outputs.run_onboarding_lighthouse == 'true' }}
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_onboarding_lighthouse == 'true' }}
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
Expand Down Expand Up @@ -1165,7 +1165,7 @@ jobs:
# so dependent jobs would silently skip on admin-UI-only PRs. This job creates
# its own ephemeral branch below and does not reuse the shared neon-db branch.
needs: [ci-fast, ci-path-changes]
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && needs.ci-path-changes.outputs.run_admin_lighthouse == 'true' }}
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]' && needs.ci-path-changes.outputs.run_admin_lighthouse == 'true' }}
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
Expand Down
Loading