feat(approval): persist post-execution audit row alongside approval (#2135)#2367
Merged
senamakel merged 8 commits intoMay 23, 2026
Merged
Conversation
…inyhumansai#2135) Approval gate currently writes a single durable row per intercepted tool call: it captures *the action was approved at X* but never the matching *the action ran at Y with outcome Z*. The acceptance criterion in tinyhumansai#2135 requires tests for audit writes both before AND after an action — this commit closes that gap. Approval domain - New `ExecutionOutcome` enum (`Success` / `Failure` / `Aborted`). - `pending_approvals` SQLite table gets three new columns: `executed_at`, `execution_outcome`, `execution_error`. Idempotent PRAGMA-table_info migration upgrades v1 DBs in place — pre-tinyhumansai#2135 rows back-fill as NULL so older audit history stays readable. - New `store::record_execution(request_id, outcome, error)`. Gated by `decided_at IS NOT NULL` so the audit trail stays monotonic (no "executed before approved" rows possible). Error strings are capped at 512 chars so upstream crash dumps can't fill the audit log; the agent still sees the full error in its tool-result envelope. - New `ApprovalGate::intercept_audited(...)` returns `(GateOutcome, Option<String>)`. The legacy `intercept(...)` is a thin wrapper that drops the id, so call sites that don't care about after-execution audit (CLI, tests) keep their current shape. - New `ApprovalGate::record_execution(...)` wraps the store call with best-effort logging — audit-write failure is logged but never bubbles back to the agent (the tool already ran). Wiring (4 call sites) - `agent/harness/tool_loop.rs`: capture request_id from the gate, stamp Success/Failure with the truncated tool result text after the tool finishes. - `agent/harness/subagent_runner/ops.rs`: same shape for sub-agent-dispatched tool calls. - `channels/proactive.rs`: stamp Success on `ch.send` Ok, Failure with the error message on Err. - `agent/triage/escalation.rs`: stamp Success/Failure on the triage-dispatched sub-agent outcome. Tests (8 new, 71 passing in `cargo test --lib approval`) - `record_execution_writes_terminal_audit_row_after_decide` — before-decide attempts must be a no-op; after decide they patch the row. - `record_execution_persists_outcome_and_redacted_error` — Failure with error text round-trips through SQLite. - `record_execution_caps_long_error_messages` — 2 000-char input is capped to ~512 chars + ellipsis. - `record_execution_unknown_id_is_safe_noop` — stray record_execution for an id that was never persisted reports `false` without writing. - `migrate_columns_is_idempotent_on_v1_databases` — synthesise a pre-tinyhumansai#2135 schema, prove the migration adds the new columns without losing existing rows AND that a second open is a no-op. - `execution_outcome_round_trips` / `_serializes_as_snake_case` — on-wire shape for the new enum. - `intercept_audited_returns_request_id_only_when_allowed_and_persisted` — Allow path returns the id so callers can record_execution. - `intercept_audited_returns_none_id_for_denied_and_allowlisted` — Deny path and session-allowlist shortcut both return None (nothing to record afterward). Out of scope (companion / follow-up PRs) - `composio.list_recent_decisions` audit-history read path — owned by tinyhumansai#2335 (open). - Lazy expiry of stale pending rows after restart — owned by tinyhumansai#2357 (open). - Policy-callback hook for unattended channel/webhook denial — follow-up.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds audited approval intercepts that return request IDs, DB migration and columns for execution audit, an ExecutionOutcome enum, and wiring to record terminal success/failure across tool harness, escalation, and proactive external-channel sends. ChangesApproval audit recording
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/approval/gate.rs (1)
200-255:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftHandle approval-vs-timeout races before forcing
Deny.
decide()commits the approval in SQLite before it sends on the waiter. If that commit lands right as the TTL expires, this branch can still time out, returnGateOutcome::Deny, and drop therequest_ideven though the row is already approved. That leaves the durable audit saying “approved” while the tool never runs and no execution outcome can be recorded. Please make the timeout path re-check the persisted decision (or otherwise serialize timeout vs. decide atomically) before denying.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/approval/gate.rs` around lines 200 - 255, The timeout branch currently unconditionally denies even if store::decide committed an approval concurrently; modify the Err(_elapsed) handling in the gate wait logic to re-check the persisted decision before returning Deny: after self.evict_waiter(&request_id) (and before logging/returning), call the store read method that fetches the stored ApprovalDecision for request_id (the same persistence API that decide() writes), and if the persisted decision is Approve return (GateOutcome::Allow, Some(request_id)) instead of denying; only if the persisted decision is Deny or missing proceed with the existing deny path and calls to store::decide. Ensure you reference self.ttl, rx, store::decide, evict_waiter, and GateOutcome when locating the fix.src/openhuman/agent/harness/subagent_runner/ops.rs (1)
1524-1550:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the structured execution result for audit status, not the rendered text.
!raw.starts_with("Error")is not a reliable success signal. A successful tool payload can legitimately begin with"Error"(for example copied logs or search results), which will persist a falseFailurein the new audit row. Carry asuccessbool out of the timeout/execute match instead of re-parsing the display string.Suggested fix
- let raw = match tokio::time::timeout(timeout, tool.execute(args)).await { - Ok(Ok(result)) => { - let raw = result.output(); - if result.is_error { - format!("Error: {raw}") - } else { - raw - } - } - Ok(Err(err)) => format!("Error executing {}: {err}", call.name), - Err(_) => format!("Error: tool '{}' timed out", call.name), - }; + let (raw, success) = match tokio::time::timeout(timeout, tool.execute(args)).await { + Ok(Ok(result)) => { + let raw = result.output(); + if result.is_error { + (format!("Error: {raw}"), false) + } else { + (raw, true) + } + } + Ok(Err(err)) => (format!("Error executing {}: {err}", call.name), false), + Err(_) => (format!("Error: tool '{}' timed out", call.name), false), + }; // Stamp the terminal status onto the // pending_approvals audit row — best-effort, // failures don't propagate to the agent (`#2135`). if let (Some(gate), Some(req_id)) = ( approval_gate_for_audit.as_ref(), approval_request_id.as_ref(), ) { - let success = !raw.starts_with("Error"); let exec_outcome = if success { crate::openhuman::approval::ExecutionOutcome::Success } else { crate::openhuman::approval::ExecutionOutcome::Failure };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/subagent_runner/ops.rs` around lines 1524 - 1550, The audit success is being derived from the rendered text with !raw.starts_with("Error"), which is brittle; instead capture a boolean success from the tokio::time::timeout(...) match that distinguishes Ok(Ok(result)) vs Ok(Err(_)) vs Err(_), and use that success value when computing exec_outcome and err_text before calling gate.record_execution; update the match around tool.execute(args) (the block that defines raw) to also set a success flag (e.g., success: bool) and use that flag in the approval_gate_for_audit / approval_request_id handling rather than parsing raw.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/approval/store.rs`:
- Around line 259-266: The truncation currently takes 512 chars then appends "…"
so stored strings can become 513 chars; update the closure that builds
trimmed_error (the error.map(|e| { ... }) block) to ensure the final string
never exceeds 512 characters by either taking 511 characters and then appending
"…" when truncated or by hard-truncating to 512 without appending; change the
chars.take(...) logic accordingly so trimmed_error respects the 512-character
cap.
- Around line 267-275: The UPDATE in the pending_approvals write currently
allows overwriting executed_at/execution_outcome/execution_error on retries;
modify the SQL in the execute call that sets executed_at, execution_outcome,
execution_error (the same execute invocation that uses params![now,
outcome.as_str(), trimmed_error, request_id]) to add "AND executed_at IS NULL"
to the WHERE clause (in addition to the existing "decided_at IS NOT NULL") so
the first terminal report wins and subsequent calls are ignored.
---
Outside diff comments:
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 1524-1550: The audit success is being derived from the rendered
text with !raw.starts_with("Error"), which is brittle; instead capture a boolean
success from the tokio::time::timeout(...) match that distinguishes
Ok(Ok(result)) vs Ok(Err(_)) vs Err(_), and use that success value when
computing exec_outcome and err_text before calling gate.record_execution; update
the match around tool.execute(args) (the block that defines raw) to also set a
success flag (e.g., success: bool) and use that flag in the
approval_gate_for_audit / approval_request_id handling rather than parsing raw.
In `@src/openhuman/approval/gate.rs`:
- Around line 200-255: The timeout branch currently unconditionally denies even
if store::decide committed an approval concurrently; modify the Err(_elapsed)
handling in the gate wait logic to re-check the persisted decision before
returning Deny: after self.evict_waiter(&request_id) (and before
logging/returning), call the store read method that fetches the stored
ApprovalDecision for request_id (the same persistence API that decide() writes),
and if the persisted decision is Approve return (GateOutcome::Allow,
Some(request_id)) instead of denying; only if the persisted decision is Deny or
missing proceed with the existing deny path and calls to store::decide. Ensure
you reference self.ttl, rx, store::decide, evict_waiter, and GateOutcome when
locating the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3047abd0-a16b-41a4-9e69-d03fa27fb0ea
📒 Files selected for processing (8)
src/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/tool_loop.rssrc/openhuman/agent/triage/escalation.rssrc/openhuman/approval/gate.rssrc/openhuman/approval/mod.rssrc/openhuman/approval/store.rssrc/openhuman/approval/types.rssrc/openhuman/channels/proactive.rs
…ror at 512 incl. ellipsis + make terminal write immutable Two CodeRabbit findings on src/openhuman/approval/store.rs: 1. **Minor — cap inclusive of ellipsis.** The previous truncation took 512 chars then appended "…", so stored values became 513 chars when truncation kicked in. Fixed by taking 511 chars when the input would overflow and appending the marker; short inputs pass through unchanged. New test asserts the truncated value is exactly 512 chars including the ellipsis. 2. **Major — terminal write must be immutable.** The UPDATE used `WHERE request_id = ?4 AND decided_at IS NOT NULL`, which let any later `record_execution` call silently overwrite executed_at / execution_outcome / execution_error. Added `AND executed_at IS NULL` so the first terminal report wins; a late retry / cleanup path now reports `false` and leaves the original audit row intact. New test `record_execution_is_idempotent_after_first_terminal_report_wins` pins the contract. `cargo test --lib approval::store` → 13 passed, 0 failed (13 = 11 pre-existing + 1 new idempotency test; the cap test was already there but its assertion now uses the exact 512-char count).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/approval/store.rs`:
- Around line 263-269: The current code only truncates the execution_error
string via trimmed_error and still persists raw backend/tool text; update the
persistence step to run execution_error through the same audit-redaction routine
used for request payloads (use the project's audit redaction function that
sanitizes tokens, emails, and PII) instead of simple truncation, or
alternatively replace the stored value with a sanitized summary/error code;
ensure you update handling around trimmed_error/execution_error in store.rs so
the redacted result (not the raw or just-truncated string) is what gets written
to the durable audit table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5efd6e5c-d848-43f2-91bd-c2e8fd108125
📒 Files selected for processing (1)
src/openhuman/approval/store.rs
coderabbitai
Bot
added
feature
…humansai#2367 review) Run the upstream error blurb through memory::safety::sanitize_text *before* truncating to 512 chars so the durable audit row cannot leak bearer tokens, API keys, private-key blocks, OAuth params, or government-ID PII the failing tool may have echoed back. Truncating first would split a secret mid-string and dodge the regex matchers. Adds record_execution_redacts_secrets_in_error_message asserting a bearer token and provider key are scrubbed from the stored row and that the [REDACTED] marker remains visible to audit readers.
# Conflicts: # src/openhuman/approval/store.rs
…ess (tinyhumansai#2367 review) - gate.rs: when the TTL elapses concurrently with a committed approval, `store::decide(Deny)` silently no-ops (its `WHERE decided_at IS NULL` guard preserves the existing row), but the gate previously returned Deny anyway — leaving the durable audit row saying Approved while the tool never ran. The timeout path now re-reads the persisted decision via the new `store::get_decision` and honors an existing Approve. - subagent_runner/ops.rs: replace `!raw.starts_with(\"Error\")` audit signal with a structured `exec_success` bool captured from the `tokio::time::timeout` match. Legitimate success payloads can start with the literal word \"Error\" (search hits, copied logs) and would otherwise persist a false Failure on the audit row. - store.rs: add `get_decision(config, request_id)` read helper used by the gate race resolution, plus a unit test covering missing/undecided/ decided round-trip. Addresses CodeRabbit review on PR tinyhumansai#2367: - src/openhuman/approval/gate.rs lines 200-255 (timeout vs decide race) - src/openhuman/agent/harness/subagent_runner/ops.rs lines 1524-1550 (brittle string-prefix success detection)
The merge from main brought back PR tinyhumansai#2367's older de-5.ts that still had duplicate keys for settings.developerMenu.mcpServer.* and settings.mcpServer.* — the canonical fix on main lives at lines 211-235. TS1117 was failing CI on this branch (Type Check TypeScript). Drop the duplicates so this file matches main.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/approval/store.rs (1)
319-328:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly stamp execution on approved rows.
decided_at IS NOT NULLalso matchesDeny/auto-expired rows, so a stray caller can create an impossible audit trail withdecision = denyandexecuted_atpopulated. Please restrict this update to approval decisions only at the store boundary.Suggested fix
+ let approve_once = ApprovalDecision::ApproveOnce.as_str(); + let approve_always = ApprovalDecision::ApproveAlwaysForTool.as_str(); let updated = conn .execute( "UPDATE pending_approvals SET executed_at = ?1, execution_outcome = ?2, execution_error = ?3 WHERE request_id = ?4 - AND decided_at IS NOT NULL + AND decided_at IS NOT NULL + AND decision IN (?5, ?6) AND executed_at IS NULL", - params![now, outcome.as_str(), trimmed_error, request_id], + params![ + now, + outcome.as_str(), + trimmed_error, + request_id, + approve_once, + approve_always, + ], )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/approval/store.rs` around lines 319 - 328, The UPDATE currently matches any decided row (including Deny/expired); modify the SQL in the conn.execute call that updates pending_approvals so it only updates rows whose decision is the approval value (e.g., add "AND decision = 'approve'" to the WHERE clause), keeping the rest of the parameters (now, outcome.as_str(), trimmed_error, request_id) and semantics the same so executed_at can only be stamped for approved requests.src/openhuman/agent/harness/subagent_runner/ops.rs (1)
1583-1619:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThread
exec_successthrough to the downstream status path.This fixes the audit write, but the later
call_success/ text-modestatuslogic still falls back to!result_text.starts_with("Error"). A successful payload that begins with"Error"will now persistExecutionOutcome::Successwhile the progress event and<tool_result status="...">still report failure.Suggested fix
- let result_text = if !allowed_names.contains(&call.name) { + let (result_text, call_success) = if !allowed_names.contains(&call.name) { tracing::warn!( task_id = %task_id, agent_id = %agent_id, tool = %call.name, "[subagent_runner] tool not in allowlist for this sub-agent" ); let mut available: Vec<&str> = allowed_names.iter().map(|s| s.as_str()).collect(); if let Some(resolver) = lazy_resolver.as_ref() { available.extend(resolver.known_slugs()); } available.sort_unstable(); available.dedup(); - format!( + ( + format!( "Error: tool '{}' is not available to the {} sub-agent. Available tools: {}", call.name, agent_id, available.join(", ") - ) + ), + false, + ) } else if let Some(tool) = extra_tools .iter() .find(|t| t.name() == call.name) .or_else(|| parent_tools.iter().find(|t| t.name() == call.name)) { @@ if let Some(reason) = gate_denial { - format!("Error: {reason}") + (format!("Error: {reason}"), false) } else { let (raw, exec_success) = match tokio::time::timeout(timeout, tool.execute(args)).await { @@ if let (Some(gate), Some(req_id)) = ( approval_gate_for_audit.as_ref(), approval_request_id.as_ref(), ) { - let success = exec_success; - let exec_outcome = if success { + let exec_outcome = if exec_success { crate::openhuman::approval::ExecutionOutcome::Success } else { crate::openhuman::approval::ExecutionOutcome::Failure }; - let err_text = if success { None } else { Some(raw.as_str()) }; + let err_text = if exec_success { None } else { Some(raw.as_str()) }; gate.record_execution(req_id, exec_outcome, err_text); } - raw + (raw, exec_success) } } else { - format!("Unknown tool: {}", call.name) + (format!("Unknown tool: {}", call.name), false) }; @@ - let call_success = !result_text.starts_with("Error"); let call_output_chars = result_text.chars().count();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/subagent_runner/ops.rs` around lines 1583 - 1619, The code currently computes exec_success from the structured execute result but later still derives call_success / text-mode status by checking result_text.starts_with("Error"), causing mismatches when a successful payload begins with "Error"; update the downstream status and progress-event logic (the places that compute call_success and status and emit the <tool_result status="..."> or progress events) to use the previously computed exec_success boolean instead of parsing raw/result_text, and pass exec_success through to any variables or function calls that currently accept call_success or status so the audit record, progress event, and tool_result all use the same ExecutionOutcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 1583-1619: The code currently computes exec_success from the
structured execute result but later still derives call_success / text-mode
status by checking result_text.starts_with("Error"), causing mismatches when a
successful payload begins with "Error"; update the downstream status and
progress-event logic (the places that compute call_success and status and emit
the <tool_result status="..."> or progress events) to use the previously
computed exec_success boolean instead of parsing raw/result_text, and pass
exec_success through to any variables or function calls that currently accept
call_success or status so the audit record, progress event, and tool_result all
use the same ExecutionOutcome.
In `@src/openhuman/approval/store.rs`:
- Around line 319-328: The UPDATE currently matches any decided row (including
Deny/expired); modify the SQL in the conn.execute call that updates
pending_approvals so it only updates rows whose decision is the approval value
(e.g., add "AND decision = 'approve'" to the WHERE clause), keeping the rest of
the parameters (now, outcome.as_str(), trimmed_error, request_id) and semantics
the same so executed_at can only be stamped for approved requests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e56c4e17-8529-4f93-96b6-4ba62d4193ec
📒 Files selected for processing (5)
app/src/lib/i18n/chunks/de-5.tssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/tool_loop.rssrc/openhuman/approval/gate.rssrc/openhuman/approval/store.rs
✅ Files skipped from review due to trivial changes (1)
- app/src/lib/i18n/chunks/de-5.ts
The 500ms TTL was racing the 50×10ms poll loop on slow CI runners — list_pending's lazy-expire would deny the row before decide() could fire, surfacing as "pending row never appeared" in approve_once_returns_allow on Linux CI. 2s gives the polling tests headroom while keeping timeout_returns_deny fast.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
executed_at,execution_outcome, andexecution_errorcolumns topending_approvalsso every approved tool call carries both a before-row (approval decision) and an after-row (terminal execution status).ApprovalGate::intercept_auditedreturns the persistedrequest_idso the four agent paths that gate external-effect tools can stamp the terminal status when the tool finishes.store::record_executionenforces monotonic audit (decided_at IS NOT NULLguard) and caps error strings at 512 chars to keep upstream crash dumps out of the audit log.Problem
pending_approvalsrows transition from(no decided_at)→(decided_at, decision)and stop there. There is no terminal entry recording whether the approved tool actually ran, what it returned, or whether it failed. A compliance reviewer can see approved but not executed — they have to cross-reference the agent transcript, which is the wrong place for an audit trail. Issue #2135 calls this gap out explicitly under "Tests cover audit write before AND after an action".Solution
Approval domain
ExecutionOutcomeenum (Success/Failure/Aborted).pending_approvalstable gets three new columns. IdempotentPRAGMA table_infomigration upgrades v1 DBs in place — pre-Make approvals and audit durable across non-CLI channels #2135 rows back-fill as NULL so older audit history stays readable.store::record_execution(request_id, outcome, error)gated bydecided_at IS NOT NULLso the audit trail stays monotonic (no executed before approved rows possible). Error strings capped at 512 chars + ellipsis.ApprovalGate::intercept_audited(...)returns(GateOutcome, Option<String>). Legacyintercept(...)is a thin wrapper that drops the id so call sites that don't care about after-execution audit (CLI, tests) keep their current shape.ApprovalGate::record_execution(...)wraps the store call with best-effort logging — audit-write failure is logged but never bubbles back to the agent (the tool already ran).Wiring (4 call sites)
agent/harness/tool_loop.rs— capture request_id from the gate, stamp Success/Failure with the truncated tool result text after the tool finishes.agent/harness/subagent_runner/ops.rs— same shape for sub-agent-dispatched tool calls.channels/proactive.rs— stamp Success onch.sendOk, Failure with the error message on Err.agent/triage/escalation.rs— stamp Success/Failure on the triage-dispatched sub-agent outcome.Submission Checklist
cargo test --lib approval→ 71 passed, 0 failed.store.rs/gate.rsis exercised by a new or updated test. Four call-site changes are 5–10 lines each, all on hot paths already covered by existing harness tests.## Related— N/A: no matrix row touched.Closes #NNNin the## Relatedsection — see below.Impact
~/.openhuman/approval/approval.dbnow carriesexecuted_at+execution_outcome+execution_errorper approved row; UI surfaces that consume these rows will see the new fields.with_connectioncall. v1 DBs (older builds) get the new columns added in place, with existing rows back-filled as NULL.Tests
cargo test --lib approval→ 71 passed, 0 failed (8 new, 63 pre-existing).record_execution_writes_terminal_audit_row_after_deciderecord_execution_persists_outcome_and_redacted_errorrecord_execution_caps_long_error_messagesrecord_execution_unknown_id_is_safe_nooprecord_executionfor an id that was never persisted reportsfalsewithout writingmigrate_columns_is_idempotent_on_v1_databasesexecution_outcome_round_trips/_serializes_as_snake_caseintercept_audited_returns_request_id_only_when_allowed_and_persistedrecord_executionintercept_audited_returns_none_id_for_denied_and_allowlistedNone(nothing to record afterward)Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2135-audit-action-execution(branched fromorigin/mainafter fresh fetch)Validation Run
pnpm --filter openhuman-app format:check— N/A: no frontend changes.pnpm typecheck— N/A: no TypeScript changes.cargo test --lib approval→ 71 passed, 0 failed.cargo fmt --manifest-path Cargo.tomlapplied;cargo checkclean.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
pending_approvalsrow.Parity Contract
intercept(...)keeps its existingGateOutcomeshape (it now wrapsintercept_auditedand drops the id). CLI and existing tests don't need changes.record_executionis gated bydecided_at IS NOT NULLso the audit trail stays monotonic; idempotent column migration is tested against a synthetic v1 schema.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Chores
Tests