Add approval audit history read path

[vaddisrinivas]

Refs #2135

Summary

Adds a durable approval audit history read path over existing approval storage:

• Exposes recently decided approval rows through the approval gate and RPC/controller registry.
• Adds ApprovalAuditEntry for decided approval rows.
• Adds store tests for durable decided-row reads and limit clamping.

Checklist

• Issue URL: Make approvals and audit durable across non-CLI channels #2135
• Branch: codex/oh-2135-approval-audit-history
• Commit SHA: dca6f62697014b101bf2e8943948ec389da4208d
• Files changed summary: src/openhuman/approval/{gate.rs,mod.rs,rpc.rs,schemas.rs,store.rs,types.rs} only.
• Validation commands run: cargo fmt --manifest-path Cargo.toml --all --check; cargo test --manifest-path Cargo.toml approval; node scripts/check-pr-checklist.mjs /tmp/oh-2135-pr-body.md.
• Validation commands blocked: pnpm pr:checklist /tmp/oh-2135-pr-body.md blocked with zsh:1: command not found: pnpm; direct node checker passed.
• Behavior change: adds a generic approval list_recent_decisions read path for durable audit/history surfaces; no change to approval decision semantics.
• Duplicate/stale PR note: no existing PR found for this head branch; gh pr list --repo tinyhumansai/openhuman --state all --search "2135" returned no PRs before creation.

Summary by CodeRabbit

• New Features

  • Fetch recent approval decisions with an optional limit (defaults to 50, clamped to 1–500).
  • Durable audit records for decided approvals are now stored and returned, including timestamps, metadata, and outcomes.
  • New API/controller endpoint to retrieve approval decision history.

• Bug Fixes

  • Improved startup/readiness behavior for the embedded core to reduce race/timeout issues during initialization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: db1ec613-9a8a-4abd-acf2-2c66b3be1458

📥 Commits

Reviewing files that changed from the base of the PR and between df0fa27 and 178d9c7.

📒 Files selected for processing (1)

• app/src-tauri/src/core_process.rs

---

📝 Walkthrough

Walkthrough

Adds an ApprovalAuditEntry type and a full request path to list recent decided approval rows: DB query and row decoding, ApprovalGate method, approval RPC handler, controller schema/handler with optional limit parsing, plus tests. Also tweaks an embedded-core readiness polling constant.

Changes

Approval Audit Listing

Layer / File(s)	Summary
Audit entry type and module exports src/openhuman/approval/types.rs, src/openhuman/approval/mod.rs	ApprovalAuditEntry struct captures post-decision audit records with request identifiers, redacted metadata, session timing, and decided_at/decision fields. Type is publicly re-exported from the approval module.
Store layer: decision queries and row transformation src/openhuman/approval/store.rs	list_recent_decisions queries pending_approvals for decided rows (non-null decided_at and decision), clamps limit to [1,500], orders by most-recent decision, and maps results via row_to_audit_entry (JSON fallback, RFC3339 parsing, strict decision parsing). Tests cover success and failure cases.
Gate and RPC APIs src/openhuman/approval/gate.rs, src/openhuman/approval/rpc.rs	ApprovalGate::list_recent_decisions delegates to store. RPC handler defaults limit to 50, returns empty outcome if gate not installed, logs and propagates store errors, and wraps results in RpcOutcome with a row-count log.
Schema and controller handler wiring src/openhuman/approval/schemas.rs	Registers list_recent_decisions controller schema (optional unsigned limit input, decisions array output). Adds handle_list_recent_decisions handler, read_optional_u64 helper, and tests verifying schema registration and parameter parsing.

Embedded core readiness

Layer / File(s)	Summary
Readiness polling constant and loop update app/src-tauri/src/core_process.rs	Adds EMBEDDED_CORE_READY_WAIT_ATTEMPTS and uses it in the embedded-core readiness polling loop, with comments explaining extended waits for CI/container timing and early-exit when ready.

Sequence Diagram(s)

sequenceDiagram
  participant Controller
  participant RPC as approval_rpc
  participant Gate as ApprovalGate
  participant Store as store
  Controller->>RPC: invoke list_recent_decisions(limit?)
  RPC->>Gate: check installed
  alt gate installed
    RPC->>Gate: list_recent_decisions(limit)
    Gate->>Store: list_recent_decisions(config, limit)
    Store-->>Gate: Vec<ApprovalAuditEntry>
    Gate-->>RPC: Vec<ApprovalAuditEntry>
    RPC-->>Controller: RpcOutcome with row-count log
  else gate not installed
    RPC-->>Controller: empty RpcOutcome
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#2149: The main PR extends the ApprovalGate/RPC/schema foundation by adding a new ApprovalAuditEntry-backed ApprovalGate::list_recent_decisions plus corresponding approval_list_recent_decisions RPC/controller wiring.

