Preserve paused leases across engine auth resume (#2631)

* Preserve paused leases across engine auth resume

* fix(review): validate paused_lease snapshot at gate resume

Addresses PR #2631 review comments from Copilot:

1. **Snapshot used without validation** (src/bridge/router.rs:684 orig):
   `pending.paused_lease.clone()` was used directly to resume a gated
   action. A gate can sit in the pending-gate store for hours or across
   process restarts; during that window the original lease may have
   been revoked, expired, or the pending record could have drifted off
   its original thread.

   Extract `snapshot_lease_still_valid` + `resume_lease_for_pending_gate`
   helpers. The snapshot must pass four checks before use:
     - `lease.thread_id == pending.thread_id`
     - `granted_actions.covers(&pending.action_name)`
     - `!revoked`
     - `expires_at` is unset or in the future

   If any check fails, fall through to `LeaseManager::find_lease_for_action`
   (the normal path). Matches the reviewer's suggestion to avoid silently
   resuming a stale snapshot; still prefers the snapshot when valid so
   the original bug (no active lease at resume after restart) stays
   fixed.

2. **No router-level regression test** for the snapshot-vs-fallback
   decision. Six new libsql-free tests in `bridge::router::tests`:
     - `resume_lease_prefers_snapshot_even_when_lease_manager_empty` —
       reproduces the original bug; snapshot must carry the resume.
     - `resume_lease_rejects_revoked_snapshot_and_falls_back`
     - `resume_lease_rejects_expired_snapshot_and_falls_back`
     - `resume_lease_rejects_snapshot_with_wrong_thread_id`
     - `resume_lease_rejects_snapshot_missing_action_coverage`
     - `resume_lease_returns_none_when_no_snapshot_and_no_active_lease`

Verified: `cargo fmt`, `cargo clippy --all --benches --tests --examples
--all-features` (0 warnings), `cargo test -p ironclaw_engine` (435
passed), `cargo test -p ironclaw --lib` (5182 passed, +6 new).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: collapse assert!(matches!()) per rustc 1.95 rustfmt

CI rustfmt (nightly/stable 1.95.0) wants the `assert!(matches!())` in
`orchestrator.rs::parse_outcome_gate_paused` collapsed to fewer lines.
Local rustfmt 1.94 was happy with the expanded form; matching CI to
unblock the fmt check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Henry Park
2026-04-19 09:45:49 -07:00
committed by GitHub
parent cad5e50f10
commit 64193474dd
15 changed files with 366 additions and 17 deletions

View File

@@ -219,6 +219,7 @@ impl EffectExecutor for InstallThenAliasEffects {
instructions: "Authenticate GitHub".into(),
auth_url: None,
}),
paused_lease: None,
resume_output: None,
});
}
@@ -283,6 +284,7 @@ impl EffectExecutor for GateMockEffects {
call_id: "call_gate_1".into(),
parameters: Box::new(parameters),
resume_kind: Box::new(ResumeKind::Approval { allow_always: true }),
paused_lease: None,
resume_output: None,
});
}
@@ -298,6 +300,7 @@ impl EffectExecutor for GateMockEffects {
instructions: "Authenticate your Notion workspace".into(),
auth_url: None,
}),
paused_lease: None,
resume_output: None,
});
}
@@ -311,6 +314,7 @@ impl EffectExecutor for GateMockEffects {
call_id: "call_gate_1".into(),
parameters: Box::new(parameters),
resume_kind: Box::new(ResumeKind::Approval { allow_always: true }),
paused_lease: None,
resume_output: None,
});
}
@@ -327,6 +331,7 @@ impl EffectExecutor for GateMockEffects {
instructions: "Provide your API key".into(),
auth_url: None,
}),
paused_lease: None,
resume_output: None,
});
}
@@ -660,6 +665,7 @@ fn sample_pending_gate(
created_at: Utc::now(),
expires_at: Utc::now() + chrono::Duration::minutes(30),
original_message: None,
paused_lease: None,
resume_output: None,
approval_already_granted: false,
}