Repository navigation
fix(agent): don't prompt for approval when the session permission layer will refuse the call - #7699
akushonkamen wants to merge 1 commit into
Conversation
…er will refuse the call ApprovalSecurityMiddleware is the outermost tool wrap while the session's channel-permission ceiling lives on the inner ToolPolicyMiddleware, so a call the session refuses anyway raised an approval card first and was denied only after the human answered. Host the permission predicates on a borrowed SessionPermissionView shared by both gates, thread the session snapshot into the approval middleware at both assembly sites (turn harness + live/voice), and consult it in requires_approval: a refused verdict skips the prompt and the permission layer answers with its own rendered reason. Paths without a session snapshot (CLI / sub-agent) prompt exactly as before. Closes tinyhumansai#7649
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 60.61% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 5 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: avoiding approval prompts when the session permission layer will deny the call. Linked Issues check Issue #7649requires removal of approval prompts for calls that the session permission layer will refuse. The PR injects the session permission snapshot at both harness assembly points. `ApprovalSecur…Out of Scope Changes check The changes remain connected to issue #7649. Thetool_policy.rschanges extract existing permission decisions intoSessionPermissionViewso the approval pre-check and enforcement use one decision …
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the gate with care,
Before an approval card appears.
If session rules say “not this way,”
The prompt stays tucked and calls obey.
Then carrots crunch; the paths are clear.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Tiny Sweeper reviewTiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Could not review: crates/openhuman-core/src/agent/tinyagents/harness_assembly.rs, crates/openhuman-core/src/agent/tinyagents/live_harness.rs, crates/openhuman-core/src/agent/tinyagents/middleware/approval.rs, crates/openhuman-core/src/agent/tinyagents/middleware/tool_policy.rs, crates/openhuman-core/src/agent/tinyagents/middleware_approval_guard_tests.rs, tinysweeper/description Before merge
How this fits togetherflowchart LR
n0["ToolPolicyMiddleware<br/>changed"]:::changed
n1["channel_permission_block"]:::impacted
n2["Box"]:::impacted
n3["non_owner_middleware"]:::impacted
n4["assemble_turn_harness"]:::impacted
n5["wrap_tool"]:::impacted
n6["non_owner_middleware_at"]:::impacted
n0 -->|uses| n2
n3 -->|uses| n0
n3 -->|calls| n6
n4 -->|uses| n0
n4 -->|uses| n2
n5 -->|calls| n1
n6 -->|uses| n0
n6 -->|uses| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/agent/tinyagents/harness_assembly.rs, crates/openhuman-core/src/agent/tinyagents/live_harness.rs, crates/openhuman-core/src/agent/tinyagents/middleware/approval.rs, crates/openhuman-core/src/agent/tinyagents/middleware/tool_policy.rs, crates/openhuman-core/src/agent/tinyagents/middleware_approval_guard_tests.rs, tinysweeper/description.
$0.0020 · 343,863 in / 27,364 out · 56,320 cached (16%) · ladder/vectors-oai3, deepseek-v4.1-flash · 677 embedded
critique: $0.0007 · 62,714 in / 9,350 out · 18,432 cached (29%) · deepseek-v4.1-flash
security: $0.0009 · 196,298 in / 11,319 out · 27,904 cached (14%) · deepseek-v4.1-flash
tests: $0.0000 · 45,456 in / 532 out · 5,888 cached (13%) · deepseek-v4.1-flash
description: $0.0002 · 30,375 in / 2,768 out · 4,096 cached (13%) · deepseek-v4.1-flash
tinysweeper
Bot
added
the
priority: p3
This branch has not been deployed
1 participant
Summary
ToolPolicyMiddleware::channel_permission_block) is exposed to the approval middleware through a shared borrow view (SessionPermissionView), keeping the permission logic in one place (single source of truth) with zero predicate duplication.Problem
Issue #7649: OpenHuman may ask the user to approve a tool action even when the session's permissions will refuse that action afterward. The user approves, and is then denied anyway — a spurious interactive prompt that interrupts the workflow.
Root cause: the approval middleware is registered as the outermost tool wrap (
ApprovalGateMiddlewareatcrates/openhuman-core/src/agent/tinyagents/harness_assembly.rs:562, same order inlive_harness.rs:95), andApprovalSecurityMiddleware::requires_approval(crates/openhuman-core/src/agent/tinyagents/middleware/approval.rs:102) only checks whether the tool declares an external effect. The session's channel-permission ceiling (ToolPolicyMiddleware::channel_permission_block,tool_policy.rs:236) runs later, on the inner wrap, so a call the session must deny still triggers an approval prompt first.Solution
ToolPolicyMiddleware, and is shared with the approval layer through a borrowed viewSessionPermissionView(tool_policy.rs) instead of copying predicates — the repo's single-source-of-truth convention.ToolPolicyEnforcementsession snapshot into the approval middleware viaApprovalSecurityMiddleware::with_session_permission(harness_assembly.rs,live_harness.rs).requires_approvalnow pre-checks with the samechannel_permission_blockbefore showing a card: if the session will refuse the call, the approval prompt is skipped and the permission layer produces the denial directly.Submission Checklist
middleware_approval_guard_tests.rsgains 4 cases —a_session_denied_external_effect_tool_is_not_held_for_approval,a_call_above_the_session_permission_ceiling_is_not_held_for_approval(both red without the fix: the prompt fired anyway), and controlswithout_a_session_snapshot_the_prompt_decision_is_unchanged,a_permitted_external_effect_tool_still_requires_approval(both green before and after).## Related: N/A — no feature IDs affected.Fixes #7649.Gates run (after rebase onto
origin/main9dbf8f0568, toolchain 1.96.1):scripts/ci-cancel-aware.sh cargo test -p openhuman --lib approval_guard→ 5 passed / 2 failed — the two new cases red, the two controls green. On the clean rebased base the new cases do not exist (3 passed / 0 failed); dropping only the new test file onto the base fails to compile withE0599: no method named with_session_permission(×3), i.e. the tests require the new API and cannot pass without the fix.scripts/ci-cancel-aware.sh cargo test -p openhuman --lib approval_guard→ 7 passed / 0 failed (9567 filtered out).cargo test -p openhuman --lib approval→ 256 passed / 1 failed. The single failure ischannels::host::channel_events::tests::approval_prompts_reach_every_chat_channel, panicking atcrates/openhuman-core/src/channels/module_fixture_tests.rs:21(tinychannels-module fixture unavailable: blocked on tinybus 0.1.4); re-run standalone on the clean base commit9dbf8f0568and it panics identically — pre-existing, unrelated to this PR. (The suite total moved from 264 to 256 because main dropped tests between the two runs, not because of this change.)cargo test -p openhuman --lib middleware→ 210 passed / 0 failed / 1 ignored;--lib harness_assembly→ 6 passed;--lib security_gate→ 29 passed;--features voice --lib live_harness→ 3 passed (module is voice-gated, not compiled under default features).cargo check -p openhuman --lib --features voice→ exit 0.cargo fmt --checkon the 5 changed files → clean.cargo clippy -p openhuman --lib --all-targets→ exit 0; 31 warnings, none from the changed files (verified by grep over clippy output).Note: the ticket's verification command names package
-p openhuman-core, which does not exist; the crate atcrates/openhuman-coreis the packageopenhuman, so all commands above use-p openhuman.Impact
channel_permission_blockthat enforces the ceiling, so approvals cannot bypass it.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/7649-approval-before-permission7f4e63ae7c8973bbdf414e3ab8927fb4638bd7a9Validation Run
pnpm --filter openhuman-app format:check— N/A: change touches onlycrates/openhuman-core(Rust), no frontend code.pnpm typecheck— N/A: no TypeScript changed.scripts/ci-cancel-aware.sh cargo test -p openhuman --lib approval_guard→ 7 passed / 0 failed; subsets and full sweep as listed under Gates run.cargo fmt --checkon the changed files → clean;cargo check -p openhuman --lib --features voice→ exit 0.Validation Blocked
command:noneerror:noneimpact:noneBehavior Changes
Parity Contract
without_a_session_snapshot_the_prompt_decision_is_unchanged).a_permitted_external_effect_tool_still_requires_approvalasserts permitted external-effect tools still prompt; approval-identity scoping suite unchanged and green (210 middleware tests).Duplicate / Superseded PR Handling
Summary by CodeRabbit