ironclaw

mirror of https://github.com/nearai/ironclaw.git synced 2026-09-02 23:56:24 +08:00

Files

Illia Polosukhin c559a58810 feat(bridge): project 7 more engine events to AppEvents (#2844 )

* feat(bridge): project 7 more engine events to AppEvents

Second slice of #2654 bridge-coverage under #2792 Phase 1. Builds on
#2797 (StepFailed, ChildCompleted, CodeExecutionFailed) by closing the
remaining cheap `EventKind` drops:

| `EventKind` | `AppEvent` |
|---|---|
| `LeaseGranted { lease_id, capability_name }` | new `LeaseGranted` |
| `LeaseRevoked { lease_id, reason }` | new `LeaseRevoked` |
| `LeaseExpired { lease_id }` | new `LeaseExpired` |
| `SelfImprovementStarted` | new `SelfImprovement { phase: Started, .. }` |
| `SelfImprovementComplete { prompt_updated, patterns_added }` | `SelfImprovement { phase: Complete, prompt_updated, patterns_added, .. }` |
| `SelfImprovementFailed { error }` | `SelfImprovement { phase: Failed, error, .. }` |
| `OrchestratorRollback { from, to, reason }` | new `OrchestratorRollback` |

The three engine `SelfImprovement*` variants collapse into one wire
event with a `SelfImprovementPhase` discriminator — consumers need one
handler, variant-specific data is conveyed via optional phase-scoped
fields. Following the types.md "Wire-stable enums" pattern — the phase
enum is snake_case serde, not a stringly-typed `status` field.

The lease events are security-visible: capability grants, revocations,
and expiries should be auditable on the UI stream. `LeaseExpired` in
particular closes a "tools start failing after TTL with no visible
reason" gap.

Also:

- Adds `impl fmt::Display for LeaseId` in the engine alongside the
  existing `ThreadId` / `ProjectId` impls. The bridge code stringifies
  `LeaseId` for the wire; missing `Display` blocked the first compile.
- Cleans up a stray doc-comment misplacement from #2797 where the
  `thread_event_to_app_events` docstring was attached to
  `code_execution_category_to_wire`.

Regression tests mirror the #2797 pattern — one per representative arm
(lease grant for the lease family, self-improvement complete for the
richest phase, orchestrator rollback). The two remaining lease
variants and two remaining self-improvement phases are covered by the
existing `event_type_matches_serde_type_field` drift-catch test.

Approval pair (`EventKind::ApprovalRequested` / `ApprovalReceived`) is
still deferred — they need to land together with the gate-manager
migration in Phase 1 PR 3 to avoid duplicate-emit with the direct
`GateRequired` / `GateResolved` broadcasts.

Refs: #2792, #2654

* refactor(bridge): typed SelfImprovementPhase + exhaustive match

Addresses two Gemini review comments on #2844.

**1. `SelfImprovementPhase` as a typed internally-tagged enum.**

Previously the `AppEvent::SelfImprovement` variant carried three
`Option<T>` fields (`prompt_updated`, `patterns_added`, `error`), only
some of which were populated per phase. Per `.claude/rules/types.md` —
and the reviewer's note — this is an `Option`-that-can-lie pattern the
type system should rule out. Phase-specific data now lives on the
variant:

```rust
enum SelfImprovementPhase {
    Started,
    Complete { prompt_updated: bool, patterns_added: usize },
    Failed { error: String },
}
```

Wire shape is preserved via `#[serde(tag = "phase")]` on the enum and
`#[serde(flatten)]` on the `AppEvent::SelfImprovement.phase` field —
JSON still looks like a flat object:
`{"type": "self_improvement", "phase": "complete", "prompt_updated": true, ...}`.

**2. Exhaustive `thread_event_to_app_events` match.**

Dropped the `_ => vec![]` wildcard in favour of explicit arms for
every `EventKind` variant. Deferred-bridge variants get `vec![]` with
a comment naming the migration plan:

- `ApprovalRequested` / `ApprovalReceived` → waiting on the gate
  manager migration in #2792 Phase 1 PR 3 to avoid duplicate-emit with
  the existing direct `GateRequired` / `GateResolved` broadcasts.
- `Unknown` → forward-compat catch-all in the engine enum; nothing
  useful to project from a variant written by a newer binary during a
  rolling deploy.

New engine variants now fail the bridge to compile, which is exactly
what the state-convergence epic (#2792) needs — no more silent drops.

Refs: #2792, #2844 review

* fix(bridge): sanitize OrchestratorRollback.reason before SSE projection

`EventKind::OrchestratorRollback.reason` originates from
`format!("execution failed: {e}")` in
`crates/ironclaw_engine/src/executor/loop_engine.rs:327`, where
`e: EngineError`. Variants like `Store { reason }` and
`Llm { reason }` render DB connection strings, file paths, and raw
upstream HTTP bodies — all of which reached every authenticated SSE
consumer verbatim through the new `AppEvent::OrchestratorRollback`
projection.

Route the reason through a new `user_facing_rollback_reason`
classifier that maps the existing `FailureCategory` taxonomy to
short operator-facing messages (`"LLM provider unavailable"`,
`"execution failed"`, etc.). The raw text still lives in the
`debug!` log for operator triage, matching the pattern already
used for `AppEvent::Error`.

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

* chore(deny): ignore RUSTSEC-2026-0104 (rustls-webpki CRL panic)

Same transitive pin as 0049/0098/0099 — rustls-webpki 0.102.8 is
held by libsql 0.6.0 → rustls 0.22 → hyper-rustls 0.25. The
advisory explicitly notes that applications not parsing CRLs are
unaffected; we do not parse CRLs.

[skip-regression-check] — deny.toml-only config change.

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

* test(bridge): cover 4 new engine→AppEvent arms; sharpen rollback test

Review follow-ups on PR #2844:

- Add unit tests for `LeaseRevoked`, `LeaseExpired`,
  `SelfImprovementStarted`, and `SelfImprovementFailed` bridge arms
  — only `LeaseGranted` / `SelfImprovementComplete` /
  `OrchestratorRollback` had coverage before, leaving four new
  projections untested.
- Rewrite `rollback_reason_drops_engine_error_detail` to drive the
  sanitiser with two unrelated leaky inputs and assert identical
  outputs (`execution failed`). The load-bearing check is
  input-independence; `!contains` probes remain as sentinel sniffs
  for the specific leak shapes. Avoids classifier-triggering tokens
  (no `upstream`, no `http 5xx`) so both inputs fall through to
  `Unknown`.
- Reword the `ApprovalRequested` / `ApprovalReceived` comment: they
  are temporarily suppressed pending the gate-manager migration, not
  permanently dropped. The bridge will eventually map them here.

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

---------

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