Skip to content

feat(output): emit warning and compact events on JSONL stdout stream#215

Merged
emal-avala merged 1 commit intomainfrom
feat/jsonl-warning-compact-events
Apr 23, 2026
Merged

feat(output): emit warning and compact events on JSONL stdout stream#215
emal-avala merged 1 commit intomainfrom
feat/jsonl-warning-compact-events

Conversation

@emal-avala
Copy link
Copy Markdown
Member

Summary

`JsonStreamSink::on_warning` and `on_compact` only wrote to stderr as plain text, so automation consumers parsing the JSONL stream from stdout silently lost signal for budget warnings, rate-limit backoffs, and autocompaction events. A run that hit a cost-limit or compacted twice looked identical on the wire to a clean one.

This PR adds two new `Event` variants — `Warning { message, turn }` and `Compact { freed_tokens, turn }` — that fire on stdout in parallel with the existing stderr writes. Stderr output is unchanged; JSONL is purely additive, so downstream tooling only sees more structure.

Both variants use the existing snake_case tag convention and carry the current turn number (read under the same lock `on_turn_start` writes to), so consumers can correlate them with the surrounding `TurnComplete` envelope.

Test plan

  • `cargo clippy -p agent-code --tests --no-deps -- -D warnings` — clean
  • `cargo test -p agent-code --bin agent output::` — 11 pass (8 prior + 3 new)
  • 3 new tests: warning envelope shape, compact envelope shape, turn-state correlation
  • Existing `all_events_are_single_line_json` extended to cover both new variants (with a multiline message to confirm serde escapes newlines)

JsonStreamSink::on_warning and on_compact wrote only to stderr as plain
human-readable lines. Any automation pipeline that consumed the JSONL
stream from stdout (CI runners, audit logs, monitoring) silently lost
signal for budget-approaching warnings, rate-limit backoffs, and
autocompaction events. That's a real observability gap — consumers
couldn't tell a 4x-cost session apart from a clean one without
re-parsing stderr.

Adds two new Event variants:

- Warning { message, turn } — fired in parallel with the stderr
  write, so humans still see the message and automation gets a
  structured line.
- Compact { freed_tokens, turn } — same dual-sink pattern.

Both events use the existing snake_case type tag convention and carry
the current turn number pulled under the same lock that on_turn_start
writes to. Stderr output is unchanged; the JSONL line is purely
additive.

3 new tests cover: warning envelope shape, compact envelope shape, and
turn-state correlation. The existing single-line-json check is
extended to cover both new variants (including a multiline message to
confirm serde still escapes newlines).
@emal-avala emal-avala merged commit 4dc5ac7 into main Apr 23, 2026
14 checks passed
@emal-avala emal-avala deleted the feat/jsonl-warning-compact-events branch April 23, 2026 18:30
emal-avala added a commit that referenced this pull request Apr 23, 2026
Adds a `Notification` hook event. Previously the `sink.on_warning`
path only reached stderr (and the JSONL `warning` event as of #215);
users couldn't wire a desktop notifier, Slack DM, or SMS to those
moments because there was no hook.

Context payload:

  {
    "session_id": "...",
    "message": "...",
    "title": "..." | null,
    "notification_type": "budget_limit" | "context_full" | ...
  }

`notification_type` lets hook configs filter so a Slack-on-budget
hook doesn't also fire on routine context warnings.

Wiring:
- HookEvent::Notification added with snake_case serde
- fire_notification_hooks(message, title, notification_type) async
  method on QueryEngine
- Wired at the 3 most critical user-attention sites in run_turn:
  - BudgetDecision::Stop          -> "budget_limit"
  - BudgetDecision::ContinueWithWarning -> "budget_warning"
  - Context window is_blocking    -> "context_full"
- Non-blocking context % warnings and per-turn rate-limit fallbacks
  stay warning-only — routing every retry would be noise
- HOOK_EVENT_CATALOG, format_hook_event, parse_hook_event updated
- hooks/mod.rs module docs updated

Tests: 3 new
- Schema serde round-trip (notification <-> "notification")
- run_hooks dispatches for Notification (async tokio)
- parse_hook_event accepts "notification" / "Notification"
emal-avala added a commit that referenced this pull request Apr 23, 2026
Adds a `Notification` hook event. Previously the `sink.on_warning`
path only reached stderr (and the JSONL `warning` event as of #215);
users couldn't wire a desktop notifier, Slack DM, or SMS to those
moments because there was no hook.

Context payload:

  {
    "session_id": "...",
    "message": "...",
    "title": "..." | null,
    "notification_type": "budget_limit" | "context_full" | ...
  }

`notification_type` lets hook configs filter so a Slack-on-budget
hook doesn't also fire on routine context warnings.

Wiring:
- HookEvent::Notification added with snake_case serde
- fire_notification_hooks(message, title, notification_type) async
  method on QueryEngine
- Wired at the 3 most critical user-attention sites in run_turn:
  - BudgetDecision::Stop          -> "budget_limit"
  - BudgetDecision::ContinueWithWarning -> "budget_warning"
  - Context window is_blocking    -> "context_full"
- Non-blocking context % warnings and per-turn rate-limit fallbacks
  stay warning-only — routing every retry would be noise
- HOOK_EVENT_CATALOG, format_hook_event, parse_hook_event updated
- hooks/mod.rs module docs updated

Tests: 3 new
- Schema serde round-trip (notification <-> "notification")
- run_hooks dispatches for Notification (async tokio)
- parse_hook_event accepts "notification" / "Notification"
emal-avala added a commit that referenced this pull request Apr 23, 2026
Adds a `Notification` hook event. Previously the `sink.on_warning`
path only reached stderr (and the JSONL `warning` event as of #215);
users couldn't wire a desktop notifier, Slack DM, or SMS to those
moments because there was no hook.

Context payload:

  {
    "session_id": "...",
    "message": "...",
    "title": "..." | null,
    "notification_type": "budget_limit" | "context_full" | ...
  }

`notification_type` lets hook configs filter so a Slack-on-budget
hook doesn't also fire on routine context warnings.

Wiring:
- HookEvent::Notification added with snake_case serde
- fire_notification_hooks(message, title, notification_type) async
  method on QueryEngine
- Wired at the 3 most critical user-attention sites in run_turn:
  - BudgetDecision::Stop          -> "budget_limit"
  - BudgetDecision::ContinueWithWarning -> "budget_warning"
  - Context window is_blocking    -> "context_full"
- Non-blocking context % warnings and per-turn rate-limit fallbacks
  stay warning-only — routing every retry would be noise
- HOOK_EVENT_CATALOG, format_hook_event, parse_hook_event updated
- hooks/mod.rs module docs updated

Tests: 3 new
- Schema serde round-trip (notification <-> "notification")
- run_hooks dispatches for Notification (async tokio)
- parse_hook_event accepts "notification" / "Notification"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant