docs(reborn): reconcile Tier B self-repair with lease recovery - #6458
Open
italic-jinxin wants to merge 3 commits into
Open
italic-jinxin wants to merge 3 commits into
italic-jinxin wants to merge 3 commits into
Conversation
italic-jinxin
added
size: M
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6458
July 22, 2026 12:18
Destroyed
github-actions
Bot
added
size: XS
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates TurnRunner persistence, lifecycle, and lease-recovery documentation, records Tier B self-repair scope and verification evidence, and revises crash-consistency test commentary. No executable production logic or public declarations change. ChangesTurn recovery contract
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
Contributor
Author
|
@claude review |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/ironclaw_turns/tests/row_store_crash_consistency.rs`:
- Around line 2018-2022: Update the documentation above the crash-consistency
test to describe the actual reopen/compare behavior: state that reopening the
durable bytes preserves the Running state, then the test reads the reopened
store and uses assert_recovered_matches_model to compare snapshots and prefix
invariants with the reference model. Remove the inaccurate claim about a later
lease-recovery pass.
In `@docs/reborn/tier-b-self-repair-reconciliation.md`:
- Around line 25-32: Update the “Stop and report terminal recovery outcomes” row
to state that only expired leases persisted as CancelRequested transition to
Cancelled; document that an observed interrupt racing ahead of persisted
cancellation may instead become sanitized terminal Failed. Keep the existing
mappings for exhausted pre-checkpoint recovery and checkpointed expiry
unchanged.
- Around line 25-32: The evidence cells in the reconciliation table use bare
test names that are not directly auditable. Update the table’s Evidence entries
to include the exact source file paths and, where practical, line ranges for
each referenced test or implementation symbol, including entries such as
scheduler_heartbeats_long_running_executor_until_completion,
wedged_tool_call_is_reaped_by_lease_expiry_not_left_running_forever, and the
lease recovery tests. Preserve the existing claims and dispositions while
ensuring every load-bearing citation points to implementation-level definitions
or write sites.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 038e3a40-0e16-457f-8412-4d103b6d2ecf
📒 Files selected for processing (3)
crates/ironclaw_turns/tests/row_store_crash_consistency.rsdocs/reborn/contracts/turn-runner.mddocs/reborn/tier-b-self-repair-reconciliation.md
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.24% — 304712 / 353327 lines Per-crate breakdown (62 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6458
July 22, 2026 12:40
Destroyed
|
🚅 Deployed to the ironclaw-pr-6458 environment in ironclaw-ci-preview
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@crates/ironclaw_turns/tests/row_store_crash_consistency.rs`:
- Around line 2018-2023: Update the documentation comment above the
crash-consistency test to call the tests below “persistence-level tests,” since
they directly invoke the row store’s recover_expired_leases method; do not
describe them as caller-level tests.
In `@docs/reborn/contracts/turn-persistence.md`:
- Line 117: Reconcile the documentation for RecoveryRequired with the
active-lock rules by defining one consistent behavior for legacy rows, including
whether the lock is retained or released. Update the relevant statements around
the legacy terminal-status description and active-lock guidance, and cite the
actual transition or load-path symbol that implements this 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b9217fd-21a5-45d3-bc82-739455e2a4e3
📒 Files selected for processing (5)
crates/ironclaw_turns/tests/row_store_crash_consistency.rsdocs/reborn/contracts/turn-persistence.mddocs/reborn/contracts/turn-runner.mddocs/reborn/contracts/turns-agent-loop.mddocs/reborn/tier-b-self-repair-reconciliation.md
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)
docs/reborn/contracts/turn-persistence.md (1)
103-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd implementation and test references for the changed recovery rules.
These are load-bearing reservation and lease semantics, but unlike the active-lock section they do not cite the enforcing code or caller-level tests. Reference the reservation transition implementation and the concrete
heartbeat/recover_expired_leasespaths, plus tests covering requeue versus terminal outcomes, so this contract cannot drift from behavior.Also applies to: 113-118
🤖 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 `@docs/reborn/contracts/turn-persistence.md` at line 103, Update the reservation and lease recovery rules in turn-persistence.md to include references to the implementation enforcing reservation transitions, the concrete heartbeat and recover_expired_leases paths, and caller-level tests covering requeue versus terminal outcomes. Apply the same references to the related statements at lines 113–118, without changing the documented semantics.Source: Coding guidelines
🤖 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 `@docs/reborn/contracts/turn-persistence.md`:
- Line 103: Update the reservation and lease recovery rules in
turn-persistence.md to include references to the implementation enforcing
reservation transitions, the concrete heartbeat and recover_expired_leases
paths, and caller-level tests covering requeue versus terminal outcomes. Apply
the same references to the related statements at lines 113–118, without changing
the documented semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3fd277ea-7fc5-4245-acbc-165cb58b02b5
📒 Files selected for processing (2)
crates/ironclaw_turns/tests/row_store_crash_consistency.rsdocs/reborn/contracts/turn-persistence.md
This branch was successfully deployed
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
1 participant
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.
Summary
Linked Issue
Closes #6455
Part of #6369
Validation
cargo fmt --all -- --checkcargo test -p ironclaw_runner --test turn_scheduler_contract— 31 passedcargo test -p ironclaw_turns --test row_store_crash_consistency lease_expiry_— 3 passedcargo test -p ironclaw_turns --test retry_failed_turn_store_contract lease_recovery_preserves_retryability— 2 passedcargo test --test reborn_integration_lease_wedge— 13 passedbash scripts/pre-commit-safety.shSecurity Impact
No runtime or authority changes. The documentation preserves bounded recovery and explicitly excludes automatic rebuilding of untrusted tool source.
Database Impact
None. No persistence schema or serialized status vocabulary changes.
Blast Radius
Documentation and test commentary only. Runtime behavior, recovery defaults, scheduler configuration, and production wiring are unchanged.
Risk Assessment
Low. The primary risk is documentation becoming inconsistent with runtime behavior; the updated contract links directly to existing caller-level, persistence, and whole-path regression coverage.
Rollback Plan
Revert commit
293ade150. No data or runtime rollback is required.Review track: A