feat: auto-undelegate owner-requested accounts during commit

[snawaz]

If the scheduled intent is only Commit / CommitFinalize (without planned undelegate) for an account whose metadata says undelegation_requesor == OwnerProgram, we commit/finalize and leave the account delegated even though the owner program is asking for undelegation!!

So this PR automatically appends undelegate tasks for owner-program undelegation requests in cases where undelegation is otherwise not planned.

Summary by CodeRabbit

• Bug Fixes

  • Improved finalize-stage handling to automatically add the missing implicit undelegation for committed accounts with pending owner-program undelegation requests when not explicitly targeted.
  • Avoided generating undelegation tasks for accounts already covered by undelegation intent pubkeys.
  • Refined successful-execution cache resets so only accounts planned for undelegation are updated (failure behavior unchanged).

• Tests

  • Expanded coverage for implicit owner-program undelegation across standard and commit/finalize flows, including correct task fields and negative cases.

• Documentation

  • Updated finalize-stage DLP/undelegation guidance to reflect the new implicit undelegation behavior.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Finalize task construction now adds UndelegateTasks for committed accounts with owner-program undelegation requests that are not explicitly planned. IntentExecutionReport records undelegated accounts from finalized tasks, and successful execution selectively resets their cached task information while failures retain full reset behavior. Tests and documentation cover the updated behavior.

Suggested reviewers: gabrielepicco, thlorenz

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch snawaz/auto-undelegate-owner-request

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[snawaz]

• feat: auto-undelegate owner-requested accounts during commit #1413 👈 (View in Graphite)
• fix: distinguish ER account presence from delegation status #1411
• feat: Impl UndelegationRequest backfill loop #1378 : 2 other dependent PRs (#1391 , #1402 )
• feat: Schedule undelegation from observed DLP requests #1357
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

magicblock-committor-service/src/tasks/task_builder.rs (2)

359-416: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication between create_undelegate_tasks and create_owner_program_undelegate_tasks.

Both functions fetch metadata per account and branch on undelegation_requester == UndelegationRequester::OwnerProgram to build an undelegate_task. Consider extracting a shared helper (e.g., undelegate_task_for_account(account, metadata) -> BaseTaskImpl) to avoid the two near-identical code paths drifting apart over time.

🤖 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 `@magicblock-committor-service/src/tasks/task_builder.rs` around lines 359 -
416, Extract the shared metadata lookup and undelegation-task construction from
create_undelegate_tasks and create_owner_program_undelegate_tasks into a helper
such as undelegate_task_for_account. Have the helper accept an account and its
DelegationMetadata, derive the owner-program flag from undelegation_requester,
and update both callers to use it while preserving their existing filtering and
task ordering.

---

389-459: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Delegation metadata is now required for every committed account
get_finalize_stage_metadata_pubkeys() fetches get_all_committed_pubkeys(), and create_owner_program_undelegate_tasks() requires a metadata entry for every non-explicit committed account. If any committed account lacks delegation metadata, the whole finalize build fails with MissingDelegationMetadata, even for plain commit/commit_finalize intents. Narrow the fetch/lookup to accounts that can actually undelegate.

🤖 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 `@magicblock-committor-service/src/tasks/task_builder.rs` around lines 389 -
459, The finalize builder currently requires delegation metadata for all
committed accounts, causing plain commit flows to fail when metadata is absent.
Narrow get_finalize_stage_metadata_pubkeys and the
delegation_metadata_for_pubkey lookup in create_owner_program_undelegate_tasks
to only accounts eligible for owner-program undelegation, while preserving
explicit undelegation handling and allowing plain commit/commit_finalize intents
without metadata.

🤖 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 @.agents/context/crates/magicblock-committor-service.md:
- Line 58: Correct the path entry describing IntentExecutionService by prefixing
src/service.rs and src/service/intent_client.rs with the owning crate directory,
likely magicblock-services, instead of leaving them relative to
magicblock-committor-service. Keep the existing runtime responsibility
description unchanged and distinguish it from the CommittorService entry already
documented in the table.

---

