Skip to content

feat(output): emit permission_denied event on JSONL stream#219

Merged
emal-avala merged 1 commit intomainfrom
feat/jsonl-permission-denied-event
Apr 23, 2026
Merged

feat(output): emit permission_denied event on JSONL stream#219
emal-avala merged 1 commit intomainfrom
feat/jsonl-permission-denied-event

Conversation

@emal-avala
Copy link
Copy Markdown
Member

Summary

Automation consuming the JSONL stream had to grep `tool_result.output` for the literal string `"Permission denied: "` to tell a policy-blocked tool call apart from a genuine tool error (file not found, timeout, etc.). That coupling silently breaks when the executor's error text changes — and it offered no way to distinguish the two user-facing deny paths at all.

Adds `Event::PermissionDenied { tool, reason, turn }` emitted alongside the existing `ToolResult` envelope. Detection pins the two exact shapes emitted by `tools/executor.rs`:

  • Policy-driven Deny: `"Permission denied: {reason}"` — reason is the executor's explanation (e.g. `"path outside cwd"`)
  • Interactive user Deny: `"Permission denied by user"` — synthesizes a stable reason string `"user denied at prompt"` so consumers don't have to handle missing fields

Both shapes are covered by pinning tests so a future change to `executor.rs` will fail the build here instead of silently stopping the `PermissionDenied` emission.

The existing `ToolResult` is still emitted, so consumers that don't know about the new event keep working.

Test plan

  • `cargo clippy -p agent-code --tests --no-deps -- -D warnings` — clean
  • `cargo test -p agent-code --bin agent output::` — 19 pass (13 prior + 6 new)
  • 6 new tests:
    • standard prefix detected with reason extraction
    • user-deny prompt shape detected with synthesized reason
    • non-error results ignored (even with matching prefix)
    • generic tool errors (file-not-found, timeout) NOT tagged
    • empty reason rejected so no stray `reason=""` events
    • snake_case event shape + all required fields
  • `all_events_are_single_line_json` extended to cover a multiline-reason `PermissionDenied`

Automation consuming the JSONL stream had to grep tool_result.output
for the literal string "Permission denied: " to tell a policy-blocked
tool call apart from a genuine tool error (file not found, timeout,
etc.). That coupling silently breaks when the executor's error text
changes — and it offered no way to distinguish the two user-facing
deny paths at all.

Adds Event::PermissionDenied { tool, reason, turn } emitted alongside
the existing ToolResult envelope. Detection pins the two exact shapes
emitted by tools/executor.rs:

- Policy-driven Deny: "Permission denied: {reason}" — reason is the
  executor's explanation (e.g. "path outside cwd")
- Interactive user Deny: "Permission denied by user" — synthesizes a
  stable reason string "user denied at prompt" so consumers don't
  have to handle missing fields

Both shapes are covered by pinning tests so a future change to
executor.rs will fail the build here instead of silently stopping the
PermissionDenied emission. Empty-reason values are rejected so we
don't emit meaningless events, and non-error tool results are ignored
even if their body coincidentally contains the prefix.

5 new tests plus an extension of all_events_are_single_line_json that
covers a multiline reason (proving serde still escapes newlines).
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@emal-avala emal-avala merged commit 8269191 into main Apr 23, 2026
14 checks passed
@emal-avala emal-avala deleted the feat/jsonl-permission-denied-event branch April 23, 2026 19:09
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