Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions .claude/rules/gateway-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
---
paths:
- "src/**"
- "crates/**"
---
# Gateway Events — Single Source of Truth

Every `AppEvent` reaching the SSE/WS stream must come from a **typed
source log**, or be on a small **transport-only allowlist**. Direct
`sse.broadcast(...)` / `sse.broadcast_for_user(...)` calls from tools,
handlers, or extension managers are the root cause of the UI state
drift class tracked by #2792 — the stream and the replayable source
end up telling different stories.

This is the Phase 1 rule of the gateway state-convergence epic.

## Why

When `AppEvent` has producers outside the projection layer, those
producers become a second source of truth. On SSE reconnect, replay
from the engine event log can't reconstruct them (they were never
logged). On tab focus, reconciliation against a GET endpoint can't
confirm them (no persisted state backs them). Four recent bugs
(#2654, #2534, #2731, #2079) share this shape: broadcast emitted,
backend state unchanged, UI diverges.

## Source logs

Every `AppEvent` projects from exactly one of:

| Source log | Projection function | Typical variants |
|---|---|---|
| `ironclaw_engine::EventKind` | `src/bridge/router.rs::thread_event_to_app_events` | Turn progression, tool execution, gates, leases, child threads, skills |
| Sandbox `JobEvent` | `src/worker/job.rs` (currently inline; extract under #2792 Phase 1 PR 3) | `JobStarted`, `JobMessage`, `JobToolUse`, `JobToolResult`, `JobStatus`, `JobResult` |
| Channel-lifecycle logs | `src/channels/web/features/oauth/`, `features/pairing/`, `features/extensions/`, `extensions/manager.rs` | `OnboardingState`, `ExtensionStatus` |

## Transport-only allowlist

A small number of `AppEvent` variants don't project from anything
because they have no state backing them. These are documented
exceptions, not a loophole for new state:

- `Heartbeat` — SSE keepalive, no payload, no state
- `StreamChunk` — LLM token streaming, pre-step-completion by design; formalizing into `EventKind` would pollute the durable log with token-level noise

New `AppEvent` variants that claim "transport-only" status require
review sign-off and an entry in this table.

## The rule

**No call to `SseManager::broadcast` / `SseManager::broadcast_for_user`
is allowed outside:**

1. The projection dispatcher loop that consumes one of the three source
logs above, **or**
2. A line annotated with `// projection-exempt: <category>, <detail>`.

## Annotation format

```rust
state.sse.broadcast_for_user(user_id, event); // projection-exempt: channel-lifecycle, extension activation
```

The `<category>` must name either:

- A source log — `bridge dispatcher`, `sandbox JobEvent`, `channel-lifecycle` — plus a short detail.
- A transport-only allowlist entry — `transport-only, heartbeat` or `transport-only, stream_chunk`.
- A scheduled migration — `migrate in #NNNN` where the issue tracks moving the emit into a source log.

An unnamed category (`// projection-exempt: legacy`) is not sufficient.
Either the site is legitimately exempt and the category explains why,
or it's a violation and should be migrated.

## Enforcement

Check #9 in `scripts/pre-commit-safety.sh` (label: `PROJECTION`) flags
added lines that call `SseManager::broadcast` or
`SseManager::broadcast_for_user` without a `// projection-exempt:
<category>, <detail>` annotation on the same line. The comma is
required — the check rejects bare `// projection-exempt: legacy`. Lines
in `#[cfg(test)] mod tests` blocks and under `tests/` are skipped via
the shared `strip_test_mod_lines` filter.

The matcher covers two call-site shapes:

1. Any-receiver `.broadcast_for_user(...)` — the method is defined
only on `SseManager` (`src/channels/web/platform/sse.rs`), so
matching the method name alone catches same-line receivers
(`state.sse.broadcast_for_user(...)`), rustfmt wraps
(`state\n .sse\n .broadcast_for_user(...)`), and any other
receiver name (`manager.broadcast_for_user(...)`) without
false-positive risk.
2. `<word-boundary>sse.broadcast(...)` — the single-name `broadcast`
is shared with the `Channel` trait, so this arm is deliberately
narrower and only fires when the receiver is literally named
`sse`. The boundary uses `(^|[^[:alnum:]_])` rather than `\b` so
the check is portable across GNU and BSD `grep -E`.

## Not covered by this rule

- **`Channel::broadcast` on the `Channel` trait.** Different method,
different trait, different semantics (delivery to a specific channel
endpoint like Telegram, not to SSE subscribers). The `Channel` trait
has its own invariants in `src/channels/`.
- **Non-SSE `broadcast` methods.** If you're broadcasting on a
`tokio::sync::broadcast::Sender` directly, you're below the
`AppEvent` abstraction; the rule doesn't apply.

## References

- Epic: #2792 — Gateway state convergence
- Coverage: #2654 — Engine→AppEvent bridge gaps
- Incidents: #2079 (SSE ordering), #2534 (stale approval), #2731 (Telegram thread split)
- Rule cluster: `.claude/rules/types.md` for wire-stable enums; `.claude/rules/tools.md` for the parallel "everything goes through tools" rule this mirrors
123 changes: 123 additions & 0 deletions crates/ironclaw_common/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,100 @@ pub enum AppEvent {
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},

/// A capability lease was granted to a thread.
///
/// Bridged from engine `EventKind::LeaseGranted`. Security-visible:
/// capability grants should be auditable in the UI.
#[serde(rename = "lease_granted")]
LeaseGranted {
lease_id: String,
capability_name: String,
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},

/// A capability lease was explicitly revoked.
///
/// Bridged from engine `EventKind::LeaseRevoked`. `reason` is the
/// engine's revocation message, surfaced so users can tell a
/// revocation apart from an expiry.
#[serde(rename = "lease_revoked")]
LeaseRevoked {
lease_id: String,
reason: String,
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},

/// A capability lease reached its TTL and expired.
///
/// Bridged from engine `EventKind::LeaseExpired`. Without this, tools
/// begin failing after a lease's TTL with no visible explanation.
#[serde(rename = "lease_expired")]
LeaseExpired {
lease_id: String,
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},

/// Background self-improvement lifecycle event.
///
/// Bridged from engine `EventKind::SelfImprovement{Started,Complete,Failed}`.
/// The three engine variants collapse into one wire event with a
/// nested `SelfImprovementPhase` carrying per-phase data — consumers
/// need one handler, and the compiler enforces that phase-specific
/// fields travel with their phase (no `Option<T>` sentinels that
/// claim "maybe present" when the phase excludes them).
///
/// Wire shape uses `#[serde(flatten)]` + the phase enum's
/// `#[serde(tag = "phase")]`, so the JSON payload is flat:
/// `{"type": "self_improvement", "phase": "complete", "prompt_updated": true, ...}`.
#[serde(rename = "self_improvement")]
SelfImprovement {
#[serde(flatten)]
phase: SelfImprovementPhase,
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},

/// Orchestrator version was rolled back.
///
/// Bridged from engine `EventKind::OrchestratorRollback`. Operator-
/// facing; surfaces the from/to versions so failures after an
/// upgrade are correlatable with the rollback point.
#[serde(rename = "orchestrator_rollback")]
OrchestratorRollback {
from_version: u64,
to_version: u64,
reason: String,
#[serde(skip_serializing_if = "Option::is_none")]
thread_id: Option<String>,
},
}

/// Phase data for `AppEvent::SelfImprovement`.
///
/// Mirrors engine `EventKind::SelfImprovement{Started,Complete,Failed}`
/// as a single typed wire enum. Variant-specific fields are part of the
/// variant, not optional fields on the outer event — per
/// `.claude/rules/types.md` the compiler should reject a `Failed` value
/// carrying `prompt_updated`, which an `Option`-field approach cannot.
///
/// Serialized with an internally-tagged `phase` discriminator; the
/// outer `AppEvent::SelfImprovement` flattens this into its payload so
/// the wire shape stays a single flat JSON object.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "phase", rename_all = "snake_case")]
pub enum SelfImprovementPhase {
Started,
Complete {
prompt_updated: bool,
patterns_added: usize,
},
Failed {
error: String,
},
}

/// Wire-side mirror of `ironclaw_engine::CodeExecutionFailure`.
Expand Down Expand Up @@ -603,6 +697,11 @@ impl AppEvent {
Self::MissionThreadSpawned { .. } => "mission_thread_spawned",
Self::PlanUpdate { .. } => "plan_update",
Self::CodeExecutionFailed { .. } => "code_execution_failed",
Self::LeaseGranted { .. } => "lease_granted",
Self::LeaseRevoked { .. } => "lease_revoked",
Self::LeaseExpired { .. } => "lease_expired",
Self::SelfImprovement { .. } => "self_improvement",
Self::OrchestratorRollback { .. } => "orchestrator_rollback",
}
}

