fix(approval): make "always" auto-approve work for credentialed HTTP requests

[nearfamiliarcow]

Problem

When IronClaw makes HTTP calls to APIs with stored credentials (GitHub, Attio, etc.), the user is prompted to approve each call. The prompt offers "yes", "no", and "always". Clicking "always" should auto-approve all future calls to that tool for the session — but it doesn't work. The user gets re-prompted on every single HTTP call, indefinitely.

Real-world example (Telegram)

User:  Research Client and add them to CRM
Bot:   Approval needed: http
       url: "https://api.github.com/repos/..."
       Reply "yes" to approve, "no" to deny, or "always" to auto-approve.
User:  always
Bot:   Approval needed: http        ← prompted AGAIN
       url: "https://api.github.com/repos/.../README.md"
       Reply "yes" to approve, "no" to deny, or "always" to auto-approve.
User:  always
Bot:   Approval needed: http        ← and AGAIN
       url: "https://api.github.com/search/code?q=org:..."

Root cause

IronClaw has a three-tier approval system (ApprovalRequirement):

Tier	Meaning	Checks session auto-approve set?
Never	Don't ask	N/A
UnlessAutoApproved	Ask, but respect "always"	Yes
Always	Ask every time, no exceptions	No — hardcoded to true

The HTTP tool (http.rs:839) returned ApprovalRequirement::Always for any request where the credential registry has a mapping for that host. Since virtually every useful API call has credentials (GitHub token via github.capabilities.json, Attio keys, etc.), every HTTP call hit the Always path.

In the dispatcher (dispatcher.rs:558), the Always arm is:

ApprovalRequirement::Always => true, // never checks auto_approved_tools

So even though clicking "always" correctly saves "http" to session.auto_approved_tools (in thread_ops.rs:909), the next call ignores it because Always never looks at that set.

Meanwhile, the UI always offered "always" as an option regardless of tier — so the user sees and clicks a button that the backend silently ignores.

Fix

Two changes addressing both the backend bug and the UI lie:

1. HTTP credentialed requests: Always → UnlessAutoApproved

Credentialed HTTP requests are not in the same risk class as rm -rf / or git push --force. The Always tier is reserved for genuinely destructive operations (destructive shell commands, tool_remove, skill_remove). API calls with credentials should be UnlessAutoApproved — prompt on first use, then respect "always".

-            return ApprovalRequirement::Always;
+            return ApprovalRequirement::UnlessAutoApproved;

2. Hide "always" in the UI when it won't work

A new allow_always: bool field flows from the dispatcher through PendingApproval → StatusUpdate::ApprovalNeeded → every channel UI. When a tool returns ApprovalRequirement::Always (e.g. destructive shell), the "always" button/text is hidden. When it returns UnlessAutoApproved, the option is shown.

Why not the other direction (make Always respect auto-approve)?

That would defeat the purpose of Always. The existing test test_always_approval_requirement_bypasses_session_auto_approve explicitly verifies that Always ignores the auto-approve set — that's correct behavior for tool_remove, destructive shell commands, etc. Our fix preserves that invariant.

After this fix, the example conversation becomes:

1. First HTTP call to api.github.com → UnlessAutoApproved → prompted
2. User says "always" → "http" added to session.auto_approved_tools
3. Second HTTP call → UnlessAutoApproved → checks set → found → no prompt
4. Third HTTP call → same → no prompt

Backward compatibility

• PendingApproval.allow_always uses #[serde(default = "default_true")] — old serialized values deserialize with "always" shown (safe default)
• app.js uses data.allow_always !== false — missing field defaults to showing the button
• Boxes PendingApproval in AgenticLoopResult::NeedApproval to fix a pre-existing clippy large_enum_variant warning

Review Track: C

Touches src/agent/ (dispatcher, thread_ops, session).

Rollback plan

Revert the single commit. Serde defaults ensure no deserialization breakage.

Test plan

• All 3164 existing unit tests pass
• Zero clippy warnings (--all-features)
• cargo fmt clean
• Regression test: test_credentialed_requests_respect_auto_approve — asserts HTTP with credentials returns UnlessAutoApproved, not Always
• Regression test: test_allow_always_matches_approval_requirement — verifies allow_always computation for all three ApprovalRequirement variants
• All existing HTTP approval tests updated to assert UnlessAutoApproved
• Manual: trigger credentialed HTTP tool call, say "always", verify next call skips approval
• Manual: trigger destructive shell command (rm -rf), verify "always" option is NOT shown

