Architecture: Codify Anti-Rubber-Stamp PR Defense Protocol (#10522)

* docs(ai): extract Review Response Protocol into dedicated reference (#10521)

* fix(ai): correct trailing whitespaces and stale references per review (#10521)

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>

Author: neo-gemini-pro

.agents/skills/pr-review/references/pr-review-guide.md

@@ -89,7 +89,7 @@ The Retrospective daemon explicitly regex-matches these tags during REM sleep:
 *   **`[TOOLING_GAP]`**: Use this to document failures in the development workflow, broken test commands, or MCP tools that failed during the generation of the PR.
 *   **`[RETROSPECTIVE]`**: Use this for high-level takeaways or architectural praise.
 
-**Author-side response tags (`pull-request` §6):** The `pull-request` skill §6 Review Response Protocol defines a symmetric set of author-side tags — `[ADDRESSED]`, `[DEFERRED]`, `[REJECTED_WITH_RATIONALE]` — used by PR authors when responding to Required Actions from a review. Reviewer-side and author-side tags form a unified taxonomy the Retrospective daemon ingests as a complete negotiation thread; both sides of the review cycle are mineable signal.
+**Author-side response tags (`pull-request` §6):** The `.agents/skills/pull-request/references/review-response-protocol.md` document defines a symmetric set of author-side tags — `[ADDRESSED]`, `[DEFERRED]`, `[REJECTED_WITH_RATIONALE]` — used by PR authors when responding to Required Actions from a review. Reviewer-side and author-side tags form a unified taxonomy the Retrospective daemon ingests as a complete negotiation thread; both sides of the review cycle are mineable signal.
 
 ## 5. Required Actions & Cross-Linking
 *   **Related Graph Nodes:** Every PR review MUST list related graph nodes (e.g., `Target Epic ID`, `Issue ID`) to ensure the Native Edge Graph links the evaluation to the overarching goal.

.agents/skills/pull-request/assets/review-response-template.md

@@ -2,7 +2,7 @@
 Review Response Template
 Post via manage_issue_comment with action=create on the PR thread.
 Use this for author-side responses to reviewer-initiated Required Actions
-(per pull-request-workflow.md §6 Review Response Protocol).
+(per review-response-protocol.md).
 Remove HTML comments before posting.
 -->
 

.agents/skills/pull-request/references/pull-request-workflow.md

@@ -132,7 +132,7 @@ You MUST follow this exact handoff protocol:
 
 3. **[HUMAN_ONLY] Merge Execution:** Agents are strictly forbidden from executing the merge itself. Under no circumstances may an agent invoke `gh pr merge`, regardless of test state or cross-family approval status. Handoff explicitly terminates when the PR enters the `APPROVED` state. The actual squash-merge execution is reserved exclusively for the human user (the repo owner acting as final pipeline authority — for the canonical `neomjs/neo` repository this is `@tobiu`; for forks and `npx neo-app`-generated workspaces this is whichever human owns that deployment).
 
-**Cross-Review Response Cycle:** If an external reviewer posts `Status: Request Changes` on your PR, you re-enter the author loop per §7 (Review Response Protocol). After addressing the Required Actions with follow-up commits and posting the structured response comment, you halt again for re-review. Done-ness is per-handoff, not per-lifetime.
+**Cross-Review Response Cycle:** If an external reviewer posts `Status: Request Changes` on your PR, you re-enter the author loop per `.agents/skills/pull-request/references/review-response-protocol.md`. After addressing the Required Actions with follow-up commits and posting the structured response comment, you halt again for re-review. Done-ness is per-handoff, not per-lifetime.
 
 ### 6.1 The Cross-Family Mandate
 
@@ -153,80 +153,6 @@ If you are operating inside the canonical `neomjs/neo` repository as a core swar
 
 This strict feedback loop prevents duplicated work and confusion over ticket ownership when multiple agents are running concurrently. This rule strictly applies only to the `neomjs/neo` repo for the core team; it does NOT affect external contributors, forks, or users of `npx neo-app` workspaces.
 
-## 7. Review Response Protocol
-
-Once a reviewer posts `Status: Request Changes` (per the `pr-review` skill) or `Status: Comment` with actionable Required Actions, the author MUST respond via a structured comment on the PR thread. This closes the review-negotiation loop in a way both downstream human re-reviewers and automated consumers (Retrospective daemon, graph ingestion) can parse unambiguously.
-
-### 7.1 When to Invoke
-
-Trigger this protocol when any of:
-
-- A reviewer's comment contains a Required Actions checklist
-- A reviewer's status is `Request Changes`
-- A reviewer's status is `Comment` and they have listed architectural concerns the author agrees warrant response
-
-Skip if the review is `Approved` with zero blocking concerns — a brief thank-you or silence suffices.
-
-### 7.2 Per-Item Status Tags
-
-Every Required Action from the reviewer's comment MUST receive an explicit status in the author's response comment. Three tags, mirroring `pr-review` §4 Graph Ingestion Notes so the Retrospective daemon sees a unified taxonomy:
-
-- **`[ADDRESSED]`** — fix pushed in commit X; 1-2 sentences on what changed.
-- **`[DEFERRED]`** — not addressed in this PR; follow-up ticket # cited + rationale for deferral.
-- **`[REJECTED_WITH_RATIONALE]`** — author disagrees with the reviewer's ask; rationale documented for the reviewer's potential counter-challenge. **Do NOT silently skip an item** — if you disagree, say so explicitly.
-
-### 7.3 Template
-
-Use the template at `.agents/skills/pull-request/assets/review-response-template.md` as the structural skeleton. Do NOT ad-hoc the format — the per-item tag structure is load-bearing for automated ingestion by the Retrospective daemon.
-
-### 7.4 Authorship Respect
-
-Post the response as a **NEW comment** on the PR thread. Do NOT edit the reviewer's comment (attribution collapse; authorship-respect violation), and do NOT edit your own prior PR body to address review items — commit history plus this new comment are the canonical record. Aligned with the authorship-respect rule that applies across all surfaces (tickets, PR bodies, review comments).
-
-### 7.5 Commit Message Convention
-
-Follow-up commits addressing review feedback use the standard Conventional Commits format with the ticket ID. The commit message does NOT need to cite the reviewer or specific Required Action number — the Addressed comment on the PR thread carries the link:
-
-```
-fix(scope): <concise description> (#TICKET_ID)
-```
-
-Example: `fix(ai): protect SESSION and MEMORY from getOrphanedNodes cleanup (#10151)` — the Addressed comment explicitly maps this commit SHA to the specific Required Action it closes.
-
-### 7.6 Re-Review Signal
-
-End the Addressed comment with `Re-review requested.` to signal the reviewer that the author's response cycle is complete. Do NOT add a new commit after posting the Addressed comment unless you are starting another response cycle (in response to the reviewer's follow-up feedback — new round, new comment).
-
-### 7.7 Relationship to Sibling Skills
-
-- **`pr-review` §4 (Graph Ingestion Notes)** — the tag convention here mirrors `[KB_GAP]` / `[TOOLING_GAP]` / `[RETROSPECTIVE]`. Reviewer-side and author-side tags form a unified taxonomy.
-- **`pr-review` §5 (Required Actions)** — the author's response provides per-item status against the reviewer's Required Actions.
-- **`pull-request` §1 (Stepping Back)** — the pre-PR reflection that catches obvious issues should prevent most Required Actions. If you find yourself responding to many rounds of Request Changes on the same PR, revisit Stepping Back discipline.
-- **`ideation-sandbox/references/ideation-sandbox-workflow.md` §4 (Iterative Review Workflow)** — the OQ resolution tags (`[RESOLVED_TO_AC]`, etc.) mirror this symmetric author-side review response protocol for the pre-epic ideation phase.
-
-### 7.8 Anti-Patterns
-
-| Anti-pattern | Why it harms |
-|---|---|
-| Pushing a follow-up commit without an Addressed comment | Reviewer must discover + match commits to Required Actions manually; breaks re-review efficiency |
-| Silently skipping a Required Action | Signals neither agreement (should be `[ADDRESSED]`) nor disagreement (should be `[REJECTED_WITH_RATIONALE]`) — leaves reviewer uncertain |
-| Editing the reviewer's comment | Authorship-respect violation; attribution collapse |
-| Editing your own prior PR body to "address" items | Commit + Addressed comment is the canonical record; body edits erase the review-negotiation thread |
-| Using non-standard status language (*"done"*, *"fixed"*, *"won't fix"*) | Breaks the tag taxonomy; Retrospective daemon cannot ingest consistently |
-| Appending to the first Addressed comment across multiple review rounds | Violates the polish-vs-pivot analog from #10109 — new round = new comment preserving the negotiation evolution |
-
-### 7.9 Empirical Example
-
-PR #10161 (MemorySessionIngestor) received a `Status: Request Changes` review with one Required Action (*add `SESSION` and `MEMORY` labels to `GraphService.getOrphanedNodes` protection list*). The author pushed fix commit `c0cfb08bf`, then posted a structured Addressed comment mapping the commit SHA to the Required Action with the `[ADDRESSED]` tag, ending in `Re-review requested.` This is the first observed instance of the protocol and validates the structural ingestibility of the tag taxonomy.
-
-### 7.10 The Empirical "Isolation-Test-After-Review" Pattern
-
-When a reviewer challenges an architectural pattern in your PR (e.g., claiming it violates a paradigm or introduces unnecessary complexity), you have two valid paths to resolve the dispute:
-1. **Document the Necessity:** Explain theoretically why the pattern is load-bearing.
-2. **Empirical Isolation Test (Preferred):** Run a binary isolation test. Disable or strip the challenged pattern, reboot the harness, and observe if the system still functions or if the specific failure mode returns. 
-
-If the isolation test proves the pattern is dead weight, remove it and document the empirical finding in your response. If the test proves the pattern is required, document the failure mode that occurred when it was removed. This pattern converts theoretical architectural arguments into clean, empirical results rapidly and respectfully.
-
 ## 8. PR Comment Hygiene (Polish vs. Pivot)
 
 When performing self-reviews or responding to feedback across multiple rounds, you must distinguish between "polish" (better execution of the same idea) and "pivot" (a change in architectural direction).

.agents/skills/pull-request/references/review-response-protocol.md

@@ -0,0 +1,89 @@
+# Review Response Protocol (Anti-Rubber-Stamp Defense)
+
+Once a reviewer posts `Status: Request Changes` (per the `pr-review` skill) or `Status: Comment` with actionable Required Actions on your PR, the author MUST respond via a structured comment on the PR thread. This closes the review-negotiation loop in a way both downstream human re-reviewers and automated consumers (Retrospective daemon, graph ingestion) can parse unambiguously.
+
+**CRITICAL: The Anti-Passive Compliance Mandate**
+Agents suffer from "interruption amnesia" when returning to a PR after a delay. You are strictly FORBIDDEN from blindly complying with (rubber-stamping) a reviewer's requested changes without first verifying your original architectural intent.
+
+## 1. The Triangular Evaluation
+
+When receiving change requests, you MUST execute this cognitive routine before touching any code:
+
+1. **Retrieve:** Read the `Origin Session ID` from your PR's body.
+2. **Re-hydrate:** Query the Memory Core (using `query_raw_memories` or `get_session_memories` with the origin session ID) to recover your original thought process and architectural intent for the implementation.
+3. **Evaluate & Defend (The Triangle):** Perform a dialectic analysis comparing:
+   - Your Original Intent (from Memory Core)
+   - The Reviewer's Request
+   - Your Fresh Analysis of the code
+
+   *If the reviewer's request contradicts the established architecture or your original (valid) intent, you MUST defend the PR. Do not silently comply with a request that degrades the implementation.*
+
+## 2. When to Invoke
+
+Trigger this protocol when any of:
+- A reviewer's comment contains a Required Actions checklist
+- A reviewer's status is `Request Changes`
+- A reviewer's status is `Comment` and they have listed architectural concerns the author agrees warrant response
+
+Skip if the review is `Approved` with zero blocking concerns — a brief thank-you or silence suffices.
+
+## 3. Per-Item Status Tags
+
+Every Required Action from the reviewer's comment MUST receive an explicit status in the author's response comment. Three tags, mirroring `pr-review` §4 Graph Ingestion Notes so the Retrospective daemon sees a unified taxonomy:
+
+- **`[ADDRESSED]`** — fix pushed in commit X; 1-2 sentences on what changed.
+- **`[DEFERRED]`** — not addressed in this PR; follow-up ticket # cited + rationale for deferral.
+- **`[REJECTED_WITH_RATIONALE]`** — author disagrees with the reviewer's ask; rationale documented for the reviewer's potential counter-challenge. **Do NOT silently skip an item** — if you disagree, say so explicitly. (Use this aggressively when the Triangular Evaluation proves the reviewer is hallucinating or derailing).
+
+## 4. Template
+
+Use the template at `.agents/skills/pull-request/assets/review-response-template.md` as the structural skeleton. Do NOT ad-hoc the format — the per-item tag structure is load-bearing for automated ingestion by the Retrospective daemon.
+
+## 5. Authorship Respect
+
+Post the response as a **NEW comment** on the PR thread. Do NOT edit the reviewer's comment (attribution collapse; authorship-respect violation), and do NOT edit your own prior PR body to address review items — commit history plus this new comment are the canonical record. Aligned with the authorship-respect rule that applies across all surfaces (tickets, PR bodies, review comments).
+
+## 6. Commit Message Convention
+
+Follow-up commits addressing review feedback use the standard Conventional Commits format with the ticket ID. The commit message does NOT need to cite the reviewer or specific Required Action number — the Addressed comment on the PR thread carries the link:
+
+```
+fix(scope): <concise description> (#TICKET_ID)
+```
+
+Example: `fix(ai): protect SESSION and MEMORY from getOrphanedNodes cleanup (#10151)` — the Addressed comment explicitly maps this commit SHA to the specific Required Action it closes.
+
+## 7. Re-Review Signal
+
+End the Addressed comment with `Re-review requested.` to signal the reviewer that the author's response cycle is complete. Do NOT add a new commit after posting the Addressed comment unless you are starting another response cycle (in response to the reviewer's follow-up feedback — new round, new comment).
+
+## 8. Relationship to Sibling Skills
+
+- **`pr-review` §4 (Graph Ingestion Notes)** — the tag convention here mirrors `[KB_GAP]` / `[TOOLING_GAP]` / `[RETROSPECTIVE]`. Reviewer-side and author-side tags form a unified taxonomy.
+- **`pr-review` §5 (Required Actions)** — the author's response provides per-item status against the reviewer's Required Actions.
+- **`pull-request` §1 (Stepping Back)** — the pre-PR reflection that catches obvious issues should prevent most Required Actions. If you find yourself responding to many rounds of Request Changes on the same PR, revisit Stepping Back discipline.
+- **`ideation-sandbox/references/ideation-sandbox-workflow.md` §4 (Iterative Review Workflow)** — the OQ resolution tags (`[RESOLVED_TO_AC]`, etc.) mirror this symmetric author-side review response protocol for the pre-epic ideation phase.
+
+## 9. Anti-Patterns
+
+| Anti-pattern | Why it harms |
+|---|---|
+| Passive Compliance (Rubber-Stamping) | Allows hallucinated or derailing reviewer requests to degrade the architecture because the author forgot their original intent. |
+| Pushing a follow-up commit without an Addressed comment | Reviewer must discover + match commits to Required Actions manually; breaks re-review efficiency |
+| Silently skipping a Required Action | Signals neither agreement (should be `[ADDRESSED]`) nor disagreement (should be `[REJECTED_WITH_RATIONALE]`) — leaves reviewer uncertain |
+| Editing the reviewer's comment | Authorship-respect violation; attribution collapse |
+| Editing your own prior PR body to "address" items | Commit + Addressed comment is the canonical record; body edits erase the review-negotiation thread |
+| Using non-standard status language (*"done"*, *"fixed"*, *"won't fix"*) | Breaks the tag taxonomy; Retrospective daemon cannot ingest consistently |
+| Appending to the first Addressed comment across multiple review rounds | Violates the polish-vs-pivot analog from #10109 — new round = new comment preserving the negotiation evolution |
+
+## 10. Empirical Example
+
+PR #10161 (MemorySessionIngestor) received a `Status: Request Changes` review with one Required Action (*add `SESSION` and `MEMORY` labels to `GraphService.getOrphanedNodes` protection list*). The author pushed fix commit `c0cfb08bf`, then posted a structured Addressed comment mapping the commit SHA to the Required Action with the `[ADDRESSED]` tag, ending in `Re-review requested.` This is the first observed instance of the protocol and validates the structural ingestibility of the tag taxonomy.
+
+## 11. The Empirical "Isolation-Test-After-Review" Pattern
+
+When a reviewer challenges an architectural pattern in your PR (e.g., claiming it violates a paradigm or introduces unnecessary complexity), you have two valid paths to resolve the dispute:
+1. **Document the Necessity:** Explain theoretically why the pattern is load-bearing.
+2. **Empirical Isolation Test (Preferred):** Run a binary isolation test. Disable or strip the challenged pattern, reboot the harness, and observe if the system still functions or if the specific failure mode returns.
+
+If the isolation test proves the pattern is dead weight, remove it and document the empirical finding in your response. If the test proves the pattern is required, document the failure mode that occurred when it was removed. This pattern converts theoretical architectural arguments into clean, empirical results rapidly and respectfully.

AGENTS.md

@@ -206,6 +206,8 @@ When you believe your codebase modifications are complete and ready for review,
 
 This skill governs branch generation, conventional commit standards, the critical "Stepping Back" reflection phase, and the state handoff endpoint sequence. Follow it exactly.
 
+**Cross-Review Response Cycle (Anti-Passive Compliance):** If an external reviewer posts `Status: Request Changes` on your PR, you **MUST** execute the Triangular Evaluation per `.agents/skills/pull-request/references/review-response-protocol.md` before pushing any follow-up commits. Passive compliance (rubber-stamping reviewer requests without re-hydrating your original architectural intent) is strictly forbidden.
+
 ## 8. The Resumption Protocol (Interruption Amnesia)
 
 During continuous agent sessions, an agent can succumb to "interruption amnesia." If the human commander injects a diagnostic sub-question or testing request (e.g., "test this A2A message" or "run this script") while the agent is midway through a ticket's lifecycle, the agent will typically resolve the sub-question and halt. It frequently drops the overarching "Definition of Done" (executing the `pull-request` skill) because its immediate context window was hijacked by the side-quest.