Outside diff comments:
In `@magicblock-committor-service/src/tasks/task_builder.rs`:
- Around line 359-416: Extract the shared metadata lookup and undelegation-task
construction from create_undelegate_tasks and
create_owner_program_undelegate_tasks into a helper such as
undelegate_task_for_account. Have the helper accept an account and its
DelegationMetadata, derive the owner-program flag from undelegation_requester,
and update both callers to use it while preserving their existing filtering and
task ordering.
- Around line 389-459: The finalize builder currently requires delegation
metadata for all committed accounts, causing plain commit flows to fail when
metadata is absent. Narrow get_finalize_stage_metadata_pubkeys and the
delegation_metadata_for_pubkey lookup in create_owner_program_undelegate_tasks
to only accounts eligible for owner-program undelegation, while preserving
explicit undelegation handling and allowing plain commit/commit_finalize intents
without metadata.

🪄 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

Run ID: fb1f43c4-29f4-447f-8d2e-e99b6aeb1316

📥 Commits

Reviewing files that changed from the base of the PR and between 70fb7d7 and b2eefaa.

📒 Files selected for processing (4)

• .agents/context/crates/magicblock-committor-service.md
• magicblock-committor-service/src/intent_executor/mod.rs
• magicblock-committor-service/src/tasks/task_builder.rs
• magicblock-committor-service/src/tasks/task_strategist.rs

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

magicblock-committor-service/src/tasks/task_builder.rs (2)

359-416: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication between create_undelegate_tasks and create_owner_program_undelegate_tasks.

Both functions fetch metadata per account and branch on undelegation_requester == UndelegationRequester::OwnerProgram to build an undelegate_task. Consider extracting a shared helper (e.g., undelegate_task_for_account(account, metadata) -> BaseTaskImpl) to avoid the two near-identical code paths drifting apart over time.

🤖 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 `@magicblock-committor-service/src/tasks/task_builder.rs` around lines 359 -
416, Extract the shared metadata lookup and undelegation-task construction from
create_undelegate_tasks and create_owner_program_undelegate_tasks into a helper
such as undelegate_task_for_account. Have the helper accept an account and its
DelegationMetadata, derive the owner-program flag from undelegation_requester,
and update both callers to use it while preserving their existing filtering and
task ordering.

---

389-459: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Delegation metadata is now required for every committed account
get_finalize_stage_metadata_pubkeys() fetches get_all_committed_pubkeys(), and create_owner_program_undelegate_tasks() requires a metadata entry for every non-explicit committed account. If any committed account lacks delegation metadata, the whole finalize build fails with MissingDelegationMetadata, even for plain commit/commit_finalize intents. Narrow the fetch/lookup to accounts that can actually undelegate.

🤖 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 `@magicblock-committor-service/src/tasks/task_builder.rs` around lines 389 -
459, The finalize builder currently requires delegation metadata for all
committed accounts, causing plain commit flows to fail when metadata is absent.
Narrow get_finalize_stage_metadata_pubkeys and the
delegation_metadata_for_pubkey lookup in create_owner_program_undelegate_tasks
to only accounts eligible for owner-program undelegation, while preserving
explicit undelegation handling and allowing plain commit/commit_finalize intents
without metadata.

🤖 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 @.agents/context/crates/magicblock-committor-service.md:
- Line 58: Correct the path entry describing IntentExecutionService by prefixing
src/service.rs and src/service/intent_client.rs with the owning crate directory,
likely magicblock-services, instead of leaving them relative to
magicblock-committor-service. Keep the existing runtime responsibility
description unchanged and distinguish it from the CommittorService entry already
documented in the table.

---

Outside diff comments:
In `@magicblock-committor-service/src/tasks/task_builder.rs`:
- Around line 359-416: Extract the shared metadata lookup and undelegation-task
construction from create_undelegate_tasks and
create_owner_program_undelegate_tasks into a helper such as
undelegate_task_for_account. Have the helper accept an account and its
DelegationMetadata, derive the owner-program flag from undelegation_requester,
and update both callers to use it while preserving their existing filtering and
task ordering.
- Around line 389-459: The finalize builder currently requires delegation
metadata for all committed accounts, causing plain commit flows to fail when
metadata is absent. Narrow get_finalize_stage_metadata_pubkeys and the
delegation_metadata_for_pubkey lookup in create_owner_program_undelegate_tasks
to only accounts eligible for owner-program undelegation, while preserving
explicit undelegation handling and allowing plain commit/commit_finalize intents
without metadata.

🪄 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

Run ID: fb1f43c4-29f4-447f-8d2e-e99b6aeb1316

📥 Commits

Reviewing files that changed from the base of the PR and between 70fb7d7 and b2eefaa.

📒 Files selected for processing (4)

• .agents/context/crates/magicblock-committor-service.md
• magicblock-committor-service/src/intent_executor/mod.rs
• magicblock-committor-service/src/tasks/task_builder.rs
• magicblock-committor-service/src/tasks/task_strategist.rs