Expand Down Expand Up @@ -810,6 +909,30 @@ mod tests {
code_hash: None,
thread_id: None,
},
AppEvent::LeaseGranted {
lease_id: String::new(),
capability_name: String::new(),
thread_id: None,
},
AppEvent::LeaseRevoked {
lease_id: String::new(),
reason: String::new(),
thread_id: None,
},
AppEvent::LeaseExpired {
lease_id: String::new(),
thread_id: None,
},
AppEvent::SelfImprovement {
phase: SelfImprovementPhase::Started,
thread_id: None,
},
AppEvent::OrchestratorRollback {
from_version: 0,
to_version: 0,
reason: String::new(),
thread_id: None,
},
];

for variant in &variants {
Expand Down
2 changes: 1 addition & 1 deletion crates/ironclaw_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod util;

pub use event::{
AppEvent, CodeExecutionFailureCategory, JobResultStatus, JobResultStatusParseError,
OnboardingStateDto, PlanStepDto, ToolDecisionDto,
OnboardingStateDto, PlanStepDto, SelfImprovementPhase, ToolDecisionDto,
};
pub use identity::{
CredentialName, ExtensionName, ExternalThreadId, ExternalThreadIdError, IdentityError,
Expand Down
6 changes: 6 additions & 0 deletions crates/ironclaw_engine/src/types/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,9 @@ impl fmt::Display for ProjectId {
write!(f, "{}", self.0)
}
}

impl fmt::Display for crate::types::capability::LeaseId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}
4 changes: 4 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ ignore = [
"RUSTSEC-2025-0111",
# rustls-webpki advisories — 0.102.8 remains pinned by a libsql 0.6.0 transitive dep
# (via rustls 0.22 → hyper-rustls 0.25); keep ignored until that pin is gone.
# RUSTSEC-2026-0104: panic on empty `onlySomeReasons` BIT STRING during CRL parsing.
# We do not use CRLs, and the advisory explicitly notes apps that don't parse CRLs
# are unaffected. Same transitive pin as 0049/0098/0099 — tracked with them.
"RUSTSEC-2026-0049",
"RUSTSEC-2026-0098",
"RUSTSEC-2026-0099",
"RUSTSEC-2026-0104",
# rand unsoundness with custom logger calling rand::rng() during reseed — we don't use this pattern;
# revisit/remove by 2026-06-30, or when transitive deps (tower, nanoid, phf_generator) release rand ≥0.9.3 compat
"RUSTSEC-2026-0097",
Expand Down
54 changes: 51 additions & 3 deletions scripts/pre-commit-safety.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
# 6. .unwrap(), .expect(), assert!() in production code (panics)
# 7. Gateway/CLI handlers bypassing ToolDispatcher (must go through tools)
# 8. CredentialName referenced in web-layer code (wrong identity at boundary)
# 9. SSE broadcast emitted outside the engine→AppEvent projection bridge
#
# Also runs check-i18n-parity.sh when crates/ironclaw_gateway/static/i18n/*.js
# files are staged, to ensure every language pack has the same key set.
#
# Suppress individual lines with an inline "// safety: <reason>" comment.
# For check #7, use "// dispatch-exempt: <reason>" instead.
# For check #8, use "// web-identity-exempt: <reason>" instead.
# For check #9, use "// projection-exempt: <category>, <detail>" instead.

set -euo pipefail

Expand Down Expand Up @@ -342,7 +344,7 @@ fi
if [ -n "$DISPATCH_DIFF" ]; then
DISPATCH_HITS=$(echo "$DISPATCH_DIFF" | grep -nE '^\+' \
| grep -E 'state\.(store|workspace|workspace_pool|extension_manager|skill_registry|session_manager)\.' \
| grep -vE '// dispatch-exempt:|// safety:|^\+\+\+' \
| grep -vE '// dispatch-exempt:|// safety:|:\+\+\+ ' \
| head -5 || true)
if [ -n "$DISPATCH_HITS" ]; then
warn "DISPATCH" "Handler directly touches state.{store,workspace,extension_manager,skill_registry,session_manager}. Route through ToolDispatcher::dispatch() instead. See .claude/rules/tools.md."
Expand Down Expand Up @@ -370,21 +372,67 @@ if [ -n "$WEB_IDENTITY_DIFF" ]; then
# Strip lines inside `#[cfg(test)] mod tests` blocks using the same
# precomputed boundaries used for other prod-only checks.
WEB_IDENTITY_PROD=$(printf '%s\n' "$WEB_IDENTITY_DIFF" | strip_test_mod_lines)
# `(^|[^[:alnum:]_])CredentialName([^[:alnum:]_]|$)` is a portable
# word boundary; `grep -E`'s `\b` is a GNU extension and is not
# recognised by BSD grep.
WEB_IDENTITY_HITS=$(echo "$WEB_IDENTITY_PROD" | grep -nE '^\+' \
| grep -E '\bCredentialName\b' \
| grep -vE '// web-identity-exempt:|// safety:|^\+\+\+' \
| grep -E '(^|[^[:alnum:]_])CredentialName([^[:alnum:]_]|$)' \
| grep -vE '// web-identity-exempt:|// safety:|:\+\+\+ ' \
| head -5 || true)
if [ -n "$WEB_IDENTITY_HITS" ]; then
warn "CREDNAME" "\`CredentialName\` referenced in src/channels/web/** — web code takes \`ExtensionName\`; credential identity stays backend-side. Push the mapping into bridge::auth_manager or annotate with '// web-identity-exempt: <reason>'."
echo "$WEB_IDENTITY_HITS" | sed 's/^/ /'
fi
fi

# 9. SSE `AppEvent` broadcast outside the engine→AppEvent projection bridge.
# Every `AppEvent` that hits the SSE/WS stream should project from a typed
# source log (`ironclaw_engine::EventKind`, `JobEvent`, channel-lifecycle)
# or belong to the documented transport-only allowlist. Direct
# `sse.broadcast(...)` / `sse.broadcast_for_user(...)` calls from tools,
# handlers, or extension managers drift the UI stream out of alignment
# with the replayable source, which is the root cause of the state
# desync class tracked by #2792. See `.claude/rules/gateway-events.md`.
#
# Annotation format: `// projection-exempt: <category>, <detail>` — the
# category names the source log (`bridge dispatcher`, `channel-lifecycle`,
# `sandbox JobEvent`, `legacy v1 auth`) or the transport-only allowlist
# (`transport-only, heartbeat`). The comma is required — an unnamed
# `// projection-exempt: legacy` does not suppress the check.
#
# Two match patterns:
# 1. `*.broadcast_for_user(...)` on any receiver — `broadcast_for_user`
# is unique to `SseManager` (see `src/channels/web/platform/sse.rs`),
# so matching the method name alone catches both same-line
# receivers (`state.sse.broadcast_for_user(...)`) and rustfmt
# wraps (`state\n .sse\n .broadcast_for_user(...)`) without
# risk of false positives from other types.
# 2. `<word-boundary>sse.broadcast(...)` — the single-name `.broadcast(`
# on its own line can be the `Channel` trait method, so this arm
# is deliberately narrower and anchors on an `sse` receiver.
# `(^|[^[:alnum:]_])sse\.` is a portable boundary; `grep -E`'s
# `\b` is a GNU extension and is not recognised by BSD grep, so
# we avoid it here.
# Suppression regex requires a non-empty detail after the comma:
# `[^,]+,[[:space:]]*[^[:space:]]` — `// projection-exempt: foo,`
# (empty detail) does NOT exempt; `// projection-exempt: foo, bar`
# does. This matches the documented contract in
# `.claude/rules/gateway-events.md`.
PROJECTION_HITS=$(echo "$DIFF_OUTPUT_NO_TESTS" | grep -nE '^\+' \
| grep -E '(\.broadcast_for_user|(^|[^[:alnum:]_])sse\.broadcast)[[:space:]]*\(' \
| grep -vE '// projection-exempt: [^,]+,[[:space:]]*[^[:space:]]|// safety:|:\+\+\+ ' \
| head -5 || true)
if [ -n "$PROJECTION_HITS" ]; then
warn "PROJECTION" "Direct SSE broadcast outside the engine→AppEvent bridge. Route through \`thread_event_to_app_events\` in \`src/bridge/router.rs\` (project from a typed source log) or annotate with '// projection-exempt: <category>, <detail>'. See .claude/rules/gateway-events.md."
echo "$PROJECTION_HITS" | sed 's/^/ /'
fi

if [ "$WARNINGS" -gt 0 ]; then
echo ""
echo "Found $WARNINGS potential issue(s). Fix them or add '// safety: <reason>' to suppress."
echo "(For DISPATCH warnings, use '// dispatch-exempt: <reason>' instead.)"
echo "(For CREDNAME warnings, use '// web-identity-exempt: <reason>' instead.)"
echo "(For PROJECTION warnings, use '// projection-exempt: <category>, <detail>' instead.)"
echo ""
exit 1
fi
Loading
Loading