From 2b72531eaafe65bc9d7929d4e8549d702bd57518 Mon Sep 17 00:00:00 2001
From: RaresKeY <158580472+RaresKeY@users.noreply.github.com>
Date: Sat, 15 Aug 2026 06:52:44 +0000
Subject: [PATCH] fix(agent): harden approval lifecycle
---
routes/chat_routes.py | 18 +-
src/agent_loop.py | 98 +++++++----
src/agent_tools/document_tools.py | 12 ++
src/task_scheduler.py | 36 ++++
src/teacher_escalation.py | 25 ++-
src/tool_approvals.py | 21 ++-
src/tool_capabilities.py | 36 +++-
src/tool_execution.py | 20 +++
static/app.js | 8 +-
static/index.html | 16 +-
static/js/chat.js | 120 +++++++++----
static/js/chatRenderer.js | 7 +-
static/js/chatStream.js | 6 +-
static/js/document.js | 12 +-
static/js/emailInbox.js | 2 +-
static/js/emailLibrary.js | 4 +-
static/js/sessions.js | 2 +-
static/js/settings.js | 2 +-
static/js/slashCommands.js | 2 +-
tests/test_external_context_tool_gate.py | 185 ++++++++++++++++++++-
tests/test_foreground_model_routing.py | 45 +++++
tests/test_scheduler_prompt_cache_time.py | 46 +++++
tests/test_startup_session_bootstrap_js.py | 2 +-
tests/test_teacher_eval_tier2.py | 82 +++++++++
tests/test_tool_approvals.py | 91 +++++++++-
25 files changed, 782 insertions(+), 116 deletions(-)
diff --git a/routes/chat_routes.py b/routes/chat_routes.py
index 5dfae8c1a..4d92e2e72 100644
--- a/routes/chat_routes.py
+++ b/routes/chat_routes.py
@@ -915,6 +915,7 @@ def setup_chat_routes(
or (body or {}).get("tool_approval_decision")
)
exact_tool_approval = None
+ tool_approval_continuation = False
# Workspace: confine the agent's file/shell tools to this folder.
workspace, workspace_rejected = _resolve_request_workspace(
request, form_data.get("workspace")
@@ -1087,6 +1088,7 @@ def setup_chat_routes(
owner=owner,
session_id=session,
)
+ tool_approval_continuation = True
if decision == "approve" and exact_tool_approval is None:
raise HTTPException(
409,
@@ -1186,14 +1188,24 @@ def setup_chat_routes(
resolve_session_auth(sess, session, owner=effective_user(request))
# Check for research_pending BEFORE mode persist overwrites it
- do_research = str(use_research).lower() == "true"
- if not do_research:
+ # An approval response resumes the sealed agent action. Do not let
+ # mutable form fields, or a stale research_pending session marker,
+ # consume the one-use grant on the unrelated research path.
+ do_research = (
+ not tool_approval_continuation
+ and str(use_research).lower() == "true"
+ )
+ if not do_research and not tool_approval_continuation:
if get_session_mode(session) == 'research_pending':
do_research = True
logger.info(f"Session {session} in research_pending — auto-triggering research")
att_ids = []
- if body and isinstance(body.get("attachments"), list):
+ if tool_approval_continuation:
+ # Browser composer state is unrelated to the action that was
+ # reviewed. The original turn remains in session history.
+ att_ids = []
+ elif body and isinstance(body.get("attachments"), list):
att_ids = [str(x) for x in body["attachments"]]
elif attachments:
try:
diff --git a/src/agent_loop.py b/src/agent_loop.py
index cfe405515..d79e645a8 100644
--- a/src/agent_loop.py
+++ b/src/agent_loop.py
@@ -5629,40 +5629,64 @@ async def stream_agent_loop(
in {"edit_document", "suggest_document", "update_document"}
else None
)
- pending_approval = tool_approval_store.create(
- owner=owner,
- session_id=session_id,
- origin_run_id=run_security.run_id,
- tool_name=block.tool_type,
- content=block.content,
- workspace=workspace,
- document_id=getattr(approval_document, "id", None),
- document_version=getattr(
- approval_document,
- "version_count",
- None,
- ),
- external_untrusted_context_seen=(
- run_security.external_untrusted_context_seen
- ),
- capabilities=capabilities_for_action(
+ if (
+ block.tool_type
+ in {"edit_document", "suggest_document", "update_document"}
+ and (
+ approval_document is None
+ or getattr(approval_document, "id", None) is None
+ or getattr(approval_document, "version_count", None) is None
+ )
+ ):
+ # These legacy tools otherwise fall back to a process-global
+ # or most-recent document at dispatch time. That target can
+ # change while an approval card is pending, so there is no
+ # exact action to seal until the user opens a real document.
+ desc = f"{block.tool_type}: BLOCKED"
+ result = {
+ "error": (
+ "Open the exact document to edit, then request this "
+ "action again so its id and version can be sealed."
+ ),
+ "exit_code": 1,
+ "blocked": True,
+ "policy": "exact_tool_approval_target",
+ }
+ else:
+ pending_approval = tool_approval_store.create(
+ owner=owner,
+ session_id=session_id,
+ origin_run_id=run_security.run_id,
+ tool_name=block.tool_type,
+ content=block.content,
+ workspace=workspace,
+ document_id=getattr(approval_document, "id", None),
+ document_version=getattr(
+ approval_document,
+ "version_count",
+ None,
+ ),
+ external_untrusted_context_seen=(
+ run_security.external_untrusted_context_seen
+ ),
+ capabilities=capabilities_for_action(
+ block.tool_type,
+ block.content,
+ ),
+ )
+ desc = f"{block.tool_type}: APPROVAL REQUIRED"
+ result = {
+ "output": "Waiting for an exact user approval.",
+ "exit_code": None,
+ "approval_required": True,
+ "ask_user": pending_approval.public_payload(
+ reason=security_decision.reason,
+ ),
+ }
+ logger.info(
+ "Exact approval required before tool start: %s",
block.tool_type,
- block.content,
- ),
- )
- desc = f"{block.tool_type}: APPROVAL REQUIRED"
- result = {
- "output": "Waiting for an exact user approval.",
- "exit_code": None,
- "approval_required": True,
- "ask_user": pending_approval.public_payload(
- reason=security_decision.reason,
- ),
- }
- logger.info(
- "Exact approval required before tool start: %s",
- block.tool_type,
- )
+ )
elif tool_policy and tool_policy.blocks(block.tool_type) and not _ody_clamped_tool_allowed:
desc = f"{block.tool_type}: BLOCKED"
result = {
@@ -6325,7 +6349,7 @@ async def stream_agent_loop(
# gets a turn (with its own tool calls forwarded to the user) and
# a skill is saved ONLY if the teacher actually succeeds. Skipped
# when we ARE the teacher to avoid recursion.
- if not _is_teacher_run and not guide_only:
+ if not _is_teacher_run and not guide_only and not _awaiting_user:
try:
from src.teacher_escalation import run_teacher_inline
async for evt in run_teacher_inline(
@@ -6334,6 +6358,12 @@ async def stream_agent_loop(
student_tool_events=tool_events,
student_reply=full_response,
owner=owner,
+ session_id=session_id,
+ workspace=workspace,
+ disabled_tools=disabled_tools,
+ tool_policy=tool_policy,
+ active_document=active_document,
+ active_email=active_email,
):
yield evt
except Exception as _esc_err:
diff --git a/src/agent_tools/document_tools.py b/src/agent_tools/document_tools.py
index a3134bc6f..8ffe24c93 100644
--- a/src/agent_tools/document_tools.py
+++ b/src/agent_tools/document_tools.py
@@ -475,6 +475,12 @@ class UpdateDocumentTool:
doc = None
if target_id:
doc = _get_owned_document(db, Document, target_id, owner)
+ if (
+ not doc
+ and target_id
+ and ctx.get("expected_document_version") is not None
+ ):
+ return _approved_document_version_error(None, ctx)
if not doc:
doc = _most_recent_owned_document(db, Document, owner)
if doc:
@@ -555,6 +561,12 @@ class EditDocumentTool:
doc = None
if target_id:
doc = _get_owned_document(db, Document, target_id, owner)
+ if (
+ not doc
+ and target_id
+ and ctx.get("expected_document_version") is not None
+ ):
+ return _approved_document_version_error(None, ctx)
if not doc:
# Fallback: most recently updated document. Avoids "no active doc" errors
# after server restart or when the agent loses track of which doc to edit.
diff --git a/src/task_scheduler.py b/src/task_scheduler.py
index d5b1dad62..7403ff16d 100644
--- a/src/task_scheduler.py
+++ b/src/task_scheduler.py
@@ -1883,6 +1883,7 @@ class TaskScheduler:
pass
full_text = ""
tool_results = []
+ approval_pause = None
# Honor per-task max_steps (defense against runaway agent loops).
# Falls back to 20 if not set — the historical default.
@@ -1929,9 +1930,44 @@ class TaskScheduler:
tool_summary = data.get("stdout") or data.get("output") or data.get("result") or ""
if isinstance(tool_summary, str) and tool_summary.strip():
tool_results.append(f"[{data.get('tool', '?')}] {tool_summary[:500]}")
+ approval = data.get("ask_user")
+ if (
+ isinstance(approval, dict)
+ and approval.get("kind") == "tool_approval"
+ ):
+ approval_pause = {
+ "tool": data.get("tool") or "tool",
+ "approval_id": approval.get("approval_id"),
+ }
+ # Scheduled tasks have no interactive surface that
+ # can safely resume a one-use grant. Retire the
+ # record immediately instead of leaving it pending
+ # and report an explicit manual-action boundary.
+ try:
+ from src.tool_approvals import tool_approval_store
+ tool_approval_store.consume(
+ approval_pause["approval_id"],
+ decision="deny",
+ owner=task.owner,
+ session_id=session_id,
+ )
+ except Exception:
+ logger.debug(
+ "Could not retire scheduled-task approval",
+ exc_info=True,
+ )
+ break
except (json.JSONDecodeError, KeyError):
pass
+ if approval_pause is not None:
+ return (
+ "Scheduled task paused safely: "
+ f"{approval_pause['tool']} requested an exact action after "
+ "untrusted context. That action was not executed. Run this task "
+ "interactively to inspect and approve the action."
+ )
+
# Grace summarization — if the model exhausted rounds on tool calls
# without producing a final text response, do one last LLM call
# asking it to summarize what it did. Guarantees output.
diff --git a/src/teacher_escalation.py b/src/teacher_escalation.py
index 4b6206a8d..59af4857d 100644
--- a/src/teacher_escalation.py
+++ b/src/teacher_escalation.py
@@ -563,6 +563,12 @@ async def run_teacher_inline(
student_tool_events: List[Dict[str, Any]],
student_reply: str,
owner: Optional[str] = None,
+ session_id: Optional[str] = None,
+ workspace: Optional[str] = None,
+ disabled_tools: Optional[set[str]] = None,
+ tool_policy: Any = None,
+ active_document: Any = None,
+ active_email: Optional[Dict[str, str]] = None,
):
"""Async generator. Yields SSE event strings.
@@ -668,6 +674,12 @@ async def run_teacher_inline(
messages=teacher_messages,
headers=teacher_headers,
owner=owner,
+ session_id=session_id,
+ workspace=workspace,
+ disabled_tools=disabled_tools,
+ tool_policy=tool_policy,
+ active_document=active_document,
+ active_email=active_email,
_is_teacher_run=True,
):
# Swallow teacher's own [DONE] — outer loop emits the real one
@@ -683,12 +695,15 @@ async def run_teacher_inline(
payload["teacher"] = True
typ = payload.get("type")
if typ == "tool_output":
- captured_tool_events.append({
+ captured_tool_event = {
"tool": payload.get("tool"),
"command": payload.get("command"),
"output": payload.get("output"),
"exit_code": payload.get("exit_code"),
- })
+ }
+ if isinstance(payload.get("ask_user"), dict):
+ captured_tool_event["ask_user"] = payload["ask_user"]
+ captured_tool_events.append(captured_tool_event)
if "delta" in payload and isinstance(payload["delta"], str):
if payload.get("thinking"):
continue
@@ -697,6 +712,12 @@ async def run_teacher_inline(
continue
yield evt_str
+ # A takeover that paused for a question or exact action has not completed
+ # yet. Its server-owned approval card is already in the live/persisted tool
+ # events; do not evaluate the partial trace or distill it into a skill.
+ if any(event.get("ask_user") for event in captured_tool_events):
+ return
+
teacher_text = "".join(captured_text_parts).strip()
t_status, t_reason = evaluate_turn_regex(captured_tool_events, teacher_text)
if t_status == "failure":
diff --git a/src/tool_approvals.py b/src/tool_approvals.py
index 2600edc26..d82b651d1 100644
--- a/src/tool_approvals.py
+++ b/src/tool_approvals.py
@@ -323,14 +323,19 @@ class ToolApprovalStore:
now = time.time()
with self._lock:
self._purge_expired_locked(now)
- pending = self._pending.pop(str(approval_id or ""), None)
- if pending is None:
- return None
- if (
- pending.owner != _normalized_owner(owner)
- or pending.session_id != str(session_id or "")
- ):
- return None
+ approval_key = str(approval_id or "")
+ pending = self._pending.get(approval_key)
+ if pending is None:
+ return None
+ if (
+ pending.owner != _normalized_owner(owner)
+ or pending.session_id != str(session_id or "")
+ ):
+ # Authentication is checked before destructive consumption so
+ # a leaked/guessed opaque id cannot be used to invalidate
+ # another owner's pending action.
+ return None
+ self._pending.pop(approval_key, None)
if str(decision or "").strip().lower() != "approve":
return None
return ExactToolApproval(pending)
diff --git a/src/tool_capabilities.py b/src/tool_capabilities.py
index bdbd50f23..5134b998c 100644
--- a/src/tool_capabilities.py
+++ b/src/tool_capabilities.py
@@ -332,6 +332,26 @@ _PRIVATE_ACTION_WRITES: Mapping[str, frozenset[str]] = MappingProxyType(
}
)
+_ACTION_DESTRUCTIVE: Mapping[str, frozenset[str]] = MappingProxyType(
+ {
+ "manage_calendar": frozenset({"delete_event"}),
+ "manage_contact": frozenset({"delete"}),
+ "manage_documents": frozenset({"delete", "tidy"}),
+ "manage_endpoints": frozenset({"delete"}),
+ "manage_bg_jobs": frozenset({"kill", "stop", "cancel", "terminate"}),
+ "manage_memory": frozenset({"delete"}),
+ "manage_mcp": frozenset({"delete"}),
+ "manage_notes": frozenset({"delete"}),
+ "manage_research": frozenset({"delete"}),
+ "manage_session": frozenset({"delete", "truncate"}),
+ "manage_settings": frozenset({"delete", "reset"}),
+ "manage_skills": frozenset({"delete"}),
+ "manage_tasks": frozenset({"delete"}),
+ "manage_tokens": frozenset({"delete"}),
+ "manage_webhooks": frozenset({"delete"}),
+ }
+)
+
_ACTION_DEFAULTS: Mapping[str, str] = MappingProxyType(
{
"manage_calendar": "list_events",
@@ -415,18 +435,30 @@ def _action_from_content(tool_name: str, content: Any) -> str | None:
def capabilities_for_action(tool_name: Any, content: Any) -> ToolCapabilities:
"""Classify a sealed multiplexed action; ambiguous actions fail high."""
base = capabilities_for_tool(tool_name)
- if not isinstance(tool_name, str) or tool_name not in _PRIVATE_ACTION_READS:
+ if not isinstance(tool_name, str):
return base
action = _action_from_content(tool_name, content)
+ destructive = action in _ACTION_DESTRUCTIVE.get(tool_name, ())
+ if tool_name not in _PRIVATE_ACTION_READS:
+ if not destructive:
+ return base
+ return ToolCapabilities(
+ frozenset(set(base.effects) | {ToolEffect.DESTRUCTIVE}),
+ base.result_integrity,
+ known=base.known,
+ )
if action in _PRIVATE_ACTION_READS[tool_name]:
return _capabilities(
ToolEffect.READ_PRIVATE,
result_integrity=ResultIntegrity.EXTERNAL_UNTRUSTED,
)
if action in _PRIVATE_ACTION_WRITES[tool_name]:
+ effects = set(base.effects)
+ if destructive:
+ effects.add(ToolEffect.DESTRUCTIVE)
return ToolCapabilities(
- base.effects,
+ frozenset(effects),
ResultIntegrity.EXTERNAL_UNTRUSTED,
known=base.known,
)
diff --git a/src/tool_execution.py b/src/tool_execution.py
index f4e80d3fe..f33f8c3fa 100644
--- a/src/tool_execution.py
+++ b/src/tool_execution.py
@@ -639,6 +639,26 @@ async def execute_tool_block(
"policy": "exact_tool_approval",
},
)
+ if (
+ exact_approval.pending.tool_name
+ in {"edit_document", "suggest_document", "update_document"}
+ and (
+ not exact_approval.pending.document_id
+ or exact_approval.pending.document_version is None
+ )
+ ):
+ return (
+ f"{getattr(block, 'tool_type', None)}: BLOCKED",
+ {
+ "error": (
+ "The approved document action has no sealed target and "
+ "cannot be executed."
+ ),
+ "exit_code": 1,
+ "blocked": True,
+ "policy": "exact_tool_approval",
+ },
+ )
sealed_workspace = exact_approval.pending.workspace
if sealed_workspace and vet_workspace(sealed_workspace) != sealed_workspace:
return (
diff --git a/static/app.js b/static/app.js
index 3aa8dd337..0eeb2c39b 100644
--- a/static/app.js
+++ b/static/app.js
@@ -10,9 +10,9 @@ import modelsModule from './js/models.js?v=20260715startupcalm2';
import ragModule from './js/rag.js';
import presetsModule from './js/presets.js';
import searchModule from './js/search.js';
-import chatModule from './js/chat.js?v=20260815toolapproval3';
+import chatModule from './js/chat.js?v=20260815toolapproval4';
import compareModule from './js/compare/index.js?v=20260723compareicon2';
-import documentModule from './js/document.js?v=20260722emailfastindex1';
+import documentModule from './js/document.js?v=20260815approvalsave1';
import searchChatModule from './js/search-chat.js';
import { makeWindowDraggable } from './js/windowDrag.js';
import {
@@ -22,7 +22,7 @@ import {
settleSessionHydration
} from './js/startupShell.js';
import markdownModule from './js/markdown.js';
-import chatRenderer from './js/chatRenderer.js?v=20260815toolapproval3';
+import chatRenderer from './js/chatRenderer.js?v=20260815toolapproval4';
import sessionModule from './js/sessions.js';
import memoryModule from './js/memory.js?v=20260722memoryloading1';
import voiceRecorderModule from './js/voiceRecorder.js';
@@ -33,7 +33,7 @@ import tasksModule from './js/tasks.js?v=20260723tasksbulkfeedback1';
import calendarModule from './js/calendar.js';
import notesModule from './js/notes.js';
import adminModule from './js/admin.js?v=20260716openrouter3';
-import settingsModule from './js/settings.js?v=20260722emailfastindex1';
+import settingsModule from './js/settings.js?v=20260815approvalsave1';
// Eagerly bind unified minimize/restore behavior across all tool modals.
import './js/modalManager.js?v=20260723compareicon2';
// Desktop window tiling — drag a modal near an edge/corner to snap.
diff --git a/static/index.html b/static/index.html
index f377fd143..df133010d 100644
--- a/static/index.html
+++ b/static/index.html
@@ -258,8 +258,8 @@
-
-
+
+
@@ -2532,20 +2532,20 @@
-
+
-
+
-
-
+
+
-
+
-
+
diff --git a/static/js/chat.js b/static/js/chat.js
index 387ba31d1..31f632636 100644
--- a/static/js/chat.js
+++ b/static/js/chat.js
@@ -8,18 +8,18 @@
import Storage from './storage.js';
import uiModule from './ui.js';
import sessionModule from './sessions.js';
-import chatRenderer from './chatRenderer.js?v=20260815toolapproval3';
-import chatStream from './chatStream.js';
+import chatRenderer from './chatRenderer.js?v=20260815toolapproval4';
+import chatStream from './chatStream.js?v=20260815approvalsave1';
import { addAITTSButton } from './tts-ai.js';
import markdownModule from './markdown.js';
import spinnerModule from './spinner.js';
import presetsModule from './presets.js';
import fileHandlerModule from './fileHandler.js';
import searchModule from './search.js';
-import documentModule from './document.js?v=20260722emailfastindex1';
-import * as emailInbox from './emailInbox.js?v=20260722emailfastindex1';
+import documentModule from './document.js?v=20260815approvalsave1';
+import * as emailInbox from './emailInbox.js?v=20260815approvalsave1';
import codeRunnerModule from './codeRunner.js';
-import slashCommands, { initSlashCommands, isCommand, handleSlashCommand, handleSetupInput, handleSetupWizard, typewriterInto } from './slashCommands.js?v=20260722emailfastindex1';
+import slashCommands, { initSlashCommands, isCommand, handleSlashCommand, handleSetupInput, handleSetupWizard, typewriterInto } from './slashCommands.js?v=20260815approvalsave1';
import createResearchSynapse from './researchSynapse.js';
import { createStreamRenderer } from './streamingRenderer.js';
import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArrowUpRecall.js?v=20260714promptrecall';
@@ -72,6 +72,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
}
const input = document.getElementById('message');
if (input) {
+ _pendingToolApproval.draft = input.value || '';
input.value = label;
input.dispatchEvent(new Event('input', { bubbles: true }));
}
@@ -86,6 +87,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
_pendingToolApproval = {
approval_id: String(detail.approval_id),
decision,
+ document_id: String(detail.document_id || ''),
};
_submitToolApprovalWhenIdle(
_pendingToolApproval.approval_id,
@@ -1267,6 +1269,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
if (_sendInFlight) return;
const _sendPerf = _createChatSendPerf();
_sendInFlight = true;
+ const approvalForSend = _pendingToolApproval;
_setForegroundChatBusy(true);
// Instant visual feedback so the user sees their click was accepted
// even before the streaming button state kicks in below.
@@ -1281,7 +1284,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
};
// --- Setup mode: intercept next message (but let slash commands through) ---
- {
+ if (!approvalForSend) {
const el = uiModule.el;
const rawMsg = (el('message').value || '').trim();
const currentSetupMode = slashCommands.getSetupMode();
@@ -1311,7 +1314,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
if (!msg.trim() && !fileHandlerModule.getPendingCount() && !(_pendingRegenAttachments && _pendingRegenAttachments.length)) { _releaseSendFlag(); return; }
// --- Slash commands: execute directly without AI (no session needed) ---
- if (isCommand(msg.trim())) {
+ if (!approvalForSend && isCommand(msg.trim())) {
const handled = await handleSlashCommand(msg.trim());
if (handled) {
el('message').value = '';
@@ -1438,7 +1441,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
}
// --- API key guard: warn if message looks like an API key ---
- if (API_KEY_RE.test(msg.trim())) {
+ if (!approvalForSend && API_KEY_RE.test(msg.trim())) {
if (!await window.styledConfirm('This looks like an API key. Sending it to the AI could expose it.\n\nDid you mean to use /setup instead?', { confirmText: 'Send anyway', danger: true })) {
_releaseSendFlag();
return;
@@ -1573,7 +1576,9 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
if (sessionModule.clearStreamComplete) sessionModule.clearStreamComplete(sessionModule.getCurrentSessionId());
// Check for document selection context before consuming display override
- const docSel = documentModule && documentModule.getSelectionContext();
+ const docSel = !approvalForSend && documentModule
+ ? documentModule.getSelectionContext()
+ : null;
if (docSel) {
const sels = Array.isArray(docSel) ? docSel : [docSel];
const lineRefs = sels.map(s =>
@@ -1593,7 +1598,9 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
// stuck flag can't silently eat the next turn's recovery budget.
if (!skipBubble) { _autoNudges = 0; _autoContinuePending = false; }
else if (_autoContinuePending) { _autoContinuePending = false; }
- const _pendingAttachInfo = fileHandlerModule.getPendingCount() ? fileHandlerModule.getPendingInfo() : null;
+ const _pendingAttachInfo = !approvalForSend && fileHandlerModule.getPendingCount()
+ ? fileHandlerModule.getPendingInfo()
+ : null;
// Pre-read importable file contents before upload clears pending files
const IMPORTABLE_EXT = /\.(txt|py|js|ts|html|htm|css|md|json|csv|yml|yaml|sh|sql|rs|go|java|c|cpp|h|rb|php|xml|jsx|tsx|log|toml|ini|conf|env|vue|svelte|scss|sass|less)$/i;
const _importableFiles = [];
@@ -1611,7 +1618,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
_userMsgEl = addMessage('user', userDisplay, null, _pendingAttachInfo ? { attachments: _pendingAttachInfo } : null);
}
_sendPerf.mark('user_bubble_visible');
- messageInput.value = '';
+ messageInput.value = approvalForSend ? (approvalForSend.draft || '') : '';
messageInput.style.height = '';
messageInput.dispatchEvent(new Event('input'));
// Mobile: dismiss the on-screen keyboard after sending. iOS in
@@ -1645,13 +1652,15 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
}
let ids = [];
- try {
- _sendPerf.mark('upload_begin');
- ids = await fileHandlerModule.uploadPending({ sessionId: sessionModule.getCurrentSessionId() });
- _sendPerf.mark('upload_done');
- } catch(e) {
- console.error('upload failed', e);
- _sendPerf.mark('upload_failed');
+ if (!approvalForSend) {
+ try {
+ _sendPerf.mark('upload_begin');
+ ids = await fileHandlerModule.uploadPending({ sessionId: sessionModule.getCurrentSessionId() });
+ _sendPerf.mark('upload_done');
+ } catch(e) {
+ console.error('upload failed', e);
+ _sendPerf.mark('upload_failed');
+ }
}
if (_pendingAttachInfo && !ids.length && !(_pendingRegenAttachments && _pendingRegenAttachments.length)) {
if (_userMsgEl && _userMsgEl.parentNode) _userMsgEl.remove();
@@ -1668,10 +1677,10 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
// edited OCR text via the server-side .vision cache). Always CONSUME the
// slot — even when empty / errored — so the regen ids can't bleed into
// an unrelated next message if uploadPending() above had thrown.
- if (_pendingRegenAttachments && _pendingRegenAttachments.length) {
+ if (!approvalForSend && _pendingRegenAttachments && _pendingRegenAttachments.length) {
ids = ids.concat(_pendingRegenAttachments);
}
- _pendingRegenAttachments = null;
+ if (!approvalForSend) _pendingRegenAttachments = null;
// The optimistic user bubble was rendered before the upload assigned ids,
// so image previews couldn't show (the renderer needs att.id). Now that
@@ -1752,14 +1761,50 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
if (activeEmailComposerCtx?.docId) {
activeDocIdForSend = activeEmailComposerCtx.docId;
}
- if (documentModule && activeDocIdForSend) {
+ const shouldSaveActiveDoc = !approvalForSend || (
+ approvalForSend.document_id
+ && approvalForSend.document_id === activeDocIdForSend
+ );
+ if (documentModule && activeDocIdForSend && shouldSaveActiveDoc) {
try {
_sendPerf.mark('doc_save_begin');
- await documentModule.saveDocument();
+ const documentSaved = await documentModule.saveDocument({
+ silent: !!approvalForSend,
+ });
_sendPerf.mark('doc_save_done');
+ if (approvalForSend && documentSaved === false) {
+ if (_userMsgEl && _userMsgEl.parentNode) _userMsgEl.remove();
+ if (
+ _pendingToolApproval
+ && _pendingToolApproval.approval_id === approvalForSend.approval_id
+ ) {
+ _pendingToolApproval = null;
+ }
+ uiModule.showError && uiModule.showError(
+ 'Document could not be saved, so the action was not approved. Reload the chat to retry.'
+ );
+ updateSubmitButton('idle', submitBtn);
+ _releaseSendFlag();
+ return;
+ }
} catch(e) {
console.warn('doc auto-save failed', e);
_sendPerf.mark('doc_save_failed');
+ if (approvalForSend) {
+ if (_userMsgEl && _userMsgEl.parentNode) _userMsgEl.remove();
+ if (
+ _pendingToolApproval
+ && _pendingToolApproval.approval_id === approvalForSend.approval_id
+ ) {
+ _pendingToolApproval = null;
+ }
+ uiModule.showError && uiModule.showError(
+ 'Document could not be saved, so the action was not approved. Reload the chat to retry.'
+ );
+ updateSubmitButton('idle', submitBtn);
+ _releaseSendFlag();
+ return;
+ }
}
}
@@ -1789,23 +1834,30 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
const fd = new FormData();
fd.append('message', _finalMsgWithInject);
fd.append('session', streamSessionId);
- if (_pendingToolApproval) {
- fd.append('tool_approval_id', _pendingToolApproval.approval_id);
- fd.append('tool_approval_decision', _pendingToolApproval.decision);
- _pendingToolApproval = null;
+ if (approvalForSend) {
+ fd.append('tool_approval_id', approvalForSend.approval_id);
+ fd.append('tool_approval_decision', approvalForSend.decision);
+ if (
+ _pendingToolApproval
+ && _pendingToolApproval.approval_id === approvalForSend.approval_id
+ ) {
+ _pendingToolApproval = null;
+ }
}
if (selectedRouteForSend.model) fd.append('selected_model', selectedRouteForSend.model);
if (selectedRouteForSend.endpoint_url) fd.append('selected_endpoint_url', selectedRouteForSend.endpoint_url);
if (selectedRouteForSend.endpoint_id) fd.append('selected_endpoint_id', selectedRouteForSend.endpoint_id);
if (ids.length) fd.append('attachments', JSON.stringify(ids));
// Auto-save & send active doc ID so the backend sees latest content
- if (documentModule && activeDocIdForSend) {
- try {
- _sendPerf.mark('doc_silent_save_begin');
- await documentModule.saveDocument({ silent: true });
- _sendPerf.mark('doc_silent_save_done');
- } catch (_e) {
- _sendPerf.mark('doc_silent_save_failed');
+ if (documentModule && activeDocIdForSend && shouldSaveActiveDoc) {
+ if (!approvalForSend) {
+ try {
+ _sendPerf.mark('doc_silent_save_begin');
+ await documentModule.saveDocument({ silent: true });
+ _sendPerf.mark('doc_silent_save_done');
+ } catch (_e) {
+ _sendPerf.mark('doc_silent_save_failed');
+ }
}
fd.append('active_doc_id', activeDocIdForSend);
}
@@ -1859,7 +1911,7 @@ import { createTerminalStreamError, isRecoverableStreamError } from './chatStrea
if (isAgentMode) {
fd.append('allow_web_search', el('web-toggle').checked ? 'true' : 'false');
}
- if (el('research-toggle').checked) {
+ if (!approvalForSend && el('research-toggle').checked) {
fd.append('use_research', 'true');
// Research always runs in chat mode — override agent if set
fd.set('mode', 'chat');
diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js
index 05ab652fe..023465095 100644
--- a/static/js/chatRenderer.js
+++ b/static/js/chatRenderer.js
@@ -1367,7 +1367,7 @@ document.addEventListener('click', function(e) {
} catch {}
});
} else if (kind === 'document') {
- import('./document.js?v=20260722emailfastindex1').then(mod => {
+ import('./document.js?v=20260815approvalsave1').then(mod => {
const open = mod.loadDocument
|| mod.openDocument
|| (mod.default && (mod.default.loadDocument || mod.default.openDocument));
@@ -1389,7 +1389,7 @@ document.addEventListener('click', function(e) {
if (open) open(id);
}).catch(() => {});
} else if (kind === 'email') {
- import('./emailLibrary.js?v=20260722emailfastindex1').then(mod => {
+ import('./emailLibrary.js?v=20260815approvalsave1').then(mod => {
const open = mod.openEmailLibrary || (mod.default && mod.default.openEmailLibrary);
if (open) open({ uid: id });
}).catch(() => {});
@@ -2433,6 +2433,9 @@ export function renderAskUserCard(payload, options) {
approval_id: aq.approval_id,
decision: String((opt && opt.value) || '').toLowerCase(),
label,
+ document_id: aq.action && aq.action.document_id
+ ? String(aq.action.document_id)
+ : '',
},
}));
} else {
diff --git a/static/js/chatStream.js b/static/js/chatStream.js
index 2839b3231..19bf66753 100644
--- a/static/js/chatStream.js
+++ b/static/js/chatStream.js
@@ -7,7 +7,7 @@ import Storage from './storage.js';
import themeModule from './theme.js';
import markdownModule from './markdown.js';
import sessionModule from './sessions.js';
-import documentModule from './document.js?v=20260722emailfastindex1';
+import documentModule from './document.js?v=20260815approvalsave1';
/**
* Handle a ui_control SSE event — AI-driven UI manipulation.
@@ -156,7 +156,7 @@ export function handleUIControl(uiData) {
if (fn) fn();
}).catch(function(){});
} else if (panel === 'email') {
- import('./emailLibrary.js?v=20260722emailfastindex1').then(function(mod) {
+ import('./emailLibrary.js?v=20260815approvalsave1').then(function(mod) {
var fn = mod.openEmailLibrary || (mod.default && mod.default.openEmailLibrary);
if (fn) fn();
}).catch(function(){});
@@ -205,7 +205,7 @@ export function handleUIControl(uiData) {
} catch (e) {
console.warn('open_email_reply existing draft update failed:', e);
}
- import('./emailInbox.js?v=20260722emailfastindex1').then(function(mod) {
+ import('./emailInbox.js?v=20260815approvalsave1').then(function(mod) {
var fn = mod.openReplyDraft || (mod.default && mod.default.openReplyDraft);
if (fn) fn(uiData.uid, uiData.folder || 'INBOX', uiData.mode || 'reply', uiData.body || '');
}).catch(function(e) {
diff --git a/static/js/document.js b/static/js/document.js
index e0c7a7632..b51aa56a0 100644
--- a/static/js/document.js
+++ b/static/js/document.js
@@ -3934,7 +3934,7 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
leadingIcon: 'check',
action: 'View Message',
onAction: () => {
- import('./emailLibrary.js?v=20260722emailfastindex1').then(mod => {
+ import('./emailLibrary.js?v=20260815approvalsave1').then(mod => {
const open = mod.openEmailLibrary || (mod.default && mod.default.openEmailLibrary);
if (open) open({
account_id: data.account_id || activeAccountId || null,
@@ -9401,9 +9401,9 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
/** Save manual edits */
export async function saveDocument({ silent = false, forceVersion = false } = {}) {
- if (!activeDocId) return;
+ if (!activeDocId) return false;
const textarea = document.getElementById('doc-editor-textarea');
- if (!textarea) return;
+ if (!textarea) return false;
const savingDocId = activeDocId;
saveCurrentToMap();
const localDoc = docs.get(savingDocId);
@@ -9422,7 +9422,7 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
});
if (res.status === 404) {
if (silent && localDoc?.language === 'email') {
- return;
+ return false;
}
// Streaming/empty email drafts can leave a local tab pointing at a temp
// or already-deleted document. Do not keep surfacing autosave errors for
@@ -9434,7 +9434,7 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
}
_syncDocIndicator();
if (!silent && uiModule) uiModule.showError('Document no longer exists');
- return;
+ return false;
}
if (!res.ok) throw new Error(`Document save failed: HTTP ${res.status}`);
const doc = await res.json();
@@ -9447,6 +9447,7 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
}
_syncDocIndicator();
if (!silent && uiModule) uiModule.showToast(forceVersion ? 'New version saved' : 'Document saved');
+ return true;
} catch (e) {
console.error('Failed to save document:', e);
const now = Date.now();
@@ -9454,6 +9455,7 @@ import { bindMenuDismiss, dismissOrRemove } from './escMenuStack.js';
uiModule.showError(silent ? 'Autosave failed' : 'Failed to save document');
_lastAutoSaveErrorAt = now;
}
+ return false;
}
}
diff --git a/static/js/emailInbox.js b/static/js/emailInbox.js
index cfee9ff90..93ba7b6ea 100644
--- a/static/js/emailInbox.js
+++ b/static/js/emailInbox.js
@@ -5,7 +5,7 @@
import spinnerModule from './spinner.js';
import sessionModule from './sessions.js';
-import { initEmailLibrary, openEmailLibrary, closeEmailLibrary, isOpen as isLibOpen, prewarmEmailLibrary, prewarmUnreadEmails } from './emailLibrary.js?v=20260722emailfastindex1';
+import { initEmailLibrary, openEmailLibrary, closeEmailLibrary, isOpen as isLibOpen, prewarmEmailLibrary, prewarmUnreadEmails } from './emailLibrary.js?v=20260815approvalsave1';
import * as Modals from './modalManager.js';
import { applyEdgeDock } from './modalSnap.js';
import { buildReplyAllCc, extractEmail } from './emailLibrary/replyRecipients.js';
diff --git a/static/js/emailLibrary.js b/static/js/emailLibrary.js
index c56109196..91fe11d80 100644
--- a/static/js/emailLibrary.js
+++ b/static/js/emailLibrary.js
@@ -5,7 +5,7 @@
import spinnerModule from './spinner.js';
import { styledConfirm, showToast, emptyStateIcon } from './ui.js';
-import { folderDisplayName, sortedFolders } from './emailInbox.js?v=20260722emailfastindex1';
+import { folderDisplayName, sortedFolders } from './emailInbox.js?v=20260815approvalsave1';
import settingsModule from './settings.js';
import * as Modals from './modalManager.js';
import { topPortalZ } from './toolWindowZOrder.js';
@@ -6680,7 +6680,7 @@ function _wireAttachmentHandlers(reader, folder) {
ownerModal.classList.add('hidden');
}
}
- const docMod = await import('./document.js?v=20260722emailfastindex1');
+ const docMod = await import('./document.js?v=20260815approvalsave1');
const load = (docMod && docMod.loadDocument) || (docMod && docMod.default && docMod.default.loadDocument);
if (typeof load === 'function') {
await load(json.doc_id);
diff --git a/static/js/sessions.js b/static/js/sessions.js
index 057d6bbab..3ab18e037 100644
--- a/static/js/sessions.js
+++ b/static/js/sessions.js
@@ -3,7 +3,7 @@
import Storage from './storage.js';
import uiModule, { autoResize, styledPrompt } from './ui.js';
-import chatRenderer from './chatRenderer.js?v=20260815toolapproval3';
+import chatRenderer from './chatRenderer.js?v=20260815toolapproval4';
import { providerLogo } from './providers.js';
import { initModelPicker, updateModelPicker } from './modelPicker.js?v=20260722ctxheader1';
import themeModule from './theme.js';
diff --git a/static/js/settings.js b/static/js/settings.js
index 160ddac7c..e9100553d 100644
--- a/static/js/settings.js
+++ b/static/js/settings.js
@@ -2745,7 +2745,7 @@ async function initEmailAccountsSettings() {
el('set-email-open-library-settings')?.addEventListener('click', async () => {
try {
- const mod = await import('./emailLibrary.js?v=20260722emailfastindex1');
+ const mod = await import('./emailLibrary.js?v=20260815approvalsave1');
if (typeof mod.openEmailLibrarySettings === 'function') {
await mod.openEmailLibrarySettings();
}
diff --git a/static/js/slashCommands.js b/static/js/slashCommands.js
index 99529d54f..fa3210b25 100644
--- a/static/js/slashCommands.js
+++ b/static/js/slashCommands.js
@@ -16,7 +16,7 @@ import modelsModule from './models.js';
import chatRenderer from './chatRenderer.js';
import spinnerModule from './spinner.js';
import themeModule from './theme.js';
-import documentModule from './document.js?v=20260722emailfastindex1';
+import documentModule from './document.js?v=20260815approvalsave1';
import workspaceModule from './workspace.js';
import settingsModule from './settings.js';
import cookbookModule from './cookbook.js';
diff --git a/tests/test_external_context_tool_gate.py b/tests/test_external_context_tool_gate.py
index 901cf1565..c8906e4d2 100644
--- a/tests/test_external_context_tool_gate.py
+++ b/tests/test_external_context_tool_gate.py
@@ -499,6 +499,63 @@ def test_private_manager_write_aliases_keep_write_effect(tool_name, content):
assert capabilities.result_integrity is ResultIntegrity.EXTERNAL_UNTRUSTED
+@pytest.mark.parametrize(
+ "tool_name,content",
+ [
+ ("manage_calendar", '{"action":"delete_event"}'),
+ ("manage_contact", '{"action":"delete"}'),
+ ("manage_documents", '{"action":"tidy"}'),
+ ("manage_endpoints", '{"action":"delete"}'),
+ ("manage_bg_jobs", '{"action":"kill","job_id":"job-1"}'),
+ ("manage_memory", "delete\nmemory-id"),
+ ("manage_mcp", '{"action":"delete"}'),
+ ("manage_notes", '{"action":"delete"}'),
+ ("manage_research", '{"action":"delete"}'),
+ ("manage_session", "truncate\nsession-id\n10"),
+ ("manage_settings", '{"action":"reset","key":"theme"}'),
+ ("manage_skills", '{"action":"delete"}'),
+ ("manage_tasks", '{"action":"delete"}'),
+ ("manage_tokens", '{"action":"delete"}'),
+ ("manage_webhooks", '{"action":"delete"}'),
+ ],
+)
+def test_multiplexed_destructive_actions_disclose_destructive_effect(
+ tool_name,
+ content,
+):
+ capabilities = capabilities_for_action(tool_name, content)
+
+ assert any(
+ effect in capabilities.effects
+ for effect in (
+ ToolEffect.WRITE_PRIVATE,
+ ToolEffect.ADMIN_CHANGE,
+ ToolEffect.EXECUTE_CODE,
+ )
+ )
+ assert ToolEffect.DESTRUCTIVE in capabilities.effects
+
+
+@pytest.mark.parametrize(
+ "tool_name,content",
+ [
+ ("manage_bg_jobs", '{"action":"output","job_id":"job-1"}'),
+ ("manage_endpoints", '{"action":"list"}'),
+ ("manage_mcp", '{"action":"reconnect"}'),
+ ("manage_settings", '{"action":"set","key":"theme","value":"dark"}'),
+ ("manage_tokens", '{"action":"create","name":"automation"}'),
+ ("manage_webhooks", '{"action":"disable"}'),
+ ],
+)
+def test_multiplexed_non_destructive_actions_do_not_claim_destructive_effect(
+ tool_name,
+ content,
+):
+ capabilities = capabilities_for_action(tool_name, content)
+
+ assert ToolEffect.DESTRUCTIVE not in capabilities.effects
+
+
def test_ambiguous_private_manager_action_fails_high():
capabilities = capabilities_for_action("manage_notes", "not json")
@@ -894,6 +951,100 @@ def test_tainted_native_route_keeps_action_schema_for_exact_approval(monkeypatch
assert "update_document" in seen_tools
+def test_tainted_document_edit_without_active_target_cannot_be_approved(monkeypatch):
+ from src.prompt_security import untrusted_context_message
+
+ import src.agent_loop as agent_loop
+
+ monkeypatch.setattr(
+ agent_loop,
+ "get_setting",
+ lambda key, default=None: default,
+ raising=False,
+ )
+ monkeypatch.setattr(agent_loop, "get_mcp_manager", lambda: None, raising=False)
+ monkeypatch.setattr(agent_loop, "estimate_tokens", lambda *args, **kwargs: 10)
+
+ async def fake_stream(*args, **kwargs):
+ yield "data: " + json.dumps({
+ "delta": "```update_document\nreplacement\n```",
+ }) + "\n\n"
+ yield "data: [DONE]\n\n"
+
+ async def should_not_execute(*args, **kwargs):
+ raise AssertionError("unsealed document edit reached executor")
+
+ monkeypatch.setattr(agent_loop, "stream_llm_with_fallback", fake_stream)
+ monkeypatch.setattr(agent_loop, "execute_tool_block", should_not_execute)
+ events = _collect_agent_events(
+ agent_loop.stream_agent_loop(
+ "http://local.test/v1",
+ "small-local-model",
+ [
+ {"role": "user", "content": "update a document"},
+ untrusted_context_message("stored context", "untrusted"),
+ ],
+ max_rounds=1,
+ relevant_tools={"update_document"},
+ )
+ )
+
+ blocked = [
+ event
+ for event in events
+ if event.get("type") == "tool_output"
+ and event.get("tool") == "update_document"
+ ]
+ assert blocked
+ assert "Open the exact document" in blocked[0]["output"]
+ assert "ask_user" not in blocked[0]
+
+
+def test_approval_pause_does_not_trigger_teacher_takeover(monkeypatch):
+ from src.prompt_security import untrusted_context_message
+
+ import src.agent_loop as agent_loop
+ import src.teacher_escalation as teacher_escalation
+
+ monkeypatch.setattr(
+ agent_loop,
+ "get_setting",
+ lambda key, default=None: default,
+ raising=False,
+ )
+ monkeypatch.setattr(agent_loop, "get_mcp_manager", lambda: None, raising=False)
+ monkeypatch.setattr(agent_loop, "estimate_tokens", lambda *args, **kwargs: 10)
+
+ async def fake_stream(*args, **kwargs):
+ yield "data: " + json.dumps({"delta": "```bash\nprintf paused\n```"}) + "\n\n"
+ yield "data: [DONE]\n\n"
+
+ async def fail_teacher(*args, **kwargs):
+ raise AssertionError("approval pause reached teacher takeover")
+ yield # pragma: no cover
+
+ monkeypatch.setattr(agent_loop, "stream_llm_with_fallback", fake_stream)
+ monkeypatch.setattr(teacher_escalation, "run_teacher_inline", fail_teacher)
+ events = _collect_agent_events(
+ agent_loop.stream_agent_loop(
+ "http://local.test/v1",
+ "small-local-model",
+ [
+ {"role": "user", "content": "run it"},
+ untrusted_context_message("stored context", "untrusted"),
+ ],
+ session_id="session-1",
+ max_rounds=1,
+ relevant_tools={"bash"},
+ )
+ )
+
+ assert any(
+ event.get("ask_user", {}).get("kind") == "tool_approval"
+ for event in events
+ )
+
+
def test_frontend_tool_approval_uses_opaque_id_and_fixed_decisions():
root = Path(__file__).parents[1]
chat = (root / "static/js/chat.js").read_text()
@@ -910,13 +1061,45 @@ def test_frontend_tool_approval_uses_opaque_id_and_fixed_decisions():
assert "if (isStreaming || _sendInFlight)" in chat
assert "_submitToolApprovalWhenIdle" in chat
assert "input.dispatchEvent(new Event('input'" in chat
+ assert "_pendingToolApproval.draft = input.value" in chat
+ assert "const approvalForSend = _pendingToolApproval" in chat
+ assert "!approvalForSend && fileHandlerModule.getPendingCount()" in chat
+ assert "if (!approvalForSend) _pendingRegenAttachments = null" in chat
+ assert "!approvalForSend && el('research-toggle').checked" in chat
+ assert "approvalForSend ? (approvalForSend.draft || '') : ''" in chat
+ assert "if (approvalForSend && documentSaved === false)" in chat
+ assert "if (!approvalForSend) {\n try {\n _sendPerf.mark('doc_silent_save_begin')" in chat
+ assert "document_id: aq.action && aq.action.document_id" in renderer
assert "const firstRound = (toolsByRound[0] || []).length ? 0 : 1" in renderer
assert "const r = ev.round ?? 1" in renderer
assert "/test-approval`" in skills
assert "approval_id: approval.approval_id" in skills
assert "['approve', 'Allow once'" in skills
- assert index.count("app.js?v=20260815toolapproval3") == 2
+ assert index.count("app.js?v=20260815toolapproval4") == 2
assert "app.js?v=20260808startupshell1" not in index
+ approval_module_sources = [
+ (root / path).read_text()
+ for path in (
+ "static/app.js",
+ "static/index.html",
+ "static/js/chat.js",
+ "static/js/chatRenderer.js",
+ "static/js/chatStream.js",
+ "static/js/document.js",
+ "static/js/emailInbox.js",
+ "static/js/emailLibrary.js",
+ "static/js/settings.js",
+ "static/js/slashCommands.js",
+ )
+ ]
+ assert all(
+ "20260722emailfastindex1" not in source
+ for source in approval_module_sources
+ )
+ assert all(
+ "20260815approvalsave1" in source
+ for source in approval_module_sources
+ )
def test_frontend_raw_fences_do_not_call_document_mutators():
diff --git a/tests/test_foreground_model_routing.py b/tests/test_foreground_model_routing.py
index ccebdebb0..481d3d32e 100644
--- a/tests/test_foreground_model_routing.py
+++ b/tests/test_foreground_model_routing.py
@@ -93,6 +93,7 @@ def _chat_stream_endpoint(
agent_chunks=None,
chat_chunks=None,
capture_completion=False,
+ capture_context=False,
endpoint_url="https://selected.example/v1",
):
def add_message(message):
@@ -136,6 +137,8 @@ def _chat_stream_endpoint(
)
async def fake_build_context(*args, **kwargs):
+ if capture_context:
+ captured["build_context"] = kwargs
return context
async def fake_chat_stream(candidates, messages, **kwargs):
@@ -336,6 +339,48 @@ async def test_chat_stream_approval_restores_exact_shell_turn_toggle(monkeypatch
assert "bash" not in captured["approval_disabled_tools"]
+@pytest.mark.asyncio
+async def test_chat_stream_approval_ignores_research_and_new_attachments(monkeypatch):
+ from src.tool_capabilities import capabilities_for_action
+
+ captured = {}
+ endpoint = _chat_stream_endpoint(
+ monkeypatch,
+ "agent",
+ captured,
+ capture_context=True,
+ )
+ monkeypatch.setattr(chat_routes, "get_session_mode", lambda _session_id: "research_pending")
+ pending = chat_routes.tool_approval_store.create(
+ owner="alice",
+ session_id="session-1",
+ origin_run_id="run-1",
+ tool_name="bash",
+ content="printf exact",
+ workspace=None,
+ external_untrusted_context_seen=True,
+ capabilities=capabilities_for_action("bash", "printf exact"),
+ )
+ request = _RouteRequest("agent")
+ request._form.update(
+ {
+ "attachments": '["unrelated-upload"]',
+ "use_research": "true",
+ "tool_approval_id": pending.approval_id,
+ "tool_approval_decision": "approve",
+ }
+ )
+
+ response = await endpoint(request)
+ async for _ in response.body_iterator:
+ pass
+
+ assert captured["exact_approval"].pending == pending
+ assert captured["build_context"]["att_ids"] == []
+ assert "agent" in captured
+ assert "chat" not in captured
+
+
@pytest.mark.asyncio
@pytest.mark.parametrize("mode", ["chat", "agent"])
@pytest.mark.parametrize("endpoint_url", ["", None])
diff --git a/tests/test_scheduler_prompt_cache_time.py b/tests/test_scheduler_prompt_cache_time.py
index 5dccf555d..bf3378bc2 100644
--- a/tests/test_scheduler_prompt_cache_time.py
+++ b/tests/test_scheduler_prompt_cache_time.py
@@ -12,6 +12,7 @@ Three focused tests:
from __future__ import annotations
+import json
from datetime import datetime, timezone
from types import SimpleNamespace
@@ -68,6 +69,51 @@ async def test_scheduler_agent_loop_path(monkeypatch):
assert msgs[2]["content"] == "run the digest"
+async def test_scheduler_retires_unattended_exact_approval(monkeypatch):
+ from src.task_scheduler import TaskScheduler
+ from src.tool_approvals import tool_approval_store
+ from src.tool_capabilities import capabilities_for_action
+
+ pending = tool_approval_store.create(
+ owner="admin",
+ session_id="s",
+ origin_run_id="scheduled-run",
+ tool_name="bash",
+ content="printf exact",
+ workspace=None,
+ external_untrusted_context_seen=True,
+ capabilities=capabilities_for_action("bash", "printf exact"),
+ )
+ approval = pending.public_payload()
+
+ async def fake_stream_agent_loop(*args, **kwargs):
+ yield "data: " + json.dumps({
+ "type": "tool_output",
+ "tool": "bash",
+ "output": "Waiting for an exact user approval.",
+ "ask_user": approval,
+ }) + "\n\n"
+
+ monkeypatch.setattr(
+ "src.agent_loop.stream_agent_loop",
+ fake_stream_agent_loop,
+ )
+ monkeypatch.setattr(
+ "src.task_endpoint.resolve_task_candidates",
+ lambda **kwargs: [],
+ )
+ result = await TaskScheduler(session_manager=None)._run_agent_loop(
+ "http://ep/v1",
+ "model",
+ _make_task(),
+ "s",
+ )
+
+ assert "paused safely" in result
+ assert "That action was not executed" in result
+ assert tool_approval_store.peek(pending.approval_id) is None
+
+
# ---------------------------------------------------------------------------
# Test 2 — fallback path receives the same datetime context
# ---------------------------------------------------------------------------
diff --git a/tests/test_startup_session_bootstrap_js.py b/tests/test_startup_session_bootstrap_js.py
index f3d47fb20..903dc91bd 100644
--- a/tests/test_startup_session_bootstrap_js.py
+++ b/tests/test_startup_session_bootstrap_js.py
@@ -23,7 +23,7 @@ _IMPORT_REWRITES = {
"import uiModule, { autoResize, styledPrompt } from './ui.js';": (
"import uiModule, { autoResize, styledPrompt } from './ui.mjs';"
),
- "import chatRenderer from './chatRenderer.js?v=20260815toolapproval3';": (
+ "import chatRenderer from './chatRenderer.js?v=20260815toolapproval4';": (
"import chatRenderer from './chatRenderer.mjs';"
),
"import { providerLogo } from './providers.js';": (
diff --git a/tests/test_teacher_eval_tier2.py b/tests/test_teacher_eval_tier2.py
index c3eb2a643..ee62c32f0 100644
--- a/tests/test_teacher_eval_tier2.py
+++ b/tests/test_teacher_eval_tier2.py
@@ -1,4 +1,5 @@
import asyncio
+import json
from types import SimpleNamespace
import pytest
@@ -217,6 +218,87 @@ async def test_run_teacher_inline_triggers_tier2_escalation(monkeypatch):
assert any("skill_saved" in evt for evt in events)
+@pytest.mark.asyncio
+async def test_teacher_approval_keeps_parent_authority_and_skips_skill_save(
+ monkeypatch,
+):
+ monkeypatch.setattr(
+ "src.settings.get_setting",
+ lambda key, default=None: {
+ "teacher_enabled": True,
+ "teacher_model": "teacher-model",
+ }.get(key, default),
+ )
+ monkeypatch.setattr(
+ "src.ai_interaction._resolve_model",
+ lambda spec, owner=None: (
+ "http://teacher.local/v1",
+ "teacher-model",
+ {},
+ ),
+ )
+ monkeypatch.setattr(
+ "src.teacher_escalation.evaluate_turn_regex",
+ lambda *args: ("failure", "student failed"),
+ )
+ captured = {}
+ approval = {
+ "kind": "tool_approval",
+ "approval_id": "opaque-id",
+ "question": "Allow this exact action once?",
+ }
+
+ async def fake_stream_agent_loop(*args, **kwargs):
+ captured.update(kwargs)
+ yield "data: " + json.dumps({
+ "type": "tool_output",
+ "tool": "bash",
+ "output": "Waiting for an exact user approval.",
+ "ask_user": approval,
+ }) + "\n\n"
+ yield "data: [DONE]\n\n"
+
+ async def fail_skill_distillation(*args, **kwargs):
+ raise AssertionError("paused teacher trace was distilled into a skill")
+
+ monkeypatch.setattr(
+ "src.agent_loop.stream_agent_loop",
+ fake_stream_agent_loop,
+ )
+ monkeypatch.setattr(
+ "src.teacher_escalation._call_teacher",
+ fail_skill_distillation,
+ )
+ active_document = object()
+ active_email = {"uid": "email-1"}
+ policy = object()
+
+ events = []
+ async for evt in teacher_escalation.run_teacher_inline(
+ student_endpoint_url="http://student.local/v1",
+ student_messages=[{"role": "user", "content": "test request"}],
+ student_tool_events=[],
+ student_reply="student reply",
+ owner="alice",
+ session_id="session-1",
+ workspace="/workspace",
+ disabled_tools={"web_fetch"},
+ tool_policy=policy,
+ active_document=active_document,
+ active_email=active_email,
+ ):
+ events.append(evt)
+
+ assert captured["session_id"] == "session-1"
+ assert captured["workspace"] == "/workspace"
+ assert captured["disabled_tools"] == {"web_fetch"}
+ assert captured["tool_policy"] is policy
+ assert captured["active_document"] is active_document
+ assert captured["active_email"] == active_email
+ assert any("opaque-id" in event for event in events)
+ assert not any("skill_saved" in event for event in events)
+
+
@pytest.mark.asyncio
async def test_run_teacher_inline_tier2_disabled_by_default(monkeypatch):
# Settings and gates (Tier 2 disabled)
diff --git a/tests/test_tool_approvals.py b/tests/test_tool_approvals.py
index 0748cd832..cce3f1ac4 100644
--- a/tests/test_tool_approvals.py
+++ b/tests/test_tool_approvals.py
@@ -61,10 +61,9 @@ def test_approval_is_bound_to_exact_action_and_claimed_once():
)
-def test_wrong_owner_and_deny_destructively_consume_pending_action():
+def test_wrong_owner_cannot_consume_but_deny_retires_pending_action():
store = ToolApprovalStore()
wrong_owner = _pending(store)
- denied = _pending(store)
assert store.consume(
wrong_owner.approval_id,
@@ -72,7 +71,9 @@ def test_wrong_owner_and_deny_destructively_consume_pending_action():
owner="mallory",
session_id="session-1",
) is None
- assert store.peek(wrong_owner.approval_id) is None
+ assert store.peek(wrong_owner.approval_id) == wrong_owner
+
+ denied = _pending(store)
assert store.consume(
denied.approval_id,
decision="deny",
@@ -222,6 +223,48 @@ async def test_dispatcher_uses_sealed_document_target(monkeypatch):
assert captured == [("document-7", 4)]
+@pytest.mark.asyncio
+async def test_dispatcher_rejects_approved_document_action_without_target(monkeypatch):
+ import src.tool_execution as tool_execution
+
+ store = ToolApprovalStore()
+ content = "replacement"
+ pending = _pending(
+ store,
+ tool_name="update_document",
+ content=content,
+ capabilities=capabilities_for_action("update_document", content),
+ )
+ grant = store.consume(
+ pending.approval_id,
+ decision="approve",
+ owner="alice",
+ session_id="session-1",
+ )
+
+ async def should_not_run(*args, **kwargs):
+ raise AssertionError("unsealed document target reached implementation")
+
+ monkeypatch.setattr(
+ tool_execution,
+ "_execute_tool_block_impl",
+ should_not_run,
+ )
+ _, result = await tool_execution.execute_tool_block(
+ ToolBlock("update_document", content),
+ session_id="session-1",
+ owner="alice",
+ workspace=None,
+ security_context=ToolRunSecurityContext(
+ external_untrusted_context_seen=True
+ ),
+ exact_approval=grant,
+ )
+
+ assert result["blocked"] is True
+ assert result["policy"] == "exact_tool_approval"
+
+
def test_approved_document_version_guard_rejects_changed_target():
from src.agent_tools.document_tools import _approved_document_version_error
@@ -235,6 +278,48 @@ def test_approved_document_version_guard_rejects_changed_target():
doc,
{"expected_document_version": 5},
) is None
+ assert _approved_document_version_error(
+ None,
+ {"expected_document_version": 5},
+ )["document_changed"] is True
+
+
+@pytest.mark.asyncio
+async def test_missing_sealed_document_does_not_fall_back_to_another(monkeypatch):
+ import src.agent_tools.document_tools as document_tools
+
+ class FakeDb:
+ def close(self):
+ pass
+
+ def rollback(self):
+ pass
+
+ monkeypatch.setattr("src.database.SessionLocal", lambda: FakeDb())
+ monkeypatch.setattr(
+ document_tools,
+ "_get_owned_document",
+ lambda *args, **kwargs: None,
+ )
+
+ def fail_fallback(*args, **kwargs):
+ raise AssertionError("sealed target fell back to a different document")
+
+ monkeypatch.setattr(
+ document_tools,
+ "_most_recent_owned_document",
+ fail_fallback,
+ )
+ result = await document_tools.UpdateDocumentTool().execute(
+ "replacement",
+ {
+ "doc_id": "deleted-document",
+ "expected_document_version": 4,
+ "owner": "alice",
+ },
+ )
+
+ assert result["document_changed"] is True
@pytest.mark.asyncio