🛑 Comments failed to post (1)

.agents/context/crates/magicblock-committor-service.md (1)

58-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Ambiguous/likely-mislabeled path entry: src/service.rs collides with row 45.

This row (unprefixed src/service.rs and src/service/intent_client.rs) follows the table's convention of unprefixed paths meaning "relative to magicblock-committor-service/" — but Line 45 already documents src/service.rs as this crate's own actor-style service handle (CommittorService::try_start, CommittorMessage, etc.), a completely different responsibility than what's described here (consuming MagicContext, submitting ScheduledCommitSent, pending-intent recovery). The content described (IntentExecutionService) appears to belong to a different crate (likely magicblock-services, based on other references in this doc and cross-file evidence), so this row should be prefixed with the correct crate path to avoid confusing readers into thinking these are files within magicblock-committor-service.

📝 Suggested fix

-| `src/service.rs` and `src/service/intent_client.rs` | Runtime producer/consumer: accepts scheduled intent bundles from `MagicContext`, schedules them with the committor, consumes result broadcasts, submits `ScheduledCommitSent`, and performs pending-intent recovery after ledger replay. |
+| `magicblock-services/src/service.rs` and `magicblock-services/src/service/intent_client.rs` | Runtime producer/consumer: accepts scheduled intent bundles from `MagicContext`, schedules them with the committor, consumes result broadcasts, submits `ScheduledCommitSent`, and performs pending-intent recovery after ledger replay. |

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

| `magicblock-services/src/service.rs` and `magicblock-services/src/service/intent_client.rs` | Runtime producer/consumer: accepts scheduled intent bundles from `MagicContext`, schedules them with the committor, consumes result broadcasts, submits `ScheduledCommitSent`, and performs pending-intent recovery after ledger replay. |

🤖 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 @.agents/context/crates/magicblock-committor-service.md at line 58, Correct
the path entry describing IntentExecutionService by prefixing src/service.rs and
src/service/intent_client.rs with the owning crate directory, likely
magicblock-services, instead of leaving them relative to
magicblock-committor-service. Keep the existing runtime responsibility
description unchanged and distinguish it from the CommittorService entry already
documented in the table.

[taco-paco]

I don't think we can modify user created intents implicitly like that. If user scheduled some intent simultaneously we shall fulfill it first and then execute UndelegationRequest intent

[snawaz]

I don't think we can modify user created intents implicitly like that. If user scheduled some intent simultaneously we shall fulfill it first and then execute UndelegationRequest intent

Why not? Softwares evolve. We only need to document the behavior.

I'm more interested in the technical reasons for why this shouldn't be implemented. Is there a technical constraint I'm missing? Otherwise, it feels like an artificial restriction.

[taco-paco]

I don't see how such behaviour is provable on L1 within a fraud proof, In classic L2 any state that surfaces is a result of transaction and hence is provable wether by ZK or challanged and resolved. Changes in intent in the following way is a result not of an ER transaction but of an L1 transaction. I see it is as very chalenging proving L1 state within L1 itself. It is a same problem we have with readonly cloned accounts.

Now the flow that is standart in this case is:

1. User request on L1(Base) a transaction to be ran on L2(ER)
2. Within a certain timeframe this TX is included on L2, e.g execute
3. User request is closed on state commitment as there's a proof that transaction was included

In our context tx - undelegation request.

Additionally, all of the current flows - AML undelegations, cloning and patching of intents(upcoming) is done via transaction. This goes ot of the standart way as say chainlink could also modify account in place but we schedule a transaction instead

[taco-paco]

This also introduces a race condition:
Say for same account

1. User on ER scheduled Commit Intent 1
2. User on ER Scheduled Commit&Undelegate intent 2.
3. User on base requested Undelegation
4. Intent one gets picked up and Undelegation added to it
5. Intent 2 becomes impossible to execute

Say Intent 2 had a critical action that expected to execute final transfers on Base and it also already settled final balances on ER. This Intent would never be able to execute now. With undelegation request being executed in separate tx it is not an issue as TXs attempting to schedule Intent for that account would fail as account marked as undelegated

All in all I'm not in favor of this flow as it circles around proper rules of intent scheduling that happens during TX execution

One of other edge cases: Intent may become impossible to fit in txs. User schedules an intent and we check during execution if it is possible to fit in 2 txs. We say it is and intent gets scheduled. We add UndelegateTask here which makes intent to large even with the split on 2 txs.