Skip to content

fix(#62): tighten code-reviewer marker-writing to enforce bare SHA#66

Merged
atlas-apex merged 1 commit into
mainfrom
fix/#62-rex-marker-format
Apr 17, 2026
Merged

fix(#62): tighten code-reviewer marker-writing to enforce bare SHA#66
atlas-apex merged 1 commit into
mainfrom
fix/#62-rex-marker-format

Conversation

@atlas-apex

@atlas-apex atlas-apex commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds an explicit "Approval marker — EXACT FORMAT REQUIRED" section to .claude/agents/code-reviewer.md. The agent file had no marker-format guidance at all — which is how apexstack#61 produced a malformed PR: 61\nSHA: … marker that the merge gate rejected and forced a rule-violating hand-edit to resolve.

What changed

.claude/agents/code-reviewer.md gains:

  • Canonical write commands (3 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:
  • "Don't write on non-approved verdicts" — the marker is a signal, not a log line
  • "Report and abort, don't fake completion" for sandbox failures
  • Rule refactor(#100): multi-project only + fork-first install #8 at the bottom of the Rules list cross-referencing the new section, so the format requirement is visible in the quick-scan view

Testing

Doc-only change to the agent definition. No functional code; no unit tests.

Validation comes from the next Rex invocation on a real PR — the marker format is verified with od -c .claude/session/reviews/<PR>-rex.approved | head -2, which should show 40 hex chars + \n + EOF.

Glossary

Term Definition
approval marker File at .claude/session/reviews/<PR>-rex.approved whose existence + correct SHA content satisfies block-unreviewed-merge.sh for the code-review half of the merge gate
Rex The code-reviewer sub-agent
merge gate The collective PreToolUse hooks that block gh pr merge until all required markers exist and all required signals are green
whitespace-strip comparison The hook reads the marker, strips whitespace, and checks the result equals the PR's HEAD SHA; any non-SHA content fails this

Related

@me2resh
fix(#62): tighten code-reviewer marker-writing to enforce bare SHA
256a1ab 
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
atlas-apex commented Apr 17, 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 #66

Commit: 256a1ab3324ab23502e13b1ea1b56fd798e71dab

Summary

Doc-only change to .claude/agents/code-reviewer.md adding an "Approval marker — EXACT FORMAT REQUIRED" section. Closes the gap that produced the apexstack#61 malformed-marker incident.

Checklist Results

  • Architecture & Design: N/A (doc-only)
  • Code Quality: Pass
  • Testing: N/A (doc-only; validated by next Rex invocation)
  • Security: Pass
  • Performance: N/A
  • PR Description & Glossary: Pass (4 terms)
  • Technical Decisions (AgDR):N/A (no new libs/patterns; formalizes existing hook contract)

Findings against review focus

  1. Malformed shapes covered — Pass. All four observed failure modes are explicitly shown as WRONG: labelled PR:N\nSHA:, JSON, SHA+timestamp, APPROVED at <SHA> prose prefix.
  2. Write commands correct — Pass. Options A (git rev-parse HEAD > …), B (gh pr view --json headRefOid --jq .headRefOid > …), and C (printf '%s\n' "$SHA" > …) all produce exactly 40 hex + \n. Redirect semantics and newline handling are right.
  3. Non-approved rule — Pass. "On REQUEST CHANGES or COMMENT verdicts … Do NOT write the marker" with the rationale that the marker is a signal, not a log line.
  4. Sandbox-failure rule — Pass. "Report the failure in plain text with the exact command … Do NOT describe the approval as complete when the marker isn't in place." Matches the PR#65 delegation pattern.
  5. Glossary — Pass. 4 terms: approval marker, Rex, merge gate, whitespace-strip comparison.

Suggestions (non-blocking)

  • Consider adding a one-line note that echo "$SHA" also works (it's equivalent to printf '%s\n' on POSIX), in case reviewers reach for it reflexively.
  • The od -c … | head -2 verification tip is excellent — might also call out wc -c should return 41 as a quicker sanity check.

Marker write

Sandbox in this review session blocked the write. Not faking the marker (that is exactly what this PR forbids). Parent must run one of:

git rev-parse HEAD > .claude/session/reviews/66-rex.approved
gh pr view 66 --json headRefOid --jq .headRefOid > .claude/session/reviews/66-rex.approved
printf '%s\n' 256a1ab3324ab23502e13b1ea1b56fd798e71dab > .claude/session/reviews/66-rex.approved

Then verify: od -c .claude/session/reviews/66-rex.approved | head -2 → second line should start with \n then *.

Verdict

APPROVED (pending parent-side marker write)

Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: 256a1ab3324ab23502e13b1ea1b56fd798e71dab

@atlas-apex atlas-apex merged commit d0d1fb8 into main Apr 17, 2026
3 checks passed
@atlas-apex atlas-apex deleted the fix/#62-rex-marker-format branch April 17, 2026 08:57
me2resh added a commit that referenced this pull request Apr 18, 2026
@me2resh
chore(#75): release v0.3.0 — multi-project + C4 + migrations + site r…
3f4a6b4 
…efresh

Cuts the v0.3.0 changelog and updates site version refs.

Themes (17 commits since v0.2.0):

- **Multi-project comes alive** — per-project active-ticket markers
  (#41), /update skill for fork sync (#58), SessionStart drift
  banner (#63).
- **C4 architecture diagrams** — Mermaid L1+L2 templates (#50),
  /handover seeds a stub L2 (#67).
- **Migration gate** — require-migration-ticket.sh (#59),
  /migration skill, agdr-migration template, workflow gate 3a.
- **Site refresh** — whole-framework positioning (#73).
- **Hook robustness** — gh api merge bypass closed (#47),
  absolute-path exemptions (#56), Rex marker format enforcement
  (#62/#66), gh-pr-view fallback for HEAD resolution.

Site index.html: all four v0.x references bumped (titlebar pill,
hero pill, hero release link, footer line) — three were stale at
v0.1 from the v0.2 release; consolidated all four to v0.3 here.

Refs #75
@atlas-apex atlas-apex mentioned this pull request Apr 18, 2026
Merged
atlas-apex added a commit that referenced this pull request Apr 18, 2026
@atlas-apex @me2resh
chore(#75): release v0.3.0 — multi-project + C4 + migrations + site r…
d155e96 
…efresh (#76)

Cuts the v0.3.0 changelog and updates site version refs.

Themes (17 commits since v0.2.0):

- **Multi-project comes alive** — per-project active-ticket markers
  (#41), /update skill for fork sync (#58), SessionStart drift
  banner (#63).
- **C4 architecture diagrams** — Mermaid L1+L2 templates (#50),
  /handover seeds a stub L2 (#67).
- **Migration gate** — require-migration-ticket.sh (#59),
  /migration skill, agdr-migration template, workflow gate 3a.
- **Site refresh** — whole-framework positioning (#73).
- **Hook robustness** — gh api merge bypass closed (#47),
  absolute-path exemptions (#56), Rex marker format enforcement
  (#62/#66), gh-pr-view fallback for HEAD resolution.

Site index.html: all four v0.x references bumped (titlebar pill,
hero pill, hero release link, footer line) — three were stale at
v0.1 from the v0.2 release; consolidated all four to v0.3 here.

Refs #75

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
@atlas-apex @me2resh
fix(me2resh#62): tighten code-reviewer marker-writing to enforce bare…
cd10b54 
… 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>
osama-abu-baker pushed a commit to osama-abu-baker/apexyard that referenced this pull request Jun 3, 2026
@atlas-apex @me2resh
chore(me2resh#75): release v0.3.0 — multi-project + C4 + migrations +…
b0ddeae 
… site refresh (me2resh#76)

Cuts the v0.3.0 changelog and updates site version refs.

Themes (17 commits since v0.2.0):

- **Multi-project comes alive** — per-project active-ticket markers
  (me2resh#41), /update skill for fork sync (me2resh#58), SessionStart drift
  banner (me2resh#63).
- **C4 architecture diagrams** — Mermaid L1+L2 templates (me2resh#50),
  /handover seeds a stub L2 (me2resh#67).
- **Migration gate** — require-migration-ticket.sh (me2resh#59),
  /migration skill, agdr-migration template, workflow gate 3a.
- **Site refresh** — whole-framework positioning (me2resh#73).
- **Hook robustness** — gh api merge bypass closed (me2resh#47),
  absolute-path exemptions (me2resh#56), Rex marker format enforcement
  (me2resh#62/me2resh#66), gh-pr-view fallback for HEAD resolution.

Site index.html: all four v0.x references bumped (titlebar pill,
hero pill, hero release link, footer line) — three were stale at
v0.1 from the v0.2 release; consolidated all four to v0.3 here.

Refs me2resh#75

Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
mosta7il pushed a commit to mosta7il/apexyard that referenced this pull request Jun 8, 2026
@atlas-apex @me2resh
fix(me2resh#62): tighten code-reviewer marker-writing to enforce bare…
0d2a6a2 
… 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>
mosta7il pushed a commit to mosta7il/apexyard that referenced this pull request Jun 8, 2026
@atlas-apex @me2resh
chore(me2resh#75): release v0.3.0 — multi-project + C4 + migrations +…
92c667e 
… site refresh (me2resh#76)

Cuts the v0.3.0 changelog and updates site version refs.

Themes (17 commits since v0.2.0):

- **Multi-project comes alive** — per-project active-ticket markers
  (me2resh#41), /update skill for fork sync (me2resh#58), SessionStart drift
  banner (me2resh#63).
- **C4 architecture diagrams** — Mermaid L1+L2 templates (me2resh#50),
  /handover seeds a stub L2 (me2resh#67).
- **Migration gate** — require-migration-ticket.sh (me2resh#59),
  /migration skill, agdr-migration template, workflow gate 3a.
- **Site refresh** — whole-framework positioning (me2resh#73).
- **Hook robustness** — gh api merge bypass closed (me2resh#47),
  absolute-path exemptions (me2resh#56), Rex marker format enforcement
  (me2resh#62/me2resh#66), gh-pr-view fallback for HEAD resolution.

Site index.html: all four v0.x references bumped (titlebar pill,
hero pill, hero release link, footer line) — three were stale at
v0.1 from the v0.2 release; consolidated all four to v0.3 here.

Refs me2resh#75

Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@me2resh me2resh me2resh approved these changes

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.

[Bug] code-reviewer agent writes malformed rex-approval marker (contains 'PR: N\nSHA: ...' instead of bare SHA)

2 participants

@atlas-apex @me2resh