Test gaps

1. No integration test for the full round-trip (user says "always" → session stores → next call skips). Unit tests cover each link individually.
2. No test that saying "always" on an Always-tier tool has no effect on future calls (covered by existing test_always_approval_requirement_bypasses_session_auto_approve at the dispatcher logic level, but not end-to-end).

Files changed (13)

File	Change
tools/builtin/http.rs	Root cause: Always → UnlessAutoApproved for credentials
agent/session.rs	allow_always: bool on PendingApproval
agent/submission.rs	allow_always on SubmissionResult::NeedApproval
agent/dispatcher.rs	Computes allow_always, threads through, boxes PendingApproval
agent/thread_ops.rs	Threads allow_always through 3 approval emission sites
channels/channel.rs	allow_always on StatusUpdate::ApprovalNeeded
channels/repl.rs	Conditionally shows "always" in CLI
channels/signal.rs	Conditionally shows "always" in Signal
channels/wasm/wrapper.rs	Conditionally shows "always" in Telegram/Slack (2 paths)
channels/relay/channel.rs	Destructures new field (Slack buttons unchanged)
channels/web/mod.rs	Passes allow_always to SSE event
channels/web/types.rs	allow_always on SseEvent::ApprovalNeeded
channels/web/static/app.js	Conditionally renders "always" button

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a bug where the 'always' auto-approve feature was ineffective for HTTP requests containing credentials. Previously, these requests were incorrectly classified as requiring explicit approval every time, bypassing the auto-approval mechanism. The changes reclassify credentialed HTTP requests to allow for auto-approval, making the 'always' option functional. Additionally, the system now intelligently suppresses the 'always' approval button in user interfaces for tools that truly necessitate per-invocation approval, enhancing the user experience and security posture.

Highlights

• Corrected Auto-Approval for Credentialed HTTP Requests: Credentialed HTTP requests now correctly return UnlessAutoApproved instead of Always, ensuring that the 'always' auto-approve option functions as intended and is no longer ignored.
• Dynamic 'Always' Option Visibility: User interfaces across all channels (Telegram, Slack, Signal, REPL, Web) now conditionally hide the 'always' approval option when a tool inherently requires ApprovalRequirement::Always (e.g., destructive shell commands), improving clarity and security.
• Propagation of 'allow_always' Flag: A new allow_always: bool field has been introduced and threaded through StatusUpdate::ApprovalNeeded, PendingApproval, and SubmissionResult::NeedApproval to control the visibility of the 'always' option throughout the system.
• Clippy Warning Resolution: The PendingApproval struct is now boxed within AgenticLoopResult to resolve a pre-existing large_enum_variant clippy warning.

Changelog

• src/agent/dispatcher.rs
  • Wrapped PendingApproval in Box within AgenticLoopResult::NeedApproval to address a clippy warning.
  • Adjusted LoopOutcome::NeedApproval handling to accommodate the boxed PendingApproval.
  • Modified ChatDelegate to pass an additional bool (allow_always) when determining approval.
  • Updated approval logic to derive allow_always from the ApprovalRequirement.
  • Passed allow_always to the PendingApproval creation.
  • Added a new test test_allow_always_matches_approval_requirement to verify the allow_always logic.
  • Updated an existing test case to include allow_always: true in PendingApproval serialization.
• src/agent/session.rs
  • Added allow_always: bool field to PendingApproval struct with a default_true serialization default.
  • Introduced default_true helper function for allow_always.
  • Updated test cases to explicitly set allow_always for PendingApproval.
• src/agent/submission.rs
  • Added allow_always: bool field to SubmissionResult::NeedApproval enum variant.
• src/agent/thread_ops.rs
  • Extracted allow_always from pending before calling thread.await_approval.
  • Passed allow_always to send_status and SubmissionResult::NeedApproval.
  • Modified approval_needed tuple to include allow_always.
  • Updated logic for deferred tool calls to determine and pass allow_always.
• src/channels/channel.rs
  • Added allow_always: bool field to StatusUpdate::ApprovalNeeded enum variant with documentation.
• src/channels/relay/channel.rs
  • Updated pattern matching for StatusUpdate::ApprovalNeeded to ignore the new allow_always field.
  • Updated test cases to include allow_always: true for StatusUpdate::ApprovalNeeded.
