Skip to content

fix(#559): resolve review-marker home pin-first in Rex + /approve-merge#564

Merged
atlas-apex merged 1 commit into
devfrom
fix/GH-559-marker-pin-first
Jun 7, 2026
Merged

fix(#559): resolve review-marker home pin-first in Rex + /approve-merge#564
atlas-apex merged 1 commit into
devfrom
fix/GH-559-marker-pin-first

Conversation

@atlas-apex

@atlas-apex atlas-apex commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Testing

  1. Ran the new pin-first block from inside a workspace/<project>/ clone (the failure scenario): resolves MARKER_HOME to the real ops fork, _lib-review-markers.sh is present, and review_marker_path yields the repo-qualified <owner>__<repo>__<pr>-rex.approved the gate expects. Pre-fix, the same cwd resolved to the portfolio sibling + bare name.
  2. markdownlint-cli2 on both files → 0 errors.

Refs #559

Glossary

Term Definition
ops fork The framework fork clone holding .claude/ hooks + the .apexyard-fork pin anchor.
session pin ~/.claude/apexyard/ops-root-<session> written at SessionStart; points at the real ops fork regardless of cwd.
pin-first resolution Consult the session pin before walking the filesystem — the strategy resolve_ops_root already uses; this PR brings the marker writers in line.
review marker .claude/session/reviews/<owner>__<repo>__<pr>-<role>.approved recording a Rex/CEO approval at a HEAD SHA.
split-portfolio mode Public framework fork + private portfolio repo (registry/onboarding live in the sibling), which is what breaks the plain walk-up.

@me2resh
fix(#559): resolve review-marker home pin-first in Rex + /approve-merge
ebf96d5 
The merge gate resolves the marker dir via _lib-ops-root.sh::resolve_ops_root
(pin-first), but the code-reviewer agent and /approve-merge skill resolved it
with a plain walk-up. In split-portfolio mode that walk-up lands on the private
portfolio sibling (it has onboarding.yaml + apexyard.projects.yaml) where
_lib-review-markers.sh doesn't exist — so markers were written to the wrong dir
under a bare (unqualified) name and the gate couldn't see them, blocking every
managed-project merge until hand-relocated.

- code-reviewer.md + approve-merge/SKILL.md now read the session pin first
  (~/.claude/apexyard/ops-root-<session>), validate + self-heal a stale pin,
  and fall back to walk-up only when no valid pin exists — matching the gate.
- Verified from a workspace clone: resolves MARKER_HOME to the real ops fork
  and produces the repo-qualified marker path the gate expects.

Follow-up to #230 (which fixed the gate's read path; this fixes the writers).

Refs #559
atlas-apex
atlas-apex commented Jun 7, 2026
View reviewed changes

@atlas-apex atlas-apex left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code Review: PR #564

Commit: ebf96d56fe761148e229f0da55d86f185e1604ff

Summary

Switches the review-marker home resolution in the code-reviewer agent (Rex) and the /approve-merge skill from a plain filesystem walk-up to a pin-first strategy that mirrors the merge gate's _lib-ops-root.sh::resolve_ops_root. This is the write-side companion to #230 (which fixed only the gate's read path). The fix matters in split-portfolio mode: the ops fork carries only .apexyard-fork while onboarding.yaml+apexyard.projects.yaml live in the private portfolio sibling — so a walk-up from a workspace/<project>/ clone climbs to the sibling (legacy anchor satisfied, but no _lib-review-markers.sh), and markers land where the gate can't see them. Both writers now read the session pin first and self-heal a stale pin before falling back to walk-up, so writer and reader always agree.

Checklist Results

  • Architecture & Design: Pass — both writers now use the gate's strategy; single source of truth preserved (resolve_ops_root remains canonical, the inline blocks are faithful copies because agent/skill markdown can't source a lib at instruction-read time).
  • Code Quality: Pass — IFS= read -r, defensive 2>/dev/null || pwd, escape-hatch parity (APEXYARD_OPS_DISABLE_PIN, APEXYARD_OPS_PIN_DIR, unset session id).
  • Testing: Pass — verified by differential execution against the real lib (see below).
  • Security: Pass — no secrets, no leaks; portfolio sibling not named with private project identifiers.
  • Performance: N/A — doc-only instruction change.
  • PR Description & Glossary: Pass — Glossary present (6 terms), Summary bullets narrative, Refs #559.
  • Summary Bullet Narrative: Pass.
  • Technical Decisions (AgDR): N/A — implements an already-recorded decision (AgDR-0060 + #381/#230 strategy); no new technical choice introduced.
  • Adopter Handbooks: N/A — no handbooks loaded (diff touches only .md instruction files; no architecture//general/ handbooks present, no language-extension match).

Verification performed

I ran the PR's inline block as a real shell test and diffed its output against the canonical resolve_ops_root from _lib-ops-root.sh across the scenarios that matter:

Scenario Inline block Canonical resolve_ops_root Match
Valid pin, cwd = ops fork ops fork ops fork yes
Valid pin, cwd = /tmp (Rex's real failure scenario) ops fork ops fork yes
APEXYARD_OPS_DISABLE_PIN=1 (walk-up fallback) ops fork ops fork yes
CLAUDE_CODE_SESSION_ID unset (walk-up fallback) ops fork ops fork yes
Stale pin → self-heal to walk-up ops fork ops fork yes

I also confirmed the new validation predicate is an exact De Morgan negation of _ops_root_anchor_valid across all four anchor combinations:

Anchor state at pinned dir Inline predicate _ops_root_anchor_valid
.apexyard-fork only KEEP KEEP
legacy pair only (onboarding.yaml+apexyard.projects.yaml) KEEP KEEP
onboarding.yaml only (incomplete pair) DROP DROP
nothing DROP DROP

Confirmed the live environment reproduces the bug substrate: the ops fork has .apexyard-fork but NOT the legacy pair; the portfolio sibling (../apexyard-portfolio) has the legacy pair but NOT _lib-review-markers.sh. The pin for this session correctly points at the real ops fork. markdownlint-cli2 → 0 errors on both files. Secrets scan clean.

Self-test of the fix end-to-end: writing THIS review's marker resolved MARKER_HOME to the real ops fork and produced the repo-qualified me2resh__apexyard__564-rex.approved the gate expects.

Issues Found

None.

Suggestions

  • nit (non-blocking): the inline blocks duplicate resolve_ops_root's logic by necessity (markdown instructions can't source the lib at read time). The two inline copies and the lib are now three places that must stay in lock-step. Consider adding a tiny CI check that asserts the inline blocks remain behaviourally equivalent to resolve_ops_root (e.g. extract+exec under fixtures) so a future edit to the lib doesn't silently drift the instructions. Not blocking — the current copies are faithful.

Verdict

APPROVED

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: ebf96d56fe761148e229f0da55d86f185e1604ff

atlas-apex
atlas-apex commented Jun 7, 2026
View reviewed changes

@atlas-apex atlas-apex left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Code Review: PR #564

Commit: ebf96d56fe761148e229f0da55d86f185e1604ff

Summary

Switches the review-marker home resolution in the code-reviewer agent (Rex) and the /approve-merge skill from a plain filesystem walk-up to a pin-first strategy that mirrors the merge gate's _lib-ops-root.sh::resolve_ops_root. This is the write-side companion to #230 (which fixed only the gate's read path). The fix matters in split-portfolio mode: the ops fork carries only .apexyard-fork while onboarding.yaml+apexyard.projects.yaml live in the private portfolio sibling — so a walk-up from a workspace/<project>/ clone climbs to the sibling (legacy anchor satisfied, but no _lib-review-markers.sh), and markers land where the gate can't see them. Both writers now read the session pin first and self-heal a stale pin before falling back to walk-up, so writer and reader always agree.

Checklist Results

  • Architecture & Design: Pass — both writers now use the gate's strategy; single source of truth preserved (resolve_ops_root remains canonical, the inline blocks are faithful copies because agent/skill markdown can't source a lib at instruction-read time).
  • Code Quality: Pass — IFS= read -r, defensive 2>/dev/null || pwd, escape-hatch parity (APEXYARD_OPS_DISABLE_PIN, APEXYARD_OPS_PIN_DIR, unset session id).
  • Testing: Pass — verified by differential execution against the real lib (see below).
  • Security: Pass — no secrets, no leaks; portfolio sibling not named with private project identifiers.
  • Performance: N/A — doc-only instruction change.
  • PR Description & Glossary: Pass — Glossary present (6 terms), Summary bullets narrative, Refs #559.
  • Summary Bullet Narrative: Pass.
  • Technical Decisions (AgDR): N/A — implements an already-recorded decision (AgDR-0060 + #381/#230 strategy); no new technical choice introduced.
  • Adopter Handbooks: N/A — no handbooks loaded (diff touches only .md instruction files; no architecture//general/ handbooks present, no language-extension match).

Verification performed

I ran the PR's inline block as a real shell test and diffed its output against the canonical resolve_ops_root from _lib-ops-root.sh across the scenarios that matter:

Scenario Inline block Canonical resolve_ops_root Match
Valid pin, cwd = ops fork ops fork ops fork yes
Valid pin, cwd = /tmp (Rex's real failure scenario) ops fork ops fork yes
APEXYARD_OPS_DISABLE_PIN=1 (walk-up fallback) ops fork ops fork yes
CLAUDE_CODE_SESSION_ID unset (walk-up fallback) ops fork ops fork yes
Stale pin → self-heal to walk-up ops fork ops fork yes

I also confirmed the new validation predicate is an exact De Morgan negation of _ops_root_anchor_valid across all four anchor combinations:

Anchor state at pinned dir Inline predicate _ops_root_anchor_valid
.apexyard-fork only KEEP KEEP
legacy pair only (onboarding.yaml+apexyard.projects.yaml) KEEP KEEP
onboarding.yaml only (incomplete pair) DROP DROP
nothing DROP DROP

Confirmed the live environment reproduces the bug substrate: the ops fork has .apexyard-fork but NOT the legacy pair; the portfolio sibling (../apexyard-portfolio) has the legacy pair but NOT _lib-review-markers.sh. The pin for this session correctly points at the real ops fork. markdownlint-cli2 → 0 errors on both files. Secrets scan clean.

Self-test of the fix end-to-end: writing THIS review's marker resolved MARKER_HOME to the real ops fork and produced the repo-qualified me2resh__apexyard__564-rex.approved the gate expects.

Issues Found

None.

Suggestions

  • nit (non-blocking): the inline blocks duplicate resolve_ops_root's logic by necessity (markdown instructions can't source the lib at read time). The two inline copies and the lib are now three places that must stay in lock-step. Consider adding a tiny CI check that asserts the inline blocks remain behaviourally equivalent to resolve_ops_root (e.g. extract+exec under fixtures) so a future edit to the lib doesn't silently drift the instructions. Not blocking — the current copies are faithful.

Verdict

APPROVED

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: ebf96d56fe761148e229f0da55d86f185e1604ff

@atlas-apex atlas-apex merged commit 50b54a5 into dev Jun 7, 2026
8 checks passed
@atlas-apex atlas-apex deleted the fix/GH-559-marker-pin-first branch June 7, 2026 13:47
@atlas-apex atlas-apex mentioned this pull request Jun 7, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

github-issue has-ticket

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@atlas-apex @me2resh