fix(loop-memory): freeze merge-bound evidence#190
Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces merge-bound protected-check evidence, schema-v4 recovery policies, and a shared atomic reconciliation command used by automatic and explicit-start workflows. Validators, workflows, documentation, policies, and regression tests are updated accordingly. ChangesMerge-bound evidence recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MergeWorkflow
participant ReconcileCommand
participant GitHubAPI
participant GeneratedMemory
MergeWorkflow->>ReconcileCommand: run reconcile for TARGET_SHA
ReconcileCommand->>GitHubAPI: collect merge-bound check evidence
GitHubAPI-->>ReconcileCommand: paginated checks and merge records
ReconcileCommand->>GeneratedMemory: prepare and apply recovery state
ReconcileCommand->>GeneratedMemory: assert consumption and validate state
ReconcileCommand-->>MergeWorkflow: complete before signing or publishing
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/update_post_merge_memory.py (1)
2511-2519: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated historical-recovery gate literal across two call sites.
reconcile_to_main(2512-2515) and the CLIupdatecommand (3438-3441) each re-derive the same "is this the R3 historical-recovery merge" boolean from the same hand-typedmerge_shaliteral and"WS-ENG-007-00R2"chunk-id check. Extracting a single helper (e.g._is_r3_historical_recovery(merge_sha, exemptions)) would remove the duplication and eliminate the risk of the two checks silently diverging if one site is ever edited without the other.♻️ Suggested consolidation
+def _is_r3_historical_recovery(merge_sha: str, exemptions: list[dict[str, Any]]) -> bool: + """Identify the one-time WS-ENG-007-00R2 recovery-only merge.""" + return merge_sha == "d3321698fb856f3fac320cdc7bc598f813fe1953" and any( + item.get("chunk_id") == "WS-ENG-007-00R2" for item in exemptions + ) + for merge_sha in planned: - historical = ( - merge_sha == "d3321698fb856f3fac320cdc7bc598f813fe1953" - and any(item.get("chunk_id") == "WS-ENG-007-00R2" for item in exemptions) - ) + historical = _is_r3_historical_recovery(merge_sha, exemptions)Also applies to: 3438-3445
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/update_post_merge_memory.py` around lines 2511 - 2519, Extract the duplicated historical-recovery condition into a shared helper such as _is_r3_historical_recovery(merge_sha, exemptions), including the merge SHA and WS-ENG-007-00R2 exemption check. Replace the inline boolean logic in both reconcile_to_main and the CLI update command with this helper while preserving the existing historical_recovery argument behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/reviews/WS-ENG-007-00R3-internal-review-evidence.md:
- Line 17: Add an explicit statement in the review evidence file that the listed
/root/eng006_* reviewer sessions were reassigned or inherited from ENG-006 for
use in WS-ENG-007. Keep the existing exact-SHA binding and complete
track-to-session mappings unchanged.
In `@scripts/test_update_post_merge_memory.py`:
- Around line 1948-1969: Update
test_shared_reconcile_orders_recovery_and_validates_atomically to include
d3321698fb856f3fac320cdc7bc598f813fe1953 in the planned SHAs and record keyword
arguments in the collect_merge_record mock. Assert that only the collection call
for this recovery SHA receives historical_recovery=True, while other collection
calls do not; preserve the existing event-order assertions.
---
Nitpick comments:
In `@scripts/update_post_merge_memory.py`:
- Around line 2511-2519: Extract the duplicated historical-recovery condition
into a shared helper such as _is_r3_historical_recovery(merge_sha, exemptions),
including the merge SHA and WS-ENG-007-00R2 exemption check. Replace the inline
boolean logic in both reconcile_to_main and the CLI update command with this
helper while preserving the existing historical_recovery argument behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e6220f04-e93f-49c0-8137-494a943364c1
📒 Files selected for processing (17)
.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/CHUNK_MAP.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/DECISIONS.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/DISCOVERY.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/RISKS.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/STATUS.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/chunks/WS-ENG-007-00R3-merge-bound-evidence-recovery.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/reviews/WS-ENG-007-00R3-internal-review-evidence.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/reviews/WS-ENG-007-00R3-pr-trust-bundle.md.agent-loop/merge-intents/WS-ENG-007-00R3.json.agent-loop/policies/loop-memory-recovery.json.github/workflows/loop-memory-start.yml.github/workflows/loop-memory.ymldocs/operations_post_merge_memory.mdscripts/check_loop_memory_state.pyscripts/test_agent_gates.pyscripts/test_update_post_merge_memory.pyscripts/update_post_merge_memory.py
abiorh-claw
self-requested a review
July 23, 2026 10:08
Collaborator
Author
|
Addressed the remaining CodeRabbit review: inherited reviewer-session reassignment is explicit, exact PR #189 recovery-only dispatch is now proven, and the duplicated historical predicate is centralized in |
abiorh-claw
approved these changes
Jul 23, 2026
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.
Workstream PR Trust Bundle
Chunk
WS-ENG-007-00R3— Merge-Bound Evidence RecoveryMerge intent:
.agent-loop/merge-intents/WS-ENG-007-00R3.jsonGoal
Restore signed reconciliation and explicit starts without allowing mutable
post-merge reruns to rewrite accepted evidence or block unrelated initiatives.
Human-Approved Intent
.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/INTENT.md.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/chunks/WS-ENG-007-00R3-merge-bound-evidence-recovery.mdWhat Changed
agent-gatesand backendtestevidence is frozen at PR merge time.reconcilecommand.Why It Changed
Signed memory remained at PR #178 because present-day reruns were re-evaluated
and explicit starts lacked recovery parity. That created a global mutable-history
lock that blocked the unrelated ENG-006 documentation initiative.
Design Chosen
Use immutable
merged_atas the evidence cutoff, paginate check evidencecompletely, persist exact provenance, consume an exact one-use recovery
certificate, and reconcile in temporary custody before applying authority or
publishing signed state.
Alternatives Rejected
Scope Control
Allowed Files Changed
operations documentation, initiative artifacts, review evidence, merge intent.
Files Outside Contract
Product Behavior
Evidence
Commands Run
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -m pytest -p pytest_cov.plugin -q --cov=scripts.update_post_merge_memory --cov-branch scripts/test_agent_gates.py scripts/test_update_post_merge_memory.py scripts/test_check_loop_memory_state.py PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -m pytest -p pytest_cov.plugin -q --cov=scripts.check_loop_memory_state --cov-branch scripts/test_agent_gates.py scripts/test_update_post_merge_memory.py scripts/test_check_loop_memory_state.py python3 scripts/test_agent_gates.py PR_HEAD_SHA="$(git rev-parse HEAD)" python3 scripts/check_internal_review_evidence.py python3 scripts/check_markdown_links.py python3 scripts/check_stale_workstream_wording.py git diff --check origin/main...HEADResult Summary
Acceptance Criteria Proof
Test Delta
Tests Added
historical_recovery=Truedispatch.Tests Modified
reconcileentry point.Tests Removed Or Skipped
Internal Reviewer Results
Reviewed code SHA:
4a3eee7fdd80970675ae2574542073b8a92e1ef9Reviewer run IDs:
/root/eng006_senior_arch_docs,/root/eng006_qa_ci_tests,/root/eng006_security_ops_reuse, explicitly reassigned to this exact ENG-007 review.External Review
External response:
.agent-loop/initiatives/WS-ENG-007-concurrent-pr-review-reconciliation/reviews/WS-ENG-007-00R3-external-review-response.mdCI And Gate Integrity
Remaining Risks
An intervening
mainmerge invalidates the exact first-parent bridge and mustfail closed. CodeRabbit availability remains rate-limited and supplementary.
Follow-Up Work
After merge and successful signed reconciliation, dispatch the already-approved
ENG-006 documentation refresh. CI coverage precision hardening requires a
separate workflow-policy chunk; this PR reaches literal coverage above 90%.
Human Review Focus
Human Merge Ownership