feat(#58): /update skill for syncing ops fork with upstream#61
Merged
Conversation
Single-command replacement for the manual "fetch → branch → merge → push → PR" dance that fork maintainers run to pull upstream apexstack changes into their ops fork. What it does: - Pre-flight: refuses on dirty tree, non-default branch, missing upstream remote. Tells the user exactly how to fix each case. - Fetches upstream and origin. - Previews the commit delta (ahead/behind) with capped 20-line lists. - Asks merge-vs-rebase (default merge, preserves fork history). - Creates a sync branch chore/#N-sync-upstream-apexstack and runs the merge/rebase there, walking per-file conflicts with three options (keep mine / accept upstream / open editor — default to editor since auto-resolution on a governance framework is risky). - Leaves a sync branch ready to push + a pre-filled PR body listing commits pulled in. Does NOT push. Divergence from the AC: The AC says "leaves updated local main for the user to push themselves". Two hooks make literal adherence impossible: - block-main-push.sh blocks `git push <remote> main` - block-main-push.sh also blocks `git commit` on main, so a merge with conflicts (which requires a commit to finalise) cannot be completed on main Creating a sync branch sidesteps both and is the same shape the project uses for every other change. Documented in the skill's "Design notes" section. Also updates: - CLAUDE.md skills list (27 → 31, adds /update row) - docs/multi-project.md "Upgrades" section points at /update first, with the raw-commands flow below it updated to use a sync branch Closes #58
atlas-apex
commented
Apr 17, 2026
atlas-apex
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review: PR #61
Commit: ecace4c3e74328863d1c6edaf2c86945900e1563
Summary
New /update skill automating the upstream sync flow, plus aligned doc updates in CLAUDE.md and docs/multi-project.md. Marker written: .claude/session/reviews/61-rex.approved.
Checklist Results
- Architecture & Design: Pass
- Code Quality: Pass (process doc, not executable code)
- Testing: N/A (process doc for Claude)
- Security: Pass
- Performance: N/A
- PR Description & Glossary: Pass (4 terms)
- Technical Decisions (AgDR):Pass — divergence from AC #8 justified inline in "Design notes" + PR body
Findings
AC #8 divergence (sync branch vs. updated local main): Correct call. block-main-push.sh blocks both direct pushes to main and commits made while on main, making literal AC adherence mechanically impossible inside this repo. Sync-branch shape matches every other change in the project and routes the merge through the standard Rex+CEO gate. Transparently documented in the skill and PR body.
Conflict walker: Safe defaults — option (3) "open editor" is the right default since auto-resolution on a governance framework is risky. Bail-out path (--abort → checkout main → delete branch) is clean.
Process completeness: Pre-flight covers the three failure modes (no upstream, dirty tree, wrong branch). Network failure on fetch correctly exits rather than working from stale refs. Dry-run semantics (preview only, no staged-merge trap) is the right choice.
Minor (non-blocking)
- Skill count off-by-one: CLAUDE.md heading says "Available skills (31)" and the summary row says "31 slash commands", but the actual table has 29 rows pre-merge + 1 new
/update= 30 post-merge. Recommend bumping both to 30 in a follow-up. - Default-branch asymmetry: step 1c detects
$DEFAULT_BRANCHdynamically, but step 3 onward hardcodesupstream/main/main. For forks defaulting tomaster, rev-list and merge targets would be wrong. Reuse$DEFAULT_BRANCHthroughout. - Tracking-issue heuristic (step 5): "Find or create a tracking issue. If a recent 'sync' issue is open, reuse its number" — worth naming the search, e.g.
gh issue list --search "sync upstream in:title" --state open.
Verdict
APPROVED (posted as comment — cannot self-approve)
Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: ecace4c3e74328863d1c6edaf2c86945900e1563
me2resh
approved these changes
Apr 17, 2026
Adds missing blank lines around 8 fenced code blocks and 1 numbered list in .claude/skills/update/SKILL.md. Pure formatting — no prose or process changes.
atlas-apex
commented
Apr 17, 2026
atlas-apex
left a comment
Collaborator
Author
There was a problem hiding this comment.
Re-review at HEAD 2933a06 — APPROVED.
Delta from previously-approved ecace4c is 9 blank-line additions around fenced code blocks and one numbered list in .claude/skills/update/SKILL.md — pure markdownlint fix (MD031/MD032). No prose, process, or logic changes. CI green.
Prior approval carries over.
Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: 2933a06e28a1e98aee8cdef18a0dcaaa0f610b08
This was referenced Apr 17, 2026
me2resh
added a commit
that referenced
this pull request
Apr 17, 2026
Adds an explicit "Approval marker — EXACT FORMAT REQUIRED" section
to .claude/agents/code-reviewer.md with:
- The canonical write command (three allowed shapes: git rev-parse,
gh pr view --json headRefOid, printf '%s\n' "$SHA")
- CORRECT vs WRONG visual examples covering the four malformed
shapes actually observed in the wild:
- "PR: N\nSHA: …" labelled multi-line
- JSON object
- SHA with trailing timestamp
- "APPROVED at <SHA>" prose prefix
- Explicit "don't write the marker on non-approved verdicts" rule
- Explicit "report and abort, don't fake completion" rule for sandbox
failures
- Reference to the hook that validates (block-unreviewed-merge.sh)
and the rule that forbids hand-editing (pr-workflow.md)
Motivation: on 2026-04-17 (apexstack PR #61) the agent wrote
`PR: 61\nSHA: 2933a06…` instead of the bare SHA, the hook rejected
it, and the workaround required hand-editing — a rule violation per
pr-workflow.md § "Mechanical enforcement". Agent file had no
marker-format guidance at all; this closes that gap.
Also adds a BLOCKING rule (#8) at the end of the Rules list pointing
back to the new section so the format requirement is visible from
the quick-scan list.
Closes #62
atlas-apex
added a commit
that referenced
this pull request
Apr 17, 2026
Adds an explicit "Approval marker — EXACT FORMAT REQUIRED" section
to .claude/agents/code-reviewer.md with:
- The canonical write command (three allowed shapes: git rev-parse,
gh pr view --json headRefOid, printf '%s\n' "$SHA")
- CORRECT vs WRONG visual examples covering the four malformed
shapes actually observed in the wild:
- "PR: N\nSHA: …" labelled multi-line
- JSON object
- SHA with trailing timestamp
- "APPROVED at <SHA>" prose prefix
- Explicit "don't write the marker on non-approved verdicts" rule
- Explicit "report and abort, don't fake completion" rule for sandbox
failures
- Reference to the hook that validates (block-unreviewed-merge.sh)
and the rule that forbids hand-editing (pr-workflow.md)
Motivation: on 2026-04-17 (apexstack PR #61) the agent wrote
`PR: 61\nSHA: 2933a06…` instead of the bare SHA, the hook rejected
it, and the workaround required hand-editing — a rule violation per
pr-workflow.md § "Mechanical enforcement". Agent file had no
marker-format guidance at all; this closes that gap.
Also adds a BLOCKING rule (#8) at the end of the Rules list pointing
back to the new section so the format requirement is visible from
the quick-scan list.
Closes #62
Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
osama-abu-baker
pushed a commit
to osama-abu-baker/apexyard
that referenced
this pull request
Jun 3, 2026
…e2resh#61) * feat(me2resh#58): /update skill for syncing ops fork with upstream Single-command replacement for the manual "fetch → branch → merge → push → PR" dance that fork maintainers run to pull upstream apexstack changes into their ops fork. What it does: - Pre-flight: refuses on dirty tree, non-default branch, missing upstream remote. Tells the user exactly how to fix each case. - Fetches upstream and origin. - Previews the commit delta (ahead/behind) with capped 20-line lists. - Asks merge-vs-rebase (default merge, preserves fork history). - Creates a sync branch chore/#N-sync-upstream-apexstack and runs the merge/rebase there, walking per-file conflicts with three options (keep mine / accept upstream / open editor — default to editor since auto-resolution on a governance framework is risky). - Leaves a sync branch ready to push + a pre-filled PR body listing commits pulled in. Does NOT push. Divergence from the AC: The AC says "leaves updated local main for the user to push themselves". Two hooks make literal adherence impossible: - block-main-push.sh blocks `git push <remote> main` - block-main-push.sh also blocks `git commit` on main, so a merge with conflicts (which requires a commit to finalise) cannot be completed on main Creating a sync branch sidesteps both and is the same shape the project uses for every other change. Documented in the skill's "Design notes" section. Also updates: - CLAUDE.md skills list (27 → 31, adds /update row) - docs/multi-project.md "Upgrades" section points at /update first, with the raw-commands flow below it updated to use a sync branch Closes me2resh#58 * fix: markdownlint blank-lines-around-fences and lists in /update skill Adds missing blank lines around 8 fenced code blocks and 1 numbered list in .claude/skills/update/SKILL.md. Pure formatting — no prose or process changes. --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
osama-abu-baker
pushed a commit
to osama-abu-baker/apexyard
that referenced
this pull request
Jun 3, 2026
… SHA (me2resh#66) Adds an explicit "Approval marker — EXACT FORMAT REQUIRED" section to .claude/agents/code-reviewer.md with: - The canonical write command (three allowed shapes: git rev-parse, gh pr view --json headRefOid, printf '%s\n' "$SHA") - CORRECT vs WRONG visual examples covering the four malformed shapes actually observed in the wild: - "PR: N\nSHA: …" labelled multi-line - JSON object - SHA with trailing timestamp - "APPROVED at <SHA>" prose prefix - Explicit "don't write the marker on non-approved verdicts" rule - Explicit "report and abort, don't fake completion" rule for sandbox failures - Reference to the hook that validates (block-unreviewed-merge.sh) and the rule that forbids hand-editing (pr-workflow.md) Motivation: on 2026-04-17 (apexstack PR me2resh#61) the agent wrote `PR: 61\nSHA: 2933a06…` instead of the bare SHA, the hook rejected it, and the workaround required hand-editing — a rule violation per pr-workflow.md § "Mechanical enforcement". Agent file had no marker-format guidance at all; this closes that gap. Also adds a BLOCKING rule (me2resh#8) at the end of the Rules list pointing back to the new section so the format requirement is visible from the quick-scan list. Closes me2resh#62 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
mosta7il
pushed a commit
to mosta7il/apexyard
that referenced
this pull request
Jun 8, 2026
…e2resh#61) * feat(me2resh#58): /update skill for syncing ops fork with upstream Single-command replacement for the manual "fetch → branch → merge → push → PR" dance that fork maintainers run to pull upstream apexstack changes into their ops fork. What it does: - Pre-flight: refuses on dirty tree, non-default branch, missing upstream remote. Tells the user exactly how to fix each case. - Fetches upstream and origin. - Previews the commit delta (ahead/behind) with capped 20-line lists. - Asks merge-vs-rebase (default merge, preserves fork history). - Creates a sync branch chore/#N-sync-upstream-apexstack and runs the merge/rebase there, walking per-file conflicts with three options (keep mine / accept upstream / open editor — default to editor since auto-resolution on a governance framework is risky). - Leaves a sync branch ready to push + a pre-filled PR body listing commits pulled in. Does NOT push. Divergence from the AC: The AC says "leaves updated local main for the user to push themselves". Two hooks make literal adherence impossible: - block-main-push.sh blocks `git push <remote> main` - block-main-push.sh also blocks `git commit` on main, so a merge with conflicts (which requires a commit to finalise) cannot be completed on main Creating a sync branch sidesteps both and is the same shape the project uses for every other change. Documented in the skill's "Design notes" section. Also updates: - CLAUDE.md skills list (27 → 31, adds /update row) - docs/multi-project.md "Upgrades" section points at /update first, with the raw-commands flow below it updated to use a sync branch Closes me2resh#58 * fix: markdownlint blank-lines-around-fences and lists in /update skill Adds missing blank lines around 8 fenced code blocks and 1 numbered list in .claude/skills/update/SKILL.md. Pure formatting — no prose or process changes. --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
mosta7il
pushed a commit
to mosta7il/apexyard
that referenced
this pull request
Jun 8, 2026
… SHA (me2resh#66) Adds an explicit "Approval marker — EXACT FORMAT REQUIRED" section to .claude/agents/code-reviewer.md with: - The canonical write command (three allowed shapes: git rev-parse, gh pr view --json headRefOid, printf '%s\n' "$SHA") - CORRECT vs WRONG visual examples covering the four malformed shapes actually observed in the wild: - "PR: N\nSHA: …" labelled multi-line - JSON object - SHA with trailing timestamp - "APPROVED at <SHA>" prose prefix - Explicit "don't write the marker on non-approved verdicts" rule - Explicit "report and abort, don't fake completion" rule for sandbox failures - Reference to the hook that validates (block-unreviewed-merge.sh) and the rule that forbids hand-editing (pr-workflow.md) Motivation: on 2026-04-17 (apexstack PR me2resh#61) the agent wrote `PR: 61\nSHA: d46ff2c…` instead of the bare SHA, the hook rejected it, and the workaround required hand-editing — a rule violation per pr-workflow.md § "Mechanical enforcement". Agent file had no marker-format guidance at all; this closes that gap. Also adds a BLOCKING rule (me2resh#8) at the end of the Rules list pointing back to the new section so the format requirement is visible from the quick-scan list. Closes me2resh#62 Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New
/updateskill automates the fork-sync flow. Single skill invocation takes an ops-fork user from "behind upstream" to "sync branch ready to push as PR" — replacing the manualgit fetch→ branch → merge → push → PR dance..claude/skills/update/SKILL.md— full process: pre-flight, preview, merge-or-rebase, conflict walker, next-steps printout/updaterow)/updatefirst, with the raw-command flow below it corrected to use a sync branchShips PRD-0002 change #1 (fork-distribution PRD opened in ops fork earlier today).
Divergence from the AC
AC #8 says "leaves updated local main for the user to push themselves." Two hooks in this repo make literal adherence impossible:
block-main-push.shblocksgit push <remote> mainblock-main-push.shalso blocksgit commitwhile on main — so a merge with conflicts cannot be completed on mainThe skill instead leaves a sync branch ready to push. Same shape as every other change in this project. Documented in the skill's "Design notes" section. Open to changing this if the maintainer prefers the literal AC path.
Testing
Glossary
Related