• src/channels/repl.rs
  • Modified StatusUpdate::ApprovalNeeded handling to destructure allow_always.
  • Conditionally displayed the 'always' approval option in the REPL UI based on allow_always.
• src/channels/signal.rs
  • Modified StatusUpdate::ApprovalNeeded handling to destructure allow_always.
  • Conditionally formatted the Signal message to include the 'always' option based on allow_always.
• src/channels/wasm/wrapper.rs
  • Modified StatusUpdate::ApprovalNeeded handling to destructure allow_always.
  • Conditionally generated the approval prompt to include or exclude the 'always' option based on allow_always.
  • Updated status_to_wit function to conditionally include 'always' in the reply hint.
  • Updated test cases to include allow_always: true for StatusUpdate::ApprovalNeeded.
• src/channels/web/mod.rs
  • Modified StatusUpdate::ApprovalNeeded handling to destructure allow_always.
  • Passed allow_always to the SseEvent::ApprovalNeeded variant.
• src/channels/web/static/app.js
  • Implemented conditional rendering of the 'always' approval button in the web UI based on the allow_always data.
• src/channels/web/types.rs
  • Added an allow_always boolean field to the SseEvent::ApprovalNeeded struct for web UI communication.
  • Updated test case to include allow_always: true for SseEvent::ApprovalNeeded.
• src/tools/builtin/http.rs
  • Changed the return value of requires_approval for credentialed HTTP requests from ApprovalRequirement::Always to ApprovalRequirement::UnlessAutoApproved.
  • Updated multiple test case names and assertions to reflect the change from Always to UnlessAutoApproved for credentialed requests.
  • Added a new regression test test_credentialed_requests_respect_auto_approve to specifically verify this behavior.

Activity

• The pull request author, nearfamiliarcow, has provided a detailed summary and test plan for these changes.
• The changes are categorized under 'Review Track: C', indicating they primarily affect the src/agent/ directory, specifically the dispatcher, thread operations, and session management.
• The author has confirmed that all 3164 existing unit tests pass and there are zero clippy warnings with --all-features.
• Several regression tests have been added and updated to cover the new logic for UnlessAutoApproved and allow_always, including test_credentialed_requests_respect_auto_approve and test_allow_always_matches_approval_requirement.
• Manual testing is planned to verify the user experience for triggering a credentialed HTTP tool call, saying 'always', and confirming subsequent calls skip approval, as well as verifying the 'always' option is not shown for destructive shell commands.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses the issue where the 'always' auto-approve option didn't work for credentialed HTTP requests. The root cause is correctly identified and fixed by changing the approval requirement from Always to UnlessAutoApproved.

The changes are comprehensive, propagating the allow_always flag through various layers of the application to ensure the UI correctly hides the 'always' option for destructive commands. The addition of new regression tests and updates to existing ones are well done and improve the robustness of the codebase.

I have one minor suggestion to improve the clarity of a new test case, but overall, this is an excellent and well-executed fix.

[zmanian]

Review: APPROVE

Excellent PR -- correct fix direction, well-tested, great PR description explaining the three-tier approval model.

What's good

• Preserves the Always invariant (destructive ops still require per-invocation approval)
• Downgrades credentialed HTTP from Always to UnlessAutoApproved -- correct risk classification
• UI fix: hides "always" button when ApprovalRequirement::Always is in effect
• Backward compatible: #[serde(default = "default_true")] on PendingApproval.allow_always
• Boxing PendingApproval in NeedApproval addresses pre-existing clippy warning
• Good regression tests

Minor requests (non-blocking)

1. Relay channel discards allow_always -- destructured as allow_always: _ in relay/channel.rs. Slack buttons would still show "always" for ApprovalRequirement::Always tools. Add a comment explaining why relay is exempt, or thread the field through.

2. Duplicated allow_always computation -- !matches!(requirement, ApprovalRequirement::Always) appears in both dispatcher.rs and thread_ops.rs. Consider centralizing as ApprovalRequirement::allows_always() method.

3. Consider documenting the approval tier model on ApprovalRequirement itself -- the PR description explains it beautifully but this knowledge shouldn't live only in PR history.

Security analysis: positive. No privilege escalation, Always invariant preserved, session-scoped, allow_always computed server-side and not client-manipulable.

CI all green.