fix(agent-loop): restore planning-intake gate admission#205
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a one-use root recovery path for first-new-initiative planning intake, with exact merge-intent scope selection, schema-v7 loop-memory evidence, strict consumption and identity validation, supporting documents, and regression tests. ChangesRoot recovery admission
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MergeIntent
participant ScopeSelector
participant MemoryRecovery
participant EvidenceValidator
MergeIntent->>ScopeSelector: submit planning-intake or root-recovery intent
ScopeSelector->>ScopeSelector: validate exact identity and changed paths
ScopeSelector->>MemoryRecovery: select recovery scope without SignedStart
MemoryRecovery->>EvidenceValidator: generate and validate schema-v7 evidence
EvidenceValidator->>MemoryRecovery: confirm signed basis, bindings, and digest
MemoryRecovery->>MemoryRecovery: consume recovery exemption once
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
|
Recovery checkpoint: the Agent Gates failure is the exact circular defect this PR repairs. Trusted |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/check_loop_memory_state.py (1)
400-434: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
_planning_tree_failurescan crash on a malformedintent_pathinstead of failing closed with a message.
first_parent/mergeare defensively guarded withisinstance(x, str)before being passed to_git_tree, butsource.get("intent_path", "")is not: if the record has anintent_pathkey present with a non-string value (e.g.None),.get(key, default)returns that value (not the default), and the chained.removeprefix(...).removesuffix(...)raises an unhandledAttributeErrorinstead of returning a[label: ...]failure string like every other branch of this function._record_failures(the caller) doesn't stop early when the earlier "planning intake lifecycle identity is invalid" check fails, so this code path is reachable with adversarial/malformedplanning_intakerecords — exactly the input this independent checker is meant to fail closed against.🛡️ Proposed fix
- initiative = source.get("intent_path", "").removeprefix( - ".agent-loop/merge-intents/" - ).removesuffix("-PLAN.json") + intent_path = source.get("intent_path") + initiative = ( + intent_path.removeprefix(".agent-loop/merge-intents/").removesuffix("-PLAN.json") + if isinstance(intent_path, str) + else "" + )Also worth adding a regression test alongside
test_checker_rejects_planning_intake_when_first_parent_has_prefixed_treethat passessource["intent_path"] = Noneand asserts a clean failure string rather than an exception.🤖 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/check_loop_memory_state.py` around lines 400 - 434, Harden intent_path handling in _planning_tree_failures by validating that source.get("intent_path") is a string before calling removeprefix or removesuffix; return a labeled planning-intake failure when it is malformed, preserving fail-closed behavior. Add a regression test alongside test_checker_rejects_planning_intake_when_first_parent_has_prefixed_tree that sets intent_path to None and asserts the expected failure string without raising.
🤖 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.
Outside diff comments:
In `@scripts/check_loop_memory_state.py`:
- Around line 400-434: Harden intent_path handling in _planning_tree_failures by
validating that source.get("intent_path") is a string before calling
removeprefix or removesuffix; return a labeled planning-intake failure when it
is malformed, preserving fail-closed behavior. Add a regression test alongside
test_checker_rejects_planning_intake_when_first_parent_has_prefixed_tree that
sets intent_path to None and asserts the expected failure string without
raising.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a245d5b-caa5-4bf3-a0fc-e4e274e2a3cd
📒 Files selected for processing (14)
.agent-loop/initiatives/WS-ENG-ROOT-001-planning-intake-gate-recovery/STATUS.md.agent-loop/initiatives/WS-ENG-ROOT-001-planning-intake-gate-recovery/chunks/WS-ENG-ROOT-001-01-exact-planning-intake-gate-recovery.md.agent-loop/initiatives/WS-ENG-ROOT-001-planning-intake-gate-recovery/reviews/WS-ENG-ROOT-001-01-internal-review-evidence.md.agent-loop/initiatives/WS-ENG-ROOT-001-planning-intake-gate-recovery/reviews/WS-ENG-ROOT-001-01-pr-trust-bundle.md.agent-loop/merge-intents/WS-ENG-ROOT-001-01.json.agent-loop/policies/loop-memory-recovery.jsonscripts/check_chunk_contract.pyscripts/check_internal_review_evidence.pyscripts/check_loop_memory_state.pyscripts/test_agent_gates.pyscripts/test_check_chunk_contract.pyscripts/test_check_loop_memory_state.pyscripts/test_update_post_merge_memory.pyscripts/update_post_merge_memory.py
2 participants
PR Trust Bundle: WS-ENG-ROOT-001-01
Goal
Restore the documented first-planning-intake path after PR #203 accidentally
made it impossible to enter, using one exact consumed root recovery.
Scope and behavior
Only trusted engineering gates, their tests, the independent memory checker,
and the exact recovery certificate change. Product behavior is unchanged.
Ordinary work remains signed-start-only.
Why root recovery is required
Trusted base code rejects both the repair and every normal planning intake, so
no ordinary signed contract can authorize the correction. The exact recovery
is bound to current signed main and this single identity, then consumed.
Human review focus
Human merge ownership
The owner must explicitly approve this specific repair PR. A failing old scope
check is the defect being repaired and must not be represented as passing.
Summary by CodeRabbit
New Features
Bug Fixes
Tests