Skip to content

Commit 1ceb5b7

Browse files
ahkcsahkcs
committed
Merge upstream/main into pr/async-executor-update
Catch up feature/mustang-ppl-integration with main. Resolved 16 merge conflicts: Mechanical / append-both (9 files): - CLAUDE.md, docs/user/ppl/functions/condition.md (took main) - explain_streamstats_global{,_null_bucket}.yaml (took main, post-#5359) - core/calcite/CalciteRelNodeVisitor.java, core/calcite/utils/PlanUtils.java (collation utility was hoisted from CalciteRelNodeVisitor.backtrackForCollation into PlanUtils.findInputCollation on main; took main) - integ-test/calcite/CalciteNoPushdownIT.java (added CalciteMixedFieldTypeIT to suite) - ppl/antlr/OpenSearchPPLParser.g4 (added unionCommand) - ppl/calcite/CalcitePPLStreamstatsTest.java (added testMultipleStreamstatsWithWindow) Took main (api LanguageSpec evolution subsumes our side, 5 files): - api/UnifiedQueryContext.java, UnifiedQueryPlanner.java - api/UnifiedQueryPlannerSqlTest.java, UnifiedQueryTestBase.java - plugin/SQLPlugin.java imports Hand-merged (2 files): - core/executor/QueryService.java: composed analytics-engine's CalciteClassLoaderHelper wrapper around main's StageErrorHandler stage-tracked phases. Both improvements are orthogonal and apply. - plugin/transport/TransportPPLQueryAction.java: kept both the queryPlanExecutor parameter (analytics-engine wiring) and extensionsHolder parameter (main's engine extensions) since both are referenced in the constructor body. Signed-off-by: Kai Huang <ahkcs@amazon.com>
2 parents 5c5ffa2 + d3bdca8 commit 1ceb5b7

279 files changed

Lines changed: 17833 additions & 1060 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/commands/ppl-bugfix.md‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
---
2+
allowed-tools: Agent, Read, Bash(gh:*), Bash(git:*)
3+
description: Run the PPL bugfix harness for a GitHub issue or follow up on an existing PR
4+
---
5+
6+
Fix a PPL bug or follow up on an existing PR using the harness in `.claude/harness/ppl-bugfix-harness.md`.
7+
8+
## Input
9+
10+
Accepts one or more issue/PR references. Multiple references are processed in parallel (each gets its own subagent + worktree).
11+
12+
- `/ppl-bugfix #1234` — single issue
13+
- `/ppl-bugfix PR#5678` — single PR
14+
- `/ppl-bugfix #1234 #5678 PR#9012` — multiple in parallel
15+
- `/ppl-bugfix https://github.com/opensearch-project/sql/issues/1234` — URL
16+
17+
Optional mode flag (append to any of the above):
18+
- `--safe` — `acceptEdits` mode. Auto-approve file edits only, Bash commands require manual approval. (Most conservative)
19+
- `--yolo` — `bypassPermissions` mode. Fully trusted, no prompts. Subagent runs in an isolated worktree so this is safe. (Default)
20+
21+
> **Note**: `bypassPermissions` skips the interactive prompt but still respects the allow-list in `~/.claude/settings.json`. Ensure git/gh write commands are in the global allow-list.
22+
23+
Examples:
24+
- `/ppl-bugfix #1234` — single issue, defaults to yolo
25+
- `/ppl-bugfix #1234 #5678 --yolo` — two issues in parallel
26+
- `/ppl-bugfix PR#5293 PR#5300` — two PRs in parallel
27+
- `/ppl-bugfix #1234 PR#5678 --safe` — mix of issue and PR
28+
29+
If no argument given, ask for an issue or PR number.
30+
31+
## Step 0: Resolve Permission Mode
32+
33+
Parse the mode flag from the input arguments:
34+
35+
| Flag | Mode |
36+
|------|------|
37+
| `--safe` | `acceptEdits` |
38+
| `--yolo` | `bypassPermissions` |
39+
| _(no flag)_ | `bypassPermissions` (default) |
40+
41+
Use the resolved mode as the `mode` parameter when dispatching the subagent in Step 2A/2B.
42+
43+
## Step 1: Resolve Each Reference
44+
45+
For each issue/PR reference in the input, resolve its state. Run these lookups in parallel when there are multiple references.
46+
47+
```bash
48+
# Issue → PR (check multiple closing keyword variants)
49+
gh pr list --search "Resolves #<issue_number>" --json number,url,state --limit 5
50+
gh pr list --search "Fixes #<issue_number>" --json number,url,state --limit 5
51+
gh pr list --search "Closes #<issue_number>" --json number,url,state --limit 5
52+
53+
# PR → Issue
54+
gh pr view <pr_number> --json body | jq -r '.body' | grep -oiE '(resolves|fixes|closes) #[0-9]+' | grep -oE '[0-9]+'
55+
```
56+
57+
| State | Action |
58+
|-------|--------|
59+
| Issue exists, no PR | **Initial Fix** (Step 2A) |
60+
| Issue exists, open PR found | **Follow-up** (Step 2B) |
61+
| PR provided directly | **Follow-up** (Step 2B) |
62+
63+
## Step 2: Dispatch Subagents
64+
65+
Dispatch one subagent per reference. When there are multiple references, dispatch all subagents in a single message (parallel execution).
66+
67+
### 2A: Initial Fix
68+
69+
```
70+
Agent(
71+
mode: "<resolved_mode>",
72+
isolation: "worktree",
73+
name: "bugfix-<issue_number>",
74+
description: "PPL bugfix #<issue_number>",
75+
prompt: "Read .claude/harness/ppl-bugfix-harness.md and follow it to fix GitHub issue #<issue_number>.
76+
Follow Phase 0 through Phase 3 in order.
77+
Phase 0.3 defines TDD execution flow. Do NOT skip any phase.
78+
CRITICAL: If Phase 0.1 determines the bug is already fixed on main, HARD STOP.
79+
Do NOT write tests, do NOT create a PR — just comment/close the issue and report back.
80+
If the bug IS reproducible, post the Decision Log (Phase 3.4) before completing."
81+
)
82+
```
83+
84+
### 2B: Follow-up
85+
86+
Before dispatching, check if an existing worktree already has the PR branch checked out:
87+
88+
```bash
89+
# List worktrees and find one on the PR branch
90+
for wt in .claude/worktrees/agent-*/; do
91+
branch=$(git -C "$wt" branch --show-current 2>/dev/null)
92+
if [ "$branch" = "<pr_branch>" ]; then
93+
echo "REUSE: $wt (branch: $branch)"
94+
fi
95+
done
96+
```
97+
98+
**If existing worktree found**: Do NOT use `isolation: "worktree"`. Pass the worktree path in the prompt so the subagent works there directly.
99+
100+
```
101+
Agent(
102+
mode: "<resolved_mode>",
103+
name: "bugfix-<issue_number>",
104+
description: "PPL bugfix #<issue_number> followup",
105+
prompt: "cd <worktree_path> first, then read .claude/harness/ppl-bugfix-followup.md and follow it.
106+
PR: <pr_number> (<pr_url>), Issue: #<issue_number>
107+
Working directory: <worktree_path>"
108+
)
109+
```
110+
111+
**If no existing worktree**: Create a new one.
112+
113+
```
114+
Agent(
115+
mode: "<resolved_mode>",
116+
isolation: "worktree",
117+
name: "bugfix-<issue_number>",
118+
description: "PPL bugfix #<issue_number> followup",
119+
prompt: "Read .claude/harness/ppl-bugfix-followup.md and follow it.
120+
PR: <pr_number> (<pr_url>), Issue: #<issue_number>"
121+
)
122+
```
123+
124+
## Step 3: Report Back
125+
126+
After all subagents complete, report a summary for each:
127+
- Classification, fix summary, PR URL, worktree path and branch, items needing human attention (2A)
128+
- What was addressed, current PR state, whether another round is needed (2B)
129+
130+
**Always include the worktree→PR mapping** from the subagent's output, e.g.:
131+
132+
```
133+
Worktree: /path/to/.claude/worktrees/agent-xxxx
134+
Branch: bugfix-1234
135+
PR: #5678
136+
```
137+
138+
**Important**: After reporting, the main agent must remember this mapping. When the user later asks to make changes to the PR (e.g., "commit this to PR #5678"), operate in the worktree directory — not the main session directory.
139+
140+
## Subagent Lifecycle
141+
142+
Subagents are task-scoped. They complete and release context — they cannot poll for events.
143+
144+
```
145+
Agent A (Phase 0-3) → creates PR → completes
146+
(CI runs, reviewers comment, conflicts arise)
147+
Agent B (Phase 3.5) → handles feedback → completes
148+
(repeat as needed)
149+
Agent N (Phase 3.5) → gh pr ready → done
150+
```
151+
152+
Context is preserved across agents via:
153+
- **Decision Log** (PR comment) — single source of truth for rejected alternatives, pitfalls, design rationale
154+
- **GitHub state** (PR diff, review comments, CI logs) — reconstructed by each follow-up agent
155+
156+
## Rules
157+
158+
- Subagent reads `.claude/harness/ppl-bugfix-harness.md` and fetches issue/PR details itself — do NOT inline content into the prompt
159+
- If bug is not reproducible (Phase 0.1), stop and report — do not proceed
160+
- Issue ↔ PR auto-resolution means the user never needs to track PR numbers manually
161+
- **Do NOT use `mode: "auto"` for subagents** — `auto` mode does not work for subagents; Bash commands still require manual approval. Only `bypassPermissions` reliably skips permission checks.
162+
- **Always dispatch subagent** — even for trivial follow-ups (remove co-author, force push). Do NOT run commands directly in the main session; subagents with `bypassPermissions` skip permission prompts, the main session does not.

‎.claude/harness/ppl-bugfix-followup.md‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# PPL Bugfix Follow-up
2+
3+
## Rules
4+
5+
- Do NOT add `Co-Authored-By` lines in commits — only DCO `Signed-off-by`
6+
7+
---
8+
9+
## Report Working Directory
10+
11+
```bash
12+
echo "Worktree: $(pwd)"
13+
echo "Branch: $(git branch --show-current)"
14+
```
15+
16+
Include this in your output so the caller knows where changes are happening.
17+
18+
## Reconstruct Context
19+
20+
First checkout the PR branch, then load state:
21+
22+
```bash
23+
# Checkout the PR branch in this worktree
24+
gh pr checkout <pr_number>
25+
26+
# Resolve fork remote — the worktree may only have origin (upstream)
27+
git remote -v
28+
# If no fork remote exists, add it:
29+
git remote add fork https://github.com/<fork_owner>/sql.git
30+
31+
# Load PR state — reviews, CI, mergeability
32+
gh pr view <pr_number> --json title,body,state,reviews,statusCheckRollup,mergeable
33+
gh pr checks <pr_number>
34+
35+
# Load ALL comments — includes bot comments (Code-Diff-Analyzer, PR Reviewer Guide, Code Suggestions) and human comments
36+
gh pr view <pr_number> --json comments --jq '.comments[] | {author: .author.login, body: .body}'
37+
```
38+
39+
Categorize ALL signals — not just CI and human reviews:
40+
41+
| Signal | Type |
42+
|--------|------|
43+
| `statusCheckRollup` has failures | CI failure |
44+
| `reviews` has CHANGES_REQUESTED | Review feedback |
45+
| `mergeable` is CONFLICTING | Merge conflict |
46+
| Bot comments with actionable suggestions | Review feedback (treat like human review) |
47+
| All pass + approved | Ready — run `gh pr ready` |
48+
49+
## Handle Review Feedback
50+
51+
For each comment (human OR bot), **cross-check against the Decision Log first**:
52+
53+
| Type | Action |
54+
|------|--------|
55+
| Code change | If already rejected in Decision Log, reply with reasoning. Otherwise make the change, new commit, push |
56+
| Question | Reply with explanation — Decision Log often has the answer |
57+
| Nit | Fix if trivial |
58+
| Disagreement | Reply with Decision Log reasoning; if reviewer insists, escalate to user |
59+
60+
```bash
61+
git add <files> && git commit -s -m "Address review feedback: <description>"
62+
git push -u fork <branch_name>
63+
```
64+
65+
## Clean Up Commit History
66+
67+
When you need to amend a commit (e.g. remove Co-Authored-By, reword message) and the branch has a merge commit on top, don't try `git reset --soft origin/main` — it will include unrelated changes if main has moved. Instead cherry-pick the fix onto latest main:
68+
69+
```bash
70+
git checkout -B clean-branch origin/main
71+
git cherry-pick <fix_commit_sha>
72+
git commit --amend -s -m "<updated message>"
73+
git push fork clean-branch:<pr_branch> --force-with-lease
74+
```
75+
76+
## Handle CI Failures
77+
78+
```bash
79+
gh pr checks <pr_number> # Identify failures
80+
gh run view <run_id> --log-failed # Read logs
81+
# Test failure → fix locally, push new commit
82+
# Spotless → ./gradlew spotlessApply, push
83+
# Flaky → gh run rerun <run_id> --failed
84+
```
85+
86+
## Handle Merge Conflicts
87+
88+
```bash
89+
git fetch origin && git merge origin/main # Resolve conflicts
90+
./gradlew spotlessApply && ./gradlew test && ./gradlew :integ-test:integTest # Re-verify
91+
git commit -s -m "Resolve merge conflicts with main"
92+
git push -u fork <branch_name>
93+
```
94+
95+
## Mark Ready
96+
97+
```bash
98+
gh pr ready <pr_number>
99+
```
100+
101+
## Retrospective
102+
103+
After handling follow-up, reflect on the feedback received and check if it reveals gaps in the harness or command:
104+
105+
For each comment addressed (bot or human):
106+
- **Does the feedback point to a pattern the harness should have prevented?** → Add guidance to the relevant Phase in `ppl-bugfix-harness.md`
107+
- **Was this a repeated mistake across PRs?** → Add to Quick Reference or Case Index
108+
- **Did the harness template produce the problematic code?** → Fix the template directly
109+
- **Was a permission or tool missing?** → Add to `.claude/settings.json`
110+
- **Did the follow-up workflow itself miss this signal?** → Update this file
111+
112+
If any improvement is needed, make the edit and include it in the same commit.
113+
114+
## Completion Gate
115+
116+
Before reporting "done":
117+
118+
1. Run `git status --porcelain` — if any uncommitted changes remain, commit and push them. This includes harness edits from Retrospective.
119+
2. Report in your final output:
120+
121+
```
122+
Worktree: <absolute path>
123+
Branch: <branch name>
124+
PR: <pr_number>
125+
```

0 commit comments

Comments
 (0)