Skip to content

fix(reborn): harden activity identity invariants - #5356

Open
hanakannzashi wants to merge 3 commits into
mainfrom
codex/5219-activity-identity-invariants
Open

hanakannzashi wants to merge 3 commits into
mainfrom
codex/5219-activity-identity-invariants

Conversation

@hanakannzashi

Copy link
Copy Markdown
Contributor

Closes #5219.

Summary

  • Fail closed when a denied resume batch contains duplicate CapabilityActivityIds, before dispatching capabilities or emitting terminal activity milestones.
  • Thread optional blocked_activity_id through direct BlockRunRequest so direct block transitions can preserve parked activity identity in state and lifecycle events.
  • Guard canonical resume dispatch through a single-call construction helper so pending resumes are not silently batched with ordinary calls.

Tests

  • cargo check -p ironclaw_turns --tests
  • cargo check -p ironclaw_agent_loop --tests
  • cargo check -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-beta,libsql --tests
  • cargo test -p ironclaw_agent_loop capability_stage_denied_auth_resume_rejects_duplicate_activity_id_before_side_effects -- --nocapture
  • cargo test -p ironclaw_agent_loop resume_capability_input_rejects_batched_resume_calls -- --nocapture
  • cargo test -p ironclaw_turns lifecycle_publishing_store_publishes_blocked_event_activity_id_from_block_run -- --nocapture
  • cargo clippy -p ironclaw_agent_loop --all-targets -- -D warnings
  • cargo clippy -p ironclaw_turns --all-targets -- -D warnings
  • cargo clippy -p ironclaw_reborn_composition --features test-support,webui-v2-beta,slack-v2-host-beta,libsql --all-targets -- -D warnings

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5356 June 26, 2026 17:10 Destroyed
@github-actions github-actions Bot added size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds single-call validation for resume dispatch, rejects duplicate activity ids in denied-resume batches, and threads blocked_activity_id through blocked-run requests, storage, and regression coverage.

Changes

Activity identity and blocked-run metadata

Layer / File(s) Summary
Resume validation
crates/ironclaw_agent_loop/src/executor.rs, crates/ironclaw_agent_loop/src/executor/canonical.rs, crates/ironclaw_agent_loop/src/executor/capabilities.rs, crates/ironclaw_agent_loop/src/executor/tests.rs
resume_capability_input now requires exactly one capability call, the canonical resume branch uses it, denied-resume handling rejects duplicate activity_ids, and tests cover both planner-contract failures.
Blocked activity id propagation
crates/ironclaw_turns/src/runner.rs, crates/ironclaw_turns/src/memory/mod.rs, crates/ironclaw_reborn_composition/src/factory/auth_tests.rs, crates/ironclaw_reborn_composition/src/runtime.rs, crates/ironclaw_reborn_composition/src/runtime/tests/auth_interaction.rs, crates/ironclaw_turns/tests/*, crates/ironclaw_loop_support/tests/turn_event_publisher_contract.rs, crates/ironclaw_product_workflow/src/auth_continuation.rs
BlockRunRequest gains optional blocked_activity_id, blocked runs persist it, and fixtures/tests set or assert the field across approval, auth, resume, and blocked-event flows.

Sequence Diagram(s)

sequenceDiagram
  participant AgentLoopExecutor
  participant CapabilityStage
  participant Host
  AgentLoopExecutor->>CapabilityStage: process_resume_capability_calls(calls)
  CapabilityStage->>CapabilityStage: resume_capability_input validates one call
  CapabilityStage->>CapabilityStage: short_circuit_denied_resume checks unique activity_id values
  CapabilityStage->>Host: process capability input when validation passes
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • nearai/ironclaw#4944 — Shares the denied-resume short-circuit path and the same CapabilityStage activity-id validation area.
  • nearai/ironclaw#4954 — Touches the same denied-resume handling in executor/capabilities.rs.
  • nearai/ironclaw#5145 — Establishes the CapabilityActivityId execution-identity work that this PR continues.

Suggested reviewers

  • think-in-universe
  • henrypark133

Poem

One resume call, one path to trace,
Duplicates denied in bounded space.
A blocked id now rides along,
Through state and tests, both clear and strong.
Rust keeps the turn loop crisp and bright.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the change summary and tests, but omits most required template sections like Change Type, Security Impact, and Rollback Plan. Fill in the full template: Change Type, Linked Issue, Validation checklist, Security Impact, Blast Radius, Rollback Plan, and Review Follow-Through.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The diff matches #5219 by rejecting duplicate denied-resume activity IDs, preserving blocked_activity_id, and enforcing single-call resume dispatch.
Out of Scope Changes check ✅ Passed The changes stay within the issue scope and are limited to invariant hardening plus matching test updates.
Title check ✅ Passed The title matches the PR’s main change and follows Conventional Commits style.

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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_agent_loop/src/executor/tests.rs`:
- Around line 6779-6809: The current test only covers resume_capability_input
directly, but the guard is enforced through DefaultExecutorPipeline::execute on
the real resume path. Add a caller-level regression that drives the actual
resume dispatch entrypoint with multiple CapabilityCallCandidate values, then
assert the PlannerContract error and verify MockHost receives no invocation.
Keep the existing helper-level check if needed, but make sure the new test
exercises DefaultExecutorPipeline::execute or the surrounding resume flow so the
side-effect gate is covered end-to-end.
🪄 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: 7e46a06a-54d7-4774-b41f-ec7069f0bd09

📥 Commits

Reviewing files that changed from the base of the PR and between 667e3cc and 4f37ae0.

📒 Files selected for processing (12)
  • crates/ironclaw_agent_loop/src/executor.rs
  • crates/ironclaw_agent_loop/src/executor/canonical.rs
  • crates/ironclaw_agent_loop/src/executor/capabilities.rs
  • crates/ironclaw_agent_loop/src/executor/tests.rs
  • crates/ironclaw_reborn_composition/src/factory/auth_tests.rs
  • crates/ironclaw_reborn_composition/src/runtime.rs
  • crates/ironclaw_reborn_composition/src/runtime/tests/auth_interaction.rs
  • crates/ironclaw_turns/src/memory/mod.rs
  • crates/ironclaw_turns/src/runner.rs
  • crates/ironclaw_turns/tests/per_inbound_type_concurrency_cap.rs
  • crates/ironclaw_turns/tests/per_user_concurrency_cap.rs
  • crates/ironclaw_turns/tests/turn_coordinator_contract.rs

Comment thread crates/ironclaw_agent_loop/src/executor/tests.rs
@railway-app

railway-app Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5356 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jun 29, 2026 at 7:06 am

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5356 June 27, 2026 11:23 Destroyed
@think-in-universe think-in-universe changed the title Harden activity identity invariants fix: harden activity identity invariants Jun 29, 2026
@think-in-universe think-in-universe changed the title fix: harden activity identity invariants fix(reborn): harden activity identity invariants Jun 29, 2026

This branch was successfully deployed

1 active deployment
ironclaw-ci-preview / ironclaw-pr-5356 — e43c719a Deployed Jun 29, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-up: harden activity identity invariants after gate lifecycle refactor

2 participants