[evoskill] Refine resolve-merge-conflicts skill: detect leftover conflict markers - #57601
Conversation
Add a --verify-markers scan to resolve.sh that checks .github/workflows/*.md source files for leftover git conflict-marker lines (<<<<<<<, |||||||, =======, >>>>>>>), and run it automatically before make recompile in the merge flow. A stray marker left in a workflow frontmatter after a manual conflict resolution is parsed by the gh-aw compiler as an invalid YAML header option, causing compile failures later (often in an unrelated scheduled recompilation run), and git diff --check does not catch markers already committed in unchanged content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Protected files were modified in this pull request and require manual scrutiny before merge.
Please verify that each protected-file change is intentional, policy-compliant, and safe:
- Protected files:
.github/skills/resolve-merge-conflicts/SKILL.md,.github/skills/resolve-merge-conflicts/resolve.sh
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review. Review already submitted before background grumpy-coder finished. Its findings referenced unrelated files outside PR #57601's changed lines, so they were discarded per review policy. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
|
Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. Changes are limited to skill documentation (.github/skills/resolve-merge-conflicts/SKILL.md) and shell script implementation (.github/skills/resolve-merge-conflicts/resolve.sh).
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Verdict
I did not find a changed-line bug here that justifies blocking the PR. The new marker scan closes a real failure mode, and the remaining diff is documentation plus one accidental file-mode change.
Highlights
resolve.shnow fails beforemake recompilewhen a workflow source file still contains merge markers, which matches the documented failure mode.- The regex intentionally avoids matching ordinary Markdown/YAML content that merely contains repeated punctuation later in a line.
- The only thing worth watching is the mode drift on
resolve.sh(100755 -> 100644); if this script is meant to stay directly executable from the skill docs, the execute bit should be restored before merge.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 25.7 AIC · ⌖ 7.34 AIC · ⊞ 21.8K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — the change directly addresses a real, recurring root cause with clear CI evidence. Approving with two minor observations worth a follow-up pass.
📋 Key Themes & Highlights
Key Themes
- Root cause addressed: The diff3
|||||||marker slipping pastgit diff --checkand being parsed as YAML is the actual failure mechanism, and the fix targets it precisely. - Regex correctness:
^(<{7}|\|{7}|={7}|>{7})([^=<>|]|$)correctly requires exactly 7 leading chars, avoiding 8-char sequences. False-positive risk in real workflow Markdown is very low. - Scope gap: Only
maxdepth 1under.github/workflows/is scanned — skills.mdfiles with YAML frontmatter are not covered (see inline comment).
Positive Highlights
- ✅
--verify-markersis a clean, standalone, read-only mode — easy to run ad-hoc. - ✅
|| truesuppression correctly preventsgrepnon-match exit codes from triggeringset -e. - ✅ Excellent PR description with CI run links as evidence — great debugging hygiene.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 37.8 AIC · ⌖ 18 AIC · ⊞ 7.6K
Comment /matt to run again
| echo "$matches" >&2 | ||
| fi | ||
| done < <(find .github/workflows -maxdepth 1 -name '*.md' -print0 2>/dev/null) | ||
|
|
There was a problem hiding this comment.
[/diagnosing-bugs] The scan covers only maxdepth 1 under .github/workflows/, but skill YAML frontmatter in .github/skills/**/*.md could also receive manual conflict resolution and carry the same stray-marker risk. The observed failure was in a workflow .md, so the current scope is correct for the reported bug — worth documenting why the boundary is intentional.
💡 Consider a comment explaining the scope boundary
A comment next to the find call would prevent future "why don't we also scan skills?" questions:
# Scan only workflow sources — skill .md files don't feed make recompile,
# so a stray marker there is less immediately harmful.
find .github/workflows -maxdepth 1 -name '*.md' -print0 2>/dev/null@copilot please address this.
There was a problem hiding this comment.
🔵 Needs a closer look
The scan omits imported shared workflow files, and protected automation changes require human scrutiny.
Pull request overview
Adds proactive conflict-marker detection before workflow recompilation.
Changes:
- Adds standalone and automatic marker verification.
- Documents the failure mode and usage.
File summaries
| File | Description |
|---|---|
SKILL.md |
Documents marker detection and recovery. |
resolve.sh |
Scans workflow Markdown before recompilation. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| fi | ||
|
|
||
| # A manually resolved .md source conflict can silently leave behind a | ||
| # conflict-marker line (e.g. "|||||| base (original)") that git diff --check |
| echo "error: leftover conflict marker(s) in $file:" >&2 | ||
| echo "$matches" >&2 | ||
| fi | ||
| done < <(find .github/workflows -maxdepth 1 -name '*.md' -print0 2>/dev/null) |
There was a problem hiding this comment.
The --verify-markers addition is well-implemented and the rationale in SKILL.md is clear and useful. Shell logic is correct: grep || true prevents false set -e triggers, (( hits )) is safely guarded inside if, and local matches is declared before assignment to preserve exit-code semantics. Covering all four conflict markers including the diff3 ||||||| marker is a notable improvement. No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 23.9 AIC · ⌖ 14.6 AIC · ⊞ 6.2K
|
🎉 This pull request is included in a new release. Release: |
test