Suggested labels

feature

Suggested reviewers

• graycyrus
• senamakel

Poem

🐰 I hopped through code with nimble feet,
From DB rows to RPCs so neat,
I baked a list of decisions bright,
And watched the audit logs take flight—
Hooray for records kept just right! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add approval audit history read path' directly and clearly describes the primary change: introducing a new read path for approval audit history through the approval gate and RPC/controller layers.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/openhuman/approval/rpc.rs (1)

36-58: ⚡ Quick win

Add an explicit exit log for this RPC path.

This method logs entry and errors, but not a debug/trace exit event like the adjacent RPCs. Add an exit log with rows and effective limit to keep diagnostics consistent.

📝 Proposed tweak

 pub async fn approval_list_recent_decisions(
@@
     let rows = match gate.list_recent_decisions(limit) {
@@
     let log = format!(
         "[approval] list_recent_decisions returned {} row(s)",
         rows.len()
     );
+    tracing::debug!(
+        rows = rows.len(),
+        limit = limit,
+        "[rpc:approval_list_recent_decisions] exit"
+    );
     Ok(RpcOutcome::single_log(rows, log))
 }

As per coding guidelines: Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone.

🤖 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/rpc.rs` around lines 36 - 58, The function
approval_list_recent_decisions currently logs entry and errors but lacks a
debug/trace exit; add a tracing::debug! exit log just before the final
Ok(RpcOutcome::single_log(...)) that records the effective limit and the number
of rows returned (e.g., tracing::debug!(limit = %limit, rows = %rows.len(),
"[rpc:approval_list_recent_decisions] exit")), so callers and diagnostics see
the RPC exit with both limit and rows metrics.

🤖 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 220-240: The row_to_audit_entry function is silently fabricating
values on decode failures; instead, fail the conversion and return an error so
corrupt DB rows surface. Change the JSON args parse (currently using
unwrap_or_else) to propagate a serde_json::Error mapped into a rusqlite::Error,
validate ApprovalDecision::from_str and return an error rather than defaulting
to Deny, and make parse_rfc3339 failures for created_str/decided_str/expires_opt
propagate as errors as well; locate these changes in row_to_audit_entry,
specifically the args_redacted parsing, the decision assignment using
ApprovalDecision::from_str, and the created_at/decided_at/expires_at parsing so
invalid timestamps or decisions cause the function to return an
Err(rusqlite::Error::FromSqlConversionFailure(...)) instead of fabricating
values.

---

Nitpick comments:
In `@src/openhuman/approval/rpc.rs`:
- Around line 36-58: The function approval_list_recent_decisions currently logs
entry and errors but lacks a debug/trace exit; add a tracing::debug! exit log
just before the final Ok(RpcOutcome::single_log(...)) that records the effective
limit and the number of rows returned (e.g., tracing::debug!(limit = %limit,
rows = %rows.len(), "[rpc:approval_list_recent_decisions] exit")), so callers
and diagnostics see the RPC exit with both limit and rows metrics.

🪄 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: f046a8ec-713b-4b2d-b678-553813db8c09

📥 Commits

Reviewing files that changed from the base of the PR and between ebd6457 and dca6f62.

📒 Files selected for processing (6)

• src/openhuman/approval/gate.rs
• src/openhuman/approval/mod.rs
• src/openhuman/approval/rpc.rs
• src/openhuman/approval/schemas.rs
• src/openhuman/approval/store.rs
• src/openhuman/approval/types.rs

[graycyrus]

Review — graycyrus

Clean, well-scoped PR. Adds a durable read path for decided approval rows with proper error handling, schema registration, and good test coverage.

Walkthrough

Area	Files	What changed
Rust core (approval)	gate.rs, rpc.rs, schemas.rs, store.rs, types.rs, mod.rs	New ApprovalAuditEntry type, list_recent_decisions store query (clamped 1–500), RPC handler, controller schema + registration, 4 new store tests + schema tests
Tauri shell	core_process.rs	Extract magic 200 to EMBEDDED_CORE_READY_WAIT_ATTEMPTS const, trim comment (unrelated but harmless)

Notes

• CodeRabbit's finding about silent value fabrication has been addressed — decision and timestamp parsing now propagate errors correctly. The args_redacted JSON fallback is a reasonable choice for non-critical metadata.
• Issue #2135 is referenced (not closed), which is correct since this covers only the read path portion of a larger effort.
• All CI green including coverage gate.
• No new findings beyond what CodeRabbit already covered.

LGTM — no blocking issues.

[graycyrus]

Looks good, nice work!