Design Review: PR #504 — AgDR-0062 (Rex marker authenticity)

Commit: 6b31c7796aacda98eb24842785f5048c4141135c

Summary

The AgDR records a three-layer fix for build-class sub-agents fabricating *-rex.approved markers: a prompt guardrail in each build-agent file, an advisory pre-write hook, and a hardened merge gate that requires a real posted GitHub review at the PR HEAD before honouring the rex marker. The authenticity anchor — "the review must exist in GitHub's audit trail, not just as a local file" — is the right shape. But the design as written does not actually close the hole it targets.

Review Lens Results

• Quality attributes / NFRs: Pass — resilience (graceful-degrade) and auditability (GitHub trail) are the stated NFRs and both are addressed.
• Design patterns & structure: Pass — defence-in-depth (prompt -> advisory -> hard gate) mirrors the established check-secrets/check-upstream-drift advisory-hook pattern.
• Technical debt: Pass — no silent debt; the degrade weakening is explicit.
• Decisions (AgDR linkage): Pass — this PR is the AgDR; the options table is genuine.
• Risk: Concern — see Blocking Findings; the residual bypass is not in the risk picture.
• Trade-off analysis: Pass — four options weighed honestly, including why prompt-only is insufficient.
• Requirements traceability: Fail — the design does not trace back to the load-bearing requirement (#494: the review must be by someone other than the author).
• Migration safety: N/A.
• Adopter Handbooks: N/A — framework-internal hook/agent change; no adopter handbooks apply.

Blocking Findings

B1 — The gate verifies that a review exists, not that an independent review exists. This is the crux of whether the fix works, and the AgDR is silent on it.

The whole point of #494 is that a build agent reviews its own work. The load-bearing property of any fix is therefore author-independence: the review at HEAD must come from an identity other than the PR author. The AgDR never states this as a checked property — and the implementation confirms the omission. block-unreviewed-merge.sh lines 239–241:

jq -r --arg sha "$CURRENT_SHA" '[.[] | select(.commit.oid == $sha)] | length'

There is no author predicate. Any review at HEAD by any account — including the PR author's own — satisfies the gate.

Worse, the AgDR's "COMMENT-type review edge case" (lines 37–39) does not just omit independence, it waives it: it explicitly accepts that "the code-reviewer agent owns the same GitHub account as the PR author" and posts a self-COMMENTED review, then concludes "the gate accepts any review state by any reviewer." So a build agent that runs gh pr review <pr> --comment on its own PR at HEAD — exactly the self-review behaviour #494 observed these agents already performing in prose — now produces a real GitHub review object at HEAD and passes the new gate. The fix raises the bar from "write a local file" to "post one comment on your own PR," which the offending agents are already fully capable of doing.

To clear B1, the AgDR must either:

• (a) State author-independence as an explicit checked property and have the gate filter on it — e.g. select(.commit.oid == $sha and .author.login != <pr-author-login>) — accepting that this requires resolving the PR author and that the same-account / self-COMMENTED rationale then no longer holds; or
• (b) If a distinct reviewer identity genuinely cannot be guaranteed in the single-account model, the AgDR must honestly record that the gate does not establish independence, name the residual bypass (author self-posts a comment-review at HEAD), and justify why the weaker "a human-visible review object exists" bar is the chosen ceiling — so the limitation is a recorded decision, not an unstated gap.

Either is acceptable as a decision. What is not acceptable is the current state: a design that presents itself as closing the self-review hole while the checked invariant ("a review exists") is satisfiable by the author. The decision is incomplete without resolving this.

Suggestions

• S1 (advisory) — Once B1 is resolved via (b), name the adversarial angle on the graceful-degrade explicitly: an agent that can induce a gh failure (or runs in a gh-less context) drops the gate to SHA-only, which the fabricated local marker already passes. Lines 41–49 honestly flag the degrade as "weaker," but frame it as an infra-outage concern rather than a deliberate-bypass vector. One sentence naming the attacker-forces-gh-failure path would complete the honesty.
• S2 (nit) — Artifacts cite "PR: #495" (line 54) but this is PR #504. Reconcile the AgDR's self-reference before merge.

Verdict

CHANGES REQUESTED

The authenticity anchor is sound and the layered structure is good. But the design does not address the author-independence requirement that determines whether the fix actually works — the gate as designed and implemented is satisfiable by the very actor it targets (a build agent self-posting a comment-review at HEAD), and the AgDR's COMMENT-type rationale waives independence without acknowledging the consequence. Resolve B1 (make independence a checked property, or record honestly that it is not and why), then re-request review.

Reviewed by Tariq (Solution Architect)
Reviewed commit: 6b31c7796aacda98eb24842785f5048c4141135c

Code Review: PR #504

Commit: 6b31c7796aacda98eb24842785f5048c4141135c
Author: atlas-apex · Base: dev · Ticket #494

Summary

Three-layer fix to stop build-class sub-agents from self-reviewing: (1) prompt guardrails in 7 build-agent files, (2) a canonical "Build agents cannot self-review" rule in pr-workflow.md + advisory warn-review-marker-write.sh, (3) a mechanical addition to block-unreviewed-merge.sh that requires a "real GitHub review at the PR HEAD" behind the *-rex.approved marker. AgDR-0062 records the rationale.

The intent is right and Layers 1–2 are solid. Layer 3 — the load-bearing mechanical gate — does not close the hole it targets. Verdict: CHANGES REQUESTED.

Checklist Results

• Architecture & Design: Pass
• Code Quality: Pass
• Testing: Fail (tests assert the wrong invariant — see Issue 1)
• Security: Fail (gate is bypassable by the exact actor it targets — Issue 1)
• Performance: Pass (one extra gh pr view per merge; acceptable)
• PR Description & Glossary: Pass
• Summary Bullet Narrative: Pass (narrative + rationale)
• Technical Decisions (AgDR): Pass (AgDR-0062 present; Gate 3b/Tariq handled separately)
• Adopter Handbooks: N/A (no handbooks loaded for this diff)

Issues Found

⛔ Issue 1 (BLOCKING) — The gate accepts the PR author's own review, so the build agent bypasses it trivially.

block-unreviewed-merge.sh:239-241 is the entire real-review check:

_REVIEW_AT_HEAD=$(echo "$_REVIEWS_JSON" | \
  jq -r --arg sha "$CURRENT_SHA" \
    '[.[] | select(.commit.oid == $sha)] | length')

It accepts any review at HEAD by any author in any state. There is no select(.author.login != <pr-author>) filter anywhere in the hook (grep -n "author\|login" block-unreviewed-merge.sh → only the approved_by=user CEO-marker strings; zero references to review author).

Threat model this PR exists to defeat (verbatim from the AgDR-0062 context and the new pr-workflow.md rule): a build sub-agent runs as the PR author's GitHub account, cannot nest the Agent tool, so cannot spawn the real Rex. With this gate in place that agent simply:

1. gh pr review 504 --comment --body "..." → posts a COMMENTED review as atlas-apex at HEAD (GitHub blocks self-approval but NOT self-comment-reviews — the AgDR relies on this exact fact for Rex's own-PR case),
2. writes <repo>__504-rex.approved with the HEAD SHA,
3. the gate finds a review at HEAD, length ≥ 1 → passes.

The check moved the bar from "a marker file with the right name exists" to "a GitHub review by anyone (incl. the author) at HEAD exists." Both are satisfiable by the author alone. The AgDR (§ "The COMMENT-type review edge case") consciously chose to accept any-author/any-state reviews to accommodate Rex posting a COMMENT on its own PR — but that accommodation is precisely the bypass: it makes the author's own self-review indistinguishable from an independent one. The new rule's own words apply to the new gate: it "satisfies the filename of the gate requirement without satisfying its intent."

Required fix: the gate must require the review author to be independent of the PR author. Concretely: fetch the PR author (gh pr view <N> --json author -q .author.login) and require at least one review at HEAD whose .author.login differs from it:

PR_AUTHOR=$(gh pr view "$PR_NUMBER" ${CMD_REPO:+--repo "$CMD_REPO"} --json author -q '.author.login' 2>/dev/null)
_INDEP_REVIEW_AT_HEAD=$(echo "$_REVIEWS_JSON" | jq -r --arg sha "$CURRENT_SHA" --arg author "$PR_AUTHOR" \
  '[.[] | select(.commit.oid == $sha and .author.login != $author)] | length')

If an independent reviewer account isn't operationally available (Rex shares the author account in some setups), this needs a different authenticity signal than "a GitHub review exists" — because a self-review is not evidence of an independent pass. Either way, "any review at HEAD" cannot be the predicate. This is the single decisive finding; everything below is secondary.

Issue 2 (BLOCKING, same root) — the test suite asserts the wrong invariant and therefore can't catch the regression.

Every gh shim in test_block_unreviewed_merge.sh returns reviews JSON with no author field at all (e.g. line 72, 396, 521, 603): [{"state":"COMMENTED","commit":{"oid":"$FIXED_SHA"}}]. Case G1 ("real GitHub review at HEAD → passes") passes an authorless review and asserts the merge is allowed — which is exactly the self-review-shaped payload that should be blocked. There is no test where the only review at HEAD is authored by the PR author and the gate blocks. The "33 PASS" headline is real but the suite encodes the bug as expected behaviour. After the fix, add: (a) review at HEAD authored by PR-author-only → BLOCK; (b) review at HEAD by an independent login → PASS.

Things that ARE correct (verified, not blocking)

• Crux #3 — empty-reviews does NOT silently degrade. gh ... -q '.reviews' on a PR with zero reviews emits [] (a non-empty string), so it skips the [ -z "$_REVIEWS_JSON" ] degrade branch (line 228) and flows to jq length == 0 → BLOCK (line 247). Confirmed jq behaviour and test G2. The degrade branch correctly triggers only on genuine gh failure (empty stdout from non-zero exit) and on jq parse failure (line 243), both WARNed to stderr. Good.
• Crux #4 — SHA matching. The review must match CURRENT_SHA (the PR HEAD, resolved via resolve_pr_head, fallback local HEAD with WARN) via select(.commit.oid == $sha), and the marker SHA is independently matched at line 196. Stale-commit reviews (test G3) correctly block. Good.
• Crux #5 — Layers 1+2. Guardrail text present in all 7 build-agent files (## You cannot self-review, MUST NOT write .claude/session/reviews/); the ## Build agents cannot self-review rule is in pr-workflow.md; warn-review-marker-write.sh is exit-0 advisory and wired to BOTH Write and Bash PreToolUse in settings.json (valid JSON confirmed). These layers are good — but they are self-discipline + advisory, so they cannot substitute for the broken mechanical gate.
• Crux #6 — tests + shellcheck. 33/33 pass; the 25 pre-existing cases (markers, structured CEO, compound-command #426, sync-squash #459, cross-repo #485) are intact and the gh-shim additions didn't neuter them. shellcheck clean on both hooks (only SC1091 info on dynamic source, pre-existing repo-wide).
• No forged *504-rex.approved marker exists in either the worktree or the ops fork reviews/ dir. Nothing to remove.

Suggestions (non-blocking)

• The AgDR's own "Cons" cell for the chosen option already flags "GitHub blocks formal self-approval, so Rex posts a COMMENT-type review" — consider adding an explicit "Consequences" line acknowledging that accepting any-author reviews is a deliberate weakening, with the independence requirement as the mitigation, once Issue 1 is addressed.

Verdict

CHANGES REQUESTED — Per the review brief, crux #1 (author-independence) is decisive: the gate accepts author self-reviews, so it is theater against the precise actor (a build sub-agent posting a COMMENT review as the PR author) the PR targets. Layers 1–2 are good and the SHA/degrade logic (cruxes #3–#4) is correct, but the load-bearing mechanical backstop must reject self-authored reviews before this can ship. No rex approval marker written.

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

Code Review: PR #504

Commit: 6b31c7796aacda98eb24842785f5048c4141135c
Author: atlas-apex · Base: dev · Ticket #494

Summary

Three-layer fix to stop build-class sub-agents from self-reviewing: (1) prompt guardrails in 7 build-agent files, (2) a canonical "Build agents cannot self-review" rule in pr-workflow.md + advisory warn-review-marker-write.sh, (3) a mechanical addition to block-unreviewed-merge.sh that requires a "real GitHub review at the PR HEAD" behind the *-rex.approved marker. AgDR-0062 records the rationale.

The intent is right and Layers 1–2 are solid. Layer 3 — the load-bearing mechanical gate — does not close the hole it targets. Verdict: CHANGES REQUESTED.

Checklist Results

• Architecture & Design: Pass
• Code Quality: Pass
• Testing: Fail (tests assert the wrong invariant — see Issue 1)
• Security: Fail (gate is bypassable by the exact actor it targets — Issue 1)
• Performance: Pass (one extra gh pr view per merge; acceptable)
• PR Description & Glossary: Pass
• Summary Bullet Narrative: Pass (narrative + rationale)
• Technical Decisions (AgDR): Pass (AgDR-0062 present; Gate 3b/Tariq handled separately)
• Adopter Handbooks: N/A (no handbooks loaded for this diff)

Issues Found

⛔ Issue 1 (BLOCKING) — The gate accepts the PR author's own review, so the build agent bypasses it trivially.

block-unreviewed-merge.sh:239-241 is the entire real-review check:

_REVIEW_AT_HEAD=$(echo "$_REVIEWS_JSON" | \
  jq -r --arg sha "$CURRENT_SHA" \
    '[.[] | select(.commit.oid == $sha)] | length')

It accepts any review at HEAD by any author in any state. There is no select(.author.login != <pr-author>) filter anywhere in the hook (grep -n "author\|login" block-unreviewed-merge.sh → only the approved_by=user CEO-marker strings; zero references to review author).

Threat model this PR exists to defeat (verbatim from the AgDR-0062 context and the new pr-workflow.md rule): a build sub-agent runs as the PR author's GitHub account, cannot nest the Agent tool, so cannot spawn the real Rex. With this gate in place that agent simply:

1. gh pr review 504 --comment --body "..." → posts a COMMENTED review as atlas-apex at HEAD (GitHub blocks self-approval but NOT self-comment-reviews — the AgDR relies on this exact fact for Rex's own-PR case),
2. writes <repo>__504-rex.approved with the HEAD SHA,
3. the gate finds a review at HEAD, length ≥ 1 → passes.

The check moved the bar from "a marker file with the right name exists" to "a GitHub review by anyone (incl. the author) at HEAD exists." Both are satisfiable by the author alone. The AgDR (§ "The COMMENT-type review edge case") consciously chose to accept any-author/any-state reviews to accommodate Rex posting a COMMENT on its own PR — but that accommodation is precisely the bypass: it makes the author's own self-review indistinguishable from an independent one. The new rule's own words apply to the new gate: it "satisfies the filename of the gate requirement without satisfying its intent."

Required fix: the gate must require the review author to be independent of the PR author. Concretely: fetch the PR author (gh pr view <N> --json author -q .author.login) and require at least one review at HEAD whose .author.login differs from it:

PR_AUTHOR=$(gh pr view "$PR_NUMBER" ${CMD_REPO:+--repo "$CMD_REPO"} --json author -q '.author.login' 2>/dev/null)
_INDEP_REVIEW_AT_HEAD=$(echo "$_REVIEWS_JSON" | jq -r --arg sha "$CURRENT_SHA" --arg author "$PR_AUTHOR" \
  '[.[] | select(.commit.oid == $sha and .author.login != $author)] | length')

If an independent reviewer account isn't operationally available (Rex shares the author account in some setups), this needs a different authenticity signal than "a GitHub review exists" — because a self-review is not evidence of an independent pass. Either way, "any review at HEAD" cannot be the predicate. This is the single decisive finding; everything below is secondary.

Issue 2 (BLOCKING, same root) — the test suite asserts the wrong invariant and therefore can't catch the regression.

Every gh shim in test_block_unreviewed_merge.sh returns reviews JSON with no author field at all (e.g. line 72, 396, 521, 603): [{"state":"COMMENTED","commit":{"oid":"$FIXED_SHA"}}]. Case G1 ("real GitHub review at HEAD → passes") passes an authorless review and asserts the merge is allowed — which is exactly the self-review-shaped payload that should be blocked. There is no test where the only review at HEAD is authored by the PR author and the gate blocks. The "33 PASS" headline is real but the suite encodes the bug as expected behaviour. After the fix, add: (a) review at HEAD authored by PR-author-only → BLOCK; (b) review at HEAD by an independent login → PASS.

Things that ARE correct (verified, not blocking)

• Crux #3 — empty-reviews does NOT silently degrade. gh ... -q '.reviews' on a PR with zero reviews emits [] (a non-empty string), so it skips the [ -z "$_REVIEWS_JSON" ] degrade branch (line 228) and flows to jq length == 0 → BLOCK (line 247). Confirmed jq behaviour and test G2. The degrade branch correctly triggers only on genuine gh failure (empty stdout from non-zero exit) and on jq parse failure (line 243), both WARNed to stderr. Good.
• Crux #4 — SHA matching. The review must match CURRENT_SHA (the PR HEAD, resolved via resolve_pr_head, fallback local HEAD with WARN) via select(.commit.oid == $sha), and the marker SHA is independently matched at line 196. Stale-commit reviews (test G3) correctly block. Good.
• Crux #5 — Layers 1+2. Guardrail text present in all 7 build-agent files (## You cannot self-review, MUST NOT write .claude/session/reviews/); the ## Build agents cannot self-review rule is in pr-workflow.md; warn-review-marker-write.sh is exit-0 advisory and wired to BOTH Write and Bash PreToolUse in settings.json (valid JSON confirmed). These layers are good — but they are self-discipline + advisory, so they cannot substitute for the broken mechanical gate.
• Crux #6 — tests + shellcheck. 33/33 pass; the 25 pre-existing cases (markers, structured CEO, compound-command #426, sync-squash #459, cross-repo #485) are intact and the gh-shim additions didn't neuter them. shellcheck clean on both hooks (only SC1091 info on dynamic source, pre-existing repo-wide).
• No forged *504-rex.approved marker exists in either the worktree or the ops fork reviews/ dir. Nothing to remove.

Suggestions (non-blocking)

• The AgDR's own "Cons" cell for the chosen option already flags "GitHub blocks formal self-approval, so Rex posts a COMMENT-type review" — consider adding an explicit "Consequences" line acknowledging that accepting any-author reviews is a deliberate weakening, with the independence requirement as the mitigation, once Issue 1 is addressed.

Verdict

CHANGES REQUESTED — Per the review brief, crux #1 (author-independence) is decisive: the gate accepts author self-reviews, so it is theater against the precise actor (a build sub-agent posting a COMMENT review as the PR author) the PR targets. Layers 1–2 are good and the SHA/degrade logic (cruxes #3–#4) is correct, but the load-bearing mechanical backstop must reject self-authored reviews before this can ship. No rex approval marker written.

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

Design Review (RE-review): PR #504 — AgDR-0062 Rex Marker Authenticity

Commit: `52232055d6f01cdeaf7fb426dd64e29d8a7a406f`

Summary

Re-review after CHANGES REQUESTED. The prior verdict flagged two B1 gaps: the AgDR was silent on author-independence (the load-bearing property) and named only infra-outage (not deliberate-bypass) in the degrade section. The author has closed both, plus the two minor fixes. Verifying against the four re-review checks + an implementation spot-check.

Review Lens Results

• Quality attributes / NFRs: Pass — one extra `gh` call per merge, bounded; graceful-degrade preserves availability.
• Design patterns & structure: Pass — defence-in-depth (prompt guardrail + advisory hook + hard gate), consistent with existing CEO-marker / advisory-hook patterns.
• Technical debt: Pass — the SHA-only degrade is explicit, justified, and now names its residual risk.
• Decisions (AgDR linkage): Pass — AgDR-0062 is the decision record; 4-option table; #494 + #504 linked.
• Risk: Pass — deliberate-bypass vector now named (forced gh-failure → SHA-only); stderr warning surfaces the skip for operator review.
• Trade-off analysis: Pass — all-three-layers chosen over each single layer, with the failure mode each addresses spelled out.
• Requirements traceability: Pass — traces to #494's observed self-review behaviour; PR #504's three `atlas-apex` COMMENTED self-reviews are the live demonstration the AgDR cites.
• Migration safety: N/A
• Adopter Handbooks: N/A — framework-internal hook/agent change.

B1 closure verification

1. Author-independence is now a clearly-stated checked property. New "Author-independence as the checked property" section (AgDR L37–41): the gate filters `.author.login != <pr_author>` and "a self-review by the PR author at HEAD — regardless of state — does not count." The "COMMENT-type review edge case" section (L43–45) no longer reads as accepting a self-comment — it explicitly closes with "a self-comment does not satisfy the gate regardless of its state." Not hand-waved.
2. Deliberate-bypass vector named (L50–51): "a determined actor could force a gh-failure (e.g. by revoking or corrupting auth credentials mid-session) to reach this weaker SHA-only path" — beyond just infra outage. Mirrored in the hook comment (L222–225).
3. Artifacts self-cite fixed — L62 now reads `#504` (was #495).
4. Otherwise sound + traceable — options table, #494 linkage, adopter blast-radius all present.

Implementation spot-check (matches the AgDR claim)

`block-unreviewed-merge.sh` L264–266 filters reviews with `select(.commit.oid == $sha and .author.login != $author)` — the author-independence check is present and load-bearing, exactly as the AgDR states. The PR author login is fetched at L228–236. Test suite is green (35/35), including the two decisive cases: "self-review by PR author at HEAD → BLOCK (author-independence, B1 fix)" and "self-review + independent review at HEAD → PASS". The live PR carrying three `atlas-apex` COMMENTED self-reviews at HEAD is the correct end-to-end demonstration — under this gate those self-comments do not satisfy the merge.

Blocking Findings

None. The B1 gap is closed in both the AgDR and the implementation.

Suggestions

• nit (non-blocking): the consequences section could note that an adopter whose Rex bot shares the PR-author identity (single-account setups) will be blocked by the independence check — that's the intended behaviour, but worth a one-liner so single-identity adopters aren't surprised.

Verdict

APPROVED

🏛️ Reviewed by Tariq (Solution Architect)
📌 Reviewed commit: `52232055d6f01cdeaf7fb426dd64e29d8a7a406f`

Code Review (RE-review): PR #504

Commit: 52232055d6f01cdeaf7fb426dd64e29d8a7a406f

Re-review after the B1 fix (author-independence in the merge-gate real-review check). I independently re-cloned the PR HEAD files via the GitHub API, ran the test suite against the PR-HEAD hook in an isolated sandbox, and adversarially traced the self-review path end-to-end (not just trusting the test assertions).

Summary

Layer 3's merge gate previously accepted any review at HEAD by any author — so a build sub-agent (the PR author) could self-post a COMMENTED review via gh pr review --comment, write the local marker, and pass the gate. The fix fetches the PR author (gh pr view --json author -q .author.login) and filters reviews to select(.commit.oid == $sha and .author.login != $author). A self-review at HEAD no longer counts.

Checklist Results

• Architecture & Design: Pass
• Code Quality: Pass
• Testing: Pass (35 green)
• Security: Pass — this is itself a security/integrity gate; B1 bypass closed
• Performance: Pass (one extra gh pr view --json author call per merge; negligible)
• PR Description & Glossary: Pass
• Summary Bullet Narrative: Pass (advisory)
• Technical Decisions (AgDR): Pass — AgDR-0062 linked; Gate 3b → Tariq reviews separately
• Adopter Handbooks: N/A (no handbooks triggered by this diff)

Crux verification (B1)

1. block-unreviewed-merge.sh — confirmed the author fetch + the .author.login != $author jq filter. Empty-after-filter returns "0" → BLOCKS (exit 2), does NOT degrade. Graceful-degrade fires ONLY when the reviews/author gh call returns empty (genuine gh/jq failure), and even then still requires the SHA to match.
2. Adversarial end-to-end trace (isolated sandbox, ops-root pin neutralised, mock gh):
   • rex marker + valid CEO marker + self-review only (PR author == review author) at HEAD → EXIT 2, message no INDEPENDENT GitHub review at HEAD … a self-review by the PR author does not count. Confirmed the bypass is genuinely closed, not just asserted.
   • rex marker + independent reviewer at HEAD → EXIT 0 (pass).
   • self-review + independent review both at HEAD → EXIT 0 (at-least-one-independent satisfies the gate).
   • reviews call empty (real gh failure) + SHA match → EXIT 0 with WARN (graceful-degrade intact).
3. Tests — ran test_block_unreviewed_merge.sh against PR-HEAD files: 35 PASS / 0 FAIL. New cases present and green: self-review by PR author at HEAD → BLOCK (author-independence, B1 fix) and self-review + independent review at HEAD → PASS. The gh shims carry --json author (atlas-apex) and --json reviews (with .author.login), and make_sandbox_with_reviews takes a pr_author param to drive the independence scenarios. No test still encodes the old authorless-passes behaviour.
4. SHA→commit match logic survived — rex sha mismatch → blocks and ceo sha mismatch → blocks both green; the rex-SHA-mismatch check runs and short-circuits before the review check.
5. Layers 1+2 intact — agent guardrail § "You cannot self-review" present in all build-class agents; pr-workflow § "Build agents cannot self-review" present with the cannot-nest-Agent rationale; warn-review-marker-write.sh wired on both PreToolUse matchers (Write + Bash).
6. shellcheck — clean. Only SC1091 info on the dynamic source paths (pre-existing repo-wide); no new warnings; warn hook fully clean.

Suggestions (non-blocking)

• nit: the hook's top-of-file comment block (lines ~51–52) still reads "the check accepts ANY review state … by any reviewer at HEAD" — stale relative to the new author-independence requirement. The inline comments at the actual check site and AgDR-0062 § "Author-independence as the checked property" are correct; only this header summary lags. Worth a one-line tweak on a future touch.
• nit: the PR body's Testing block lists 33 PASS; the suite now reports 35 (the two new B1 cases). Cosmetic — refresh the pasted output if convenient.

Verdict

APPROVED — the author-independence bypass (B1) is closed and independently verified; a self-posted COMMENT review by the PR author no longer satisfies the gate. Rex marker written at HEAD 5223205.

Note: this PR carries AgDR-0062 → Gate 3b (Solution Architect / Tariq) is a separate required sign-off before merge.

Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: 52232055d6f01cdeaf7fb426dd64e29d8a7a406f

Design Review: PR #504 (AgDR-0062, re-scoped) — VERDICT: CHANGES REQUESTED

(Posted as a comment — GitHub blocks request-changes on one's own PR. The verdict is CHANGES REQUESTED; the architecture-approval marker has NOT been written.)

Commit: 769bee4354a9192d40c97858d7499f8c59fc6479

Summary

The PR was re-scoped: the mechanical independent-review gate is dropped, and only Layer 1 (prompt-convention guardrails in 7 build-class agent files) + Layer 2 (an advisory warn-review-marker-write.sh, exit 0) ship now. AgDR-0062 records the deferral. I verified git diff upstream/dev -- .claude/hooks/block-unreviewed-merge.sh is empty — the gate is genuinely unchanged.

Review Lens Results

• Quality attributes / NFRs: Pass — no new network calls; advisory hook is exit-0 by construction
• Design patterns & structure: Pass — defense-in-depth (prompt + advisory), follows the check-upstream-drift.sh advisory-hook pattern
• Technical debt: Pass — the deferred gate is explicit debt with a named paydown path (require_independent_github_review flag)
• Decisions (AgDR linkage): Pass — AgDR-0062 captures the 4-option analysis, decision, and deferral rationale
• Risk: Pass (in the AgDR) — residual forgeability honestly stated and accepted under the human CEO nod
• Trade-off analysis: Pass — the single-account author-independence fatal flaw is the load-bearing reason, clearly argued
• Requirements traceability: Pass — #494 ↔ #504 linkage present; Closes #494
• Migration safety: N/A
• Adopter Handbooks: N/A (no handbooks matched these paths)

Blocking Findings

B1 — The advisory warn hook overclaims the deferred gate as live enforcement (contradicts the AgDR it ships with).

AgDR-0062, pr-workflow.md, and the 7 agent guardrails are all honest: the mechanical gate is deferred, block-unreviewed-merge.sh is unchanged, and today's protection is human-in-the-loop + convention. Good.

But warn-review-marker-write.sh asserts the opposite in two places — including the runtime banner an agent actually reads:

• Lines 12–13 (comment): "The hard enforcement is in block-unreviewed-merge.sh, which requires a real posted GitHub review at HEAD in addition to the local marker file (#494)."
• Lines 61–62 (the stderr banner shown at write time): "A fabricated marker will be rejected at merge time — block-unreviewed-merge.sh requires a real posted GitHub review at HEAD in addition to the local marker file (#494)."

Neither is true in the shipped state. The gate requires no GitHub review; a fabricated rex marker will NOT be rejected at merge time (only the CEO nod stands between it and merge). This is exactly the failure mode the verdict bar names: agent-facing text overclaiming the remaining layers as a mechanical gate. A build agent reading this banner is told a lock exists that does not — which both misleads the agent and undercuts the "defer, don't pretend" honesty the rest of the PR earns.

This is residue from the earlier iteration when layer 3 was in scope. The fix is text-only: change both passages to describe the actual current state, e.g. "A fabricated marker is a visible rule violation; the merge still requires an explicit human CEO nod via /approve-merge, which is the current safety. A stronger mechanical gate is deferred per AgDR-0062."

Assessment of the deferral (the four questions)

1. Deferral sound + honestly recorded: Yes — in the AgDR. Ships layers 1+2; defers the gate behind a named opt-in flag; explains the single-account → author-independence-unsatisfiable → blocks-every-merge chain; states the residual forgeability risk and its acceptance condition. Not hand-waved.
2. "Defer, don't pretend" is the right call: Yes — shipping a gate that blocks every merge in the default setup would be worse than none. But B1 is the one spot where the PR does pretend (in the hook banner), so it must be fixed for the call to hold end-to-end.
3. Traceability: Intact — 4-option table retained, Decision/Consequences updated, #494/#504 linked, require_independent_github_review: true named as the future path.
4. Implementation matches "deferred": Confirmed — gate hook diff is empty.

Suggestions

• suggestion: After fixing B1, consider a one-line cross-ref in the AgDR Consequences noting the warn hook is the only mechanical artifact shipped and it is non-blocking, so no reader conflates it with the gate.

Verdict

CHANGES REQUESTED — one blocking text fix (B1). The architecture of the deferral is sound and honest; the only defect is the warn hook's banner claiming a mechanical merge-time rejection that this PR deliberately did not ship. Fix the two passages in warn-review-marker-write.sh to match the AgDR, push, and re-request — fast turnaround.

🏛️ Reviewed by Tariq (Solution Architect)
📌 Reviewed commit: 769bee4354a9192d40c97858d7499f8c59fc6479

Code Review: PR #504 — Verdict: CHANGES REQUESTED

Posted as a comment (--request-changes is blocked on own PR). Verdict is CHANGES REQUESTED — two items block; do not write the rex approval marker.

Commit: 769bee4354a9192d40c97858d7499f8c59fc6479

Summary

Re-scoped fix for #494 (build-agent self-review). The earlier mechanical merge-gate change is fully reverted; this PR ships only the prompt-convention guardrails (Layer 1, 7 build-class agents) + an advisory warn hook (Layer 2), with the mechanical independent-review gate deferred to an opt-in per AgDR-0062. The core re-scope goal is met — but two issues block.

Checklist Results

• ✅ Architecture & Design: Pass — minimal, additive, follows the advisory-hook pattern (check-upstream-drift / detect-role-trigger)
• ✅ Code Quality: Pass — warn hook shellcheck-clean, exit 0 always
• ✅ Testing: Pass — 29/29 (W1–W4 present, G1–G4 correctly removed)
• ✅ Security: Pass — no secrets, no new attack surface; honest residual-risk disclosure in AgDR-0062
• ✅ Performance: Pass — no new network calls, gate untouched
• ❌ PR Description & Glossary: Fail — no Glossary section (mandatory)
• ✅ Summary Bullet Narrative: Pass — bullets are narrative (what + why)
• ✅ Technical Decisions (AgDR):Pass — AgDR-0062 present, covers the decision and the deferral
• ✅ Adopter Handbooks: N/A — no language handbooks load (diff is .sh/.md/.json)

Verification of the re-scope (load-bearing checks)

• ✅ Merge gate reverted: git diff upstream/dev -- .claude/hooks/block-unreviewed-merge.sh is EMPTY (0 lines). Byte-identical to baseline. The single-account maintainer is not locked out. This was the whole point of the re-scope and it holds.
• ✅ Scope clean: diff vs upstream/dev = exactly the 12 expected files (7 agents + warn hook + settings.json + pr-workflow.md + test file + AgDR-0062), 337 insertions / 0 deletions. block-unreviewed-merge.sh shows no diff.
• ✅ Layer 1: all 7 build-class agents carry the "You cannot self-review" section, tailored per role.
• ✅ Layer 2 (rule text): pr-workflow.md § "Build agents cannot self-review" correctly describes enforcement as warn hook + prompt convention + CEO nod, and explicitly defers the mechanical gate to AgDR-0062. No false claim that the gate enforces independence. Good.
• ✅ settings.json valid JSON (jq .), warn hook wired on both Write + Bash.
• ✅ Stale *504-rex.approved (5223205, did not match HEAD) removed during this review.
• ℹ️ Note: the *504-architecture.approved marker (5223205) also predates the rescope and won't match HEAD — Tariq's re-review at 769bee4 is still pending (correctly out of scope for this code review).

Issues Found (blocking)

1. ⛔ Missing Glossary — .claude/rules/pr-quality.md (No exceptions)
The PR body has no ## Glossary section. The description introduces several terms that warrant one: build-class sub-agent, two-reviews merge gate, rex marker / ceo marker, advisory warn hook, independent-review gate. Add a Glossary table.

2. ⛔ Stale false claims inside warn-review-marker-write.sh — contradict the re-scope
The merge gate was reverted (confirmed empty diff; block-unreviewed-merge.sh does zero review-checking), yet the warn hook still asserts the opposite in two places:

• Line 12–13 (file comment): "The hard enforcement is in block-unreviewed-merge.sh, which requires a real posted GitHub review at HEAD in addition to the local marker file (#494)."
• Line 61–62 (banner shown LIVE to the agent at write time): "A fabricated marker will be rejected at merge time — block-unreviewed-merge.sh requires a real posted GitHub review at HEAD in addition to the local marker file (#494)."

Both are now false. This is the exact false claim the re-scope exists to remove — fixed correctly in pr-workflow.md, but it leaked into the hook's own comment and (worse) its live banner. The banner actively misleads: it tells an agent a fabricated marker "will be rejected at merge time" when, post-revert, nothing rejects it. Rewrite both to reflect actual enforcement (per-PR CEO nod via /approve-merge + orchestrator running a real separate Rex review) and point at AgDR-0062 for the deferred opt-in gate. The W1–W4 tests only assert exit code + advisory presence, so they won't catch wording — consider asserting the banner does NOT claim the gate rejects markers.

Suggestions (non-blocking)

• The agent-file heading is ## You cannot self-review while the rule-file heading + AgDR use ## Build agents cannot self-review. Aligning the two improves grep-ability.

Verdict

CHANGES REQUESTED — the re-scope's load-bearing goal (gate reverted, byte-identical to baseline) is fully met and verified. Two small, localized items block: (1) mandatory Glossary missing; (2) the warn hook's own comment + live banner still make the false "gate requires a real posted GitHub review" claim the re-scope exists to remove. After both land + a re-push, re-request review.

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

Design Review (re-review): PR #504 — AgDR-0062

Commit: `dbb2d57b8283cc405322ec1f44f74c64a263de2c`

Summary

Re-review of the single blocking finding (B1) from my prior CHANGES REQUESTED verdict: warn-review-marker-write.sh overclaimed a mechanical gate (asserting block-unreviewed-merge.sh "requires a real posted GitHub review at HEAD" / "fabricated marker rejected at merge time") that this PR actually reverts. The author pushed a text-only fix.

B1 — Resolved ✅

• Runtime banner (warn-review-marker-write.sh L54-71) no longer asserts any mechanical merge-time rejection. It now states the real safety is the per-PR human CEO nod (/approve-merge) plus the orchestrator running the independent review, and names the mechanical gate as a deferred opt-in citing AgDR-0062.
• Header comment (L11-17) makes the same correction: advisory-only, never blocks; deferred config opt-in for hands-off / multi-account setups; cites AgDR-0062 for why it is off by default.
• The artifact no longer contradicts the AgDR it ships with.

Deferral truthfulness — re-confirmed ✅

• git diff upstream/dev -- .claude/hooks/block-unreviewed-merge.sh → empty (merge gate genuinely unchanged), matching AgDR-0062 L73/L93 and the PR test-plan checkbox.
• AgDR-0062 honestly records all three required points:
  • (a) deferral + opt-in flag require_independent_github_review: true (L29-37, L67, L75)
  • (b) single-account rationale — same GitHub identity means author-independence is unsatisfiable and would block every merge (L23, L33-38)
  • (c) residual forgeability of the bare-SHA rex marker, accepted while every merge has the human CEO nod (L62-67, L74)

Review Lens Results

• Quality attributes / NFRs: Pass (advisory hook, exit 0 always; no merge-path latency added)
• Design patterns & structure: Pass (mirrors check-upstream-drift.sh / detect-role-trigger.sh advisory pattern)
• Technical debt: Pass (deferred gate explicitly tracked with a re-enable path, not silent debt)
• Decisions (AgDR linkage): Pass (AgDR-0062 captures the defer-vs-ship decision with options table)
• Risk: Pass (residual forgeability named and accepted with stated mitigation)
• Trade-off analysis: Pass (four options weighed; single-account fatal flaw of the mechanical gate documented)
• Requirements traceability: Pass (closes #494; layers 1+2 address the observed self-review behaviour)
• Migration safety: N/A
• Adopter Handbooks: N/A (framework-internal change)

Blocking Findings

None — B1 resolved.

Verdict

APPROVED

🏛️ Reviewed by Tariq (Solution Architect)
📌 Reviewed commit: `dbb2d57b8283cc405322ec1f44f74c64a263de2c`

Code Review: PR #504 (re-review)

Commit: dbb2d57b8283cc405322ec1f44f74c64a263de2c

Summary

Build-class self-review hardening. Layered approach: (1) prompt guardrails in 7 build-class agent files prohibiting self-written review markers and APPROVED framing, (2) an advisory warn-review-marker-write.sh hook (exit 0 always) that nudges when a *-rex.approved / *-ceo.approved write is attempted. The earlier-iteration mechanical gate was dropped; block-unreviewed-merge.sh is restored to the upstream/dev baseline, and AgDR-0062 documents the deferral as a config opt-in for hands-off / multi-account setups.

Re-review of prior CHANGES REQUESTED findings — both resolved

1. Stale false claims in warn-review-marker-write.sh — FIXED. grep -n 'rejected at merge time\|requires a real posted' → no matches. The comment (L11-17) and runtime banner (L63-67) now correctly attribute the real safety to the per-PR human CEO nod (/approve-merge) plus the orchestrator running the independent review, and cite AgDR-0062 for the deferred opt-in gate. No claim that the merge gate rejects fabricated markers at merge time.
2. PR body Glossary — PRESENT. ## Glossary section with 4 terms (review marker, build-class agent, author-independence, advisory hook).

Load-bearing invariants re-confirmed

3. git diff upstream/dev -- .claude/hooks/block-unreviewed-merge.sh → EMPTY. Merge gate unchanged from baseline.
4. bash .claude/hooks/tests/test_block_unreviewed_merge.sh → 29 PASS / 0 FAIL. shellcheck warn-review-marker-write.sh → clean. Hook is exit-0 advisory; wired in settings.json on both Write and Bash matchers via the ops-root-resolving launcher; jq . .claude/settings.json valid.

Checklist Results

• Architecture & Design: Pass (additive layered guardrails; gate untouched)
• Code Quality: Pass (shellcheck clean, exit-0 advisory contract honored)
• Testing: Pass (29/29; W1-W4 warn-hook cases added, stale G1-G4 removed)
• Security: Pass (residual forgeable-marker risk explicitly documented + accepted in AgDR-0062)
• Performance: N/A
• PR Description & Glossary: Pass
• Technical Decisions (AgDR):Pass — AgDR-0062 documents the deferred-gate decision and trade-off
• Adopter Handbooks: N/A (no handbooks triggered by this diff)

Issues Found

None.

Verdict

APPROVED (posted as comment — GitHub blocks self-approval on own PR)

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

Design Review (re-bless): PR #504 — AgDR-0062

Commit: 70d83994c575c0b5ea5274cadeae6db424289177
Prior approval: dbb2d57 (stands)

Summary

A CI-only fix pushed a new commit after my prior APPROVED at dbb2d57. Focused delta check confirms the new commit did not touch the AgDR's substance.

Delta findings

• AgDR-0062 diff dbb2d57..70d8399 — blank-lines-only. All four hunks are markdownlint MD032 fixes (blank line before a list). No change to the Decision, the "gate only adds value when" rationale, the residual-risk section, or the opt-in flag (require_independent_github_review).
• Deferral truthfulness — .claude/hooks/block-unreviewed-merge.sh diff vs upstream/dev is empty. The deferred mechanical independent-review gate (layer 3) remains genuinely unbuilt, so the AgDR's deferral statement is still accurate.

Verdict

APPROVED — prior approval re-blessed at the new HEAD. No substantive change. Architecture sign-off marker re-issued at 70d8399.

🏛️ Reviewed by Tariq (Solution Architect)
📌 Reviewed commit: 70d83994c575c0b5ea5274cadeae6db424289177