fix(approvals): an approval survives a broader posture change (E2)

apply_change_mode now returns whether the live posture actually changed,
and apply_pending_runtime_authority passes that through instead of
reporting every drained revision as a change. A republished identical
posture no longer invalidates planned or approved calls.

After an approval wait, the turn loop compares the newly applied posture
with the one the call was approved under (LiveRuntimeAuthority::narrows).
A call the user approved keeps running when the new posture is equal or
broader (Ask -> Auto-Review / Full Access, Plan -> Work); only a
narrowing (Work -> Plan, lost shell/trust/auto-approve, a stricter
approval posture or configured sandbox) sends it back to the model.
Later batches in the same step are checked against the posture they were
planned under, so a narrowing applied mid-batch also stops them.

A runtime PATCH that changes posture mid-turn publishes through the same
authority drain, so it no longer fails the call it lands on.

Checks: cargo test -p codewhale-tui --lib -- live_runtime_authority_narrows
posture_patch_during_approval_wait runtime_authority change_mode approval_
-> 166 passed, 4 failed. The 4 failures (tui::approval impact summary,
two tui::widgets approval-card renders, task_manager
pending_approval_suspends_idle_...) are in files other lanes have
uncommitted edits in and do not touch this path. The broader-posture
regression test fails with the old unconditional check (verified).

Refs Hmbown/codewhale-app#97

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
CodeWhale BotandClaude Opus 5.5 committed 2026-09-22 19:23:25 -07:00
1 parent 802002a51b
commit 59c2a1668c
3 files changed
+319 -23

No files matched your search

+66 -6
View File
@@ -1025,6 +1025,49 @@ impl LiveRuntimeAuthority {
}
}
/// Whether `self` grants less than `prior` along any axis: a stricter
/// approval posture, a lost shell/trust/auto-approve bit, a stricter
/// configured sandbox, or a mode switch that is not a step out of Plan.
///
/// A call the user approved under `prior` stays approved under a posture
/// that is equal or broader; only a narrowing sends it back for a retry.
fn narrows(&self, prior: &Self) -> bool {
fn posture_rank(mode: ApprovalMode) -> u8 {
match mode {
ApprovalMode::Never => 0,
ApprovalMode::Suggest => 1,
ApprovalMode::Auto => 2,
ApprovalMode::Bypass => 3,
}
}
fn sandbox_rank(mode: Option<&str>) -> Option<u8> {
match mode {
Some("read-only") => Some(0),
Some("workspace-write") => Some(1),
Some("external-sandbox") => Some(2),
None => Some(3),
// An unknown value cannot be ordered; treat any move to or
// from it as a narrowing.
Some(_) => None,
}
}
let mode_narrowed = self.mode != prior.mode && prior.mode != AppMode::Plan;
let sandbox_narrowed = self.configured_sandbox_mode != prior.configured_sandbox_mode
&& match (
sandbox_rank(self.configured_sandbox_mode.as_deref()),
sandbox_rank(prior.configured_sandbox_mode.as_deref()),
) {
(Some(now), Some(before)) => now < before,
_ => true,
};
mode_narrowed
|| sandbox_narrowed
|| posture_rank(self.approval_mode) < posture_rank(prior.approval_mode)
|| (prior.allow_shell && !self.allow_shell)
|| (prior.trust_mode && !self.trust_mode)
|| (prior.auto_approve && !self.auto_approve)
}
fn permission_snapshot(&self) -> RuntimePermissionAuthority {
RuntimePermissionAuthority {
auto_approve: self.auto_approve,
@@ -2095,7 +2138,7 @@ impl Engine {
auto_approve: bool,
approval_mode: ApprovalMode,
configured_sandbox_mode: Option<String>,
) {
) -> bool {
let authority = TurnAuthority::from_effective_fields(
mode,
allow_shell,
@@ -2114,7 +2157,7 @@ impl Engine {
self.api_config.sandbox_mode = configured_sandbox_mode;
self.apply_runtime_mode_policy(&authority);
if !changed {
return;
return false;
}
self.emit_session_updated().await;
let _ = self
@@ -2131,6 +2174,7 @@ impl Engine {
mode.label(),
)))
.await;
true
}
fn take_pending_runtime_authority(&self) -> Option<LiveRuntimeAuthority> {
@@ -2153,7 +2197,7 @@ impl Engine {
.clone()
}
async fn apply_runtime_authority(&mut self, authority: LiveRuntimeAuthority) {
async fn apply_runtime_authority(&mut self, authority: LiveRuntimeAuthority) -> bool {
self.apply_change_mode(
authority.mode,
authority.allow_shell,
@@ -2162,15 +2206,31 @@ impl Engine {
authority.approval_mode,
authority.configured_sandbox_mode,
)
.await;
.await
}
/// Apply the newest published authority, if any. Returns whether the
/// live posture actually changed: a republished identical posture (a
/// PATCH that only renamed the thread, a repeated mode pick) is not a
/// change and must not invalidate planned or approved calls.
async fn apply_pending_runtime_authority(&mut self) -> bool {
let Some(authority) = self.take_pending_runtime_authority() else {
return false;
};
self.apply_runtime_authority(authority).await;
true
self.apply_runtime_authority(authority).await
}
/// The posture this engine is enforcing right now, read from the live
/// session rather than the shared (possibly newer, unapplied) snapshot.
fn applied_runtime_authority(&self) -> LiveRuntimeAuthority {
LiveRuntimeAuthority {
mode: self.current_mode,
allow_shell: self.session.allow_shell,
trust_mode: self.session.trust_mode,
auto_approve: self.session.auto_approve,
approval_mode: self.session.approval_mode,
configured_sandbox_mode: self.api_config.sandbox_mode.clone(),
}
}
fn record_applied_runtime_authority(&self, authority: &TurnAuthority) {
+213
View File
@@ -12892,6 +12892,219 @@ async fn operate_model_shell_uses_normal_approval_and_workspace_sandbox() {
assert_eq!(written.trim_end(), "operate-approved");
}
/// Drives one model turn whose single `Bash` call needs approval, publishes
/// `change_to` (as a runtime PATCH does) while the approval is pending, then
/// approves. Returns the call's result and whether the file was written.
async fn posture_change_during_approval_wait(
change_to: (AppMode, ApprovalMode, bool),
) -> (Result<crate::tools::spec::ToolResult, ToolError>, bool) {
use wiremock::matchers::{body_string_contains, method, path};
use wiremock::{Mock, MockServer, ResponseTemplate};
let workspace = tempdir().expect("tempdir");
let server = MockServer::start().await;
let tool_call_sse = concat!(
"data: {\"id\":\"chatcmpl-e2\",\"choices\":[{\"index\":0,\"delta\":{\"tool_calls\":[",
"{\"index\":0,\"id\":\"call_e2_shell\",\"type\":\"function\",\"function\":{\"name\":\"Bash\",",
"\"arguments\":\"{\\\"action\\\":\\\"run\\\",\\\"command\\\":\\\"echo approved > e2-approved.txt\\\"}\"}}",
"]},\"finish_reason\":null}]}\n\n",
"data: {\"id\":\"chatcmpl-e2\",\"choices\":[{\"index\":0,\"delta\":{},",
"\"finish_reason\":\"tool_calls\"}]}\n\n",
"data: [DONE]\n\n",
);
let done_sse = concat!(
"data: {\"id\":\"chatcmpl-e2-done\",\"choices\":[{\"index\":0,",
"\"delta\":{\"content\":\"done\"},\"finish_reason\":null}]}\n\n",
"data: {\"id\":\"chatcmpl-e2-done\",\"choices\":[{\"index\":0,\"delta\":{},",
"\"finish_reason\":\"stop\"}]}\n\n",
"data: [DONE]\n\n",
);
Mock::given(method("POST"))
.and(path("/v1/chat/completions"))
.and(body_string_contains("call_e2_shell"))
.respond_with(
ResponseTemplate::new(200)
.insert_header("content-type", "text/event-stream")
.set_body_string(done_sse),
)
.with_priority(1)
.mount(&server)
.await;
Mock::given(method("POST"))
.and(path("/v1/chat/completions"))
.respond_with(
ResponseTemplate::new(200)
.insert_header("content-type", "text/event-stream")
.set_body_string(tool_call_sse),
)
.expect(1)
.with_priority(2)
.mount(&server)
.await;
let api_config = Config {
api_key: Some("test-key".to_string()),
base_url: Some(server.uri()),
..Config::default()
};
let (engine, handle) = Engine::new(
EngineConfig {
model: crate::config::DEFAULT_TEXT_MODEL.to_string(),
workspace: workspace.path().to_path_buf(),
snapshots_enabled: false,
subagents_enabled: false,
terminal_chrome_enabled: false,
..EngineConfig::default()
},
&api_config,
);
let run_task = tokio::spawn(engine.run());
handle
.send(Op::SendMessage(TurnSpec {
max_output_tokens: None,
content: "Record the approval fixture in the workspace".to_string(),
images: Vec::new(),
mode: AppMode::Agent,
route: resolved_route_for_test(&api_config, crate::config::DEFAULT_TEXT_MODEL),
compaction: Box::new(CompactionConfig::default()),
initial_routed_usage: Box::default(),
goal_objective: None,
goal_token_budget: None,
goal_status: crate::tools::goal::GoalStatus::Active,
reasoning_effort: None,
reasoning_effort_auto: false,
auto_model: false,
allow_shell: true,
trust_mode: false,
auto_approve: false,
approval_mode: ApprovalMode::Suggest,
translation_enabled: false,
allowed_tools: None,
dynamic_tools: Vec::new(),
hook_executor: None,
verbosity: None,
provenance: UserInputProvenance::ExternalUser,
}))
.await
.expect("send model turn");
let (mode, approval_mode, auto_approve) = change_to;
let mut shell_result = None;
let mut rx = handle.rx_event.write().await;
while let Some(event) = tokio::time::timeout(model_turn_event_timeout(), rx.recv())
.await
.expect("timed out waiting for turn event")
{
match event {
Event::ApprovalRequired { id, .. } => {
// The PATCH lands while the approval card is open.
handle
.try_send(Op::ChangeMode {
mode,
allow_shell: true,
trust_mode: false,
auto_approve,
approval_mode,
configured_sandbox_mode: None,
})
.expect("publish posture change");
handle.approve_tool_call(id).await.expect("approve shell");
}
Event::ToolCallComplete { name, result, .. } if name == "Bash" => {
shell_result = Some(result);
}
Event::TurnComplete { .. } => break,
_ => {}
}
}
drop(rx);
handle.send(Op::Shutdown).await.expect("shutdown engine");
run_task.await.expect("engine task");
let written = workspace.path().join("e2-approved.txt").exists();
(shell_result.expect("the approved call completes"), written)
}
#[test]
fn live_runtime_authority_narrows_only_when_a_grant_is_withdrawn() {
let at = |mode, approval_mode, sandbox: Option<&str>| {
LiveRuntimeAuthority::from_fields(
mode,
true,
false,
approval_mode == ApprovalMode::Bypass,
approval_mode,
sandbox.map(str::to_string),
)
};
let ask = at(AppMode::Agent, ApprovalMode::Suggest, None);
assert!(!ask.narrows(&ask));
assert!(!at(AppMode::Agent, ApprovalMode::Auto, None).narrows(&ask));
assert!(!at(AppMode::Agent, ApprovalMode::Bypass, None).narrows(&ask));
assert!(!ask.narrows(&at(AppMode::Plan, ApprovalMode::Suggest, None)));
assert!(at(AppMode::Plan, ApprovalMode::Suggest, None).narrows(&ask));
assert!(at(AppMode::Operate, ApprovalMode::Suggest, None).narrows(&ask));
assert!(ask.narrows(&at(AppMode::Agent, ApprovalMode::Bypass, None)));
assert!(at(AppMode::Agent, ApprovalMode::Never, None).narrows(&ask));
assert!(at(AppMode::Agent, ApprovalMode::Suggest, Some("read-only")).narrows(&ask));
assert!(
!at(
AppMode::Agent,
ApprovalMode::Suggest,
Some("workspace-write")
)
.narrows(&at(
AppMode::Agent,
ApprovalMode::Suggest,
Some("read-only")
))
);
assert!(at(AppMode::Agent, ApprovalMode::Suggest, Some("custom")).narrows(&ask));
let mut no_shell = ask.clone();
no_shell.allow_shell = false;
assert!(no_shell.narrows(&ask));
}
/// E2: approving a call must never invalidate the call it approves. A posture
/// PATCH that is equal or broader (Ask -> Auto-Review, Ask -> Full Access)
/// while the approval card is open leaves the approved call running.
#[tokio::test]
#[allow(clippy::await_holding_lock)]
async fn broader_posture_patch_during_approval_wait_keeps_the_approved_call() {
let _lock = lock_test_env();
for change_to in [
(AppMode::Agent, ApprovalMode::Auto, false),
(AppMode::Agent, ApprovalMode::Bypass, true),
(AppMode::Agent, ApprovalMode::Suggest, false),
] {
let (result, written) = posture_change_during_approval_wait(change_to).await;
let result = result.unwrap_or_else(|err| panic!("{change_to:?}: {err}"));
assert!(result.success, "{change_to:?}: {result:?}");
assert!(written, "{change_to:?}: the approved shell ran");
}
}
/// E2 counterpart: a narrowing PATCH (Work -> Plan, Ask -> Never) still sends
/// the approved call back to the model instead of running it under a grant
/// the user has since withdrawn.
#[tokio::test]
#[allow(clippy::await_holding_lock)]
async fn narrower_posture_patch_during_approval_wait_fails_the_call() {
let _lock = lock_test_env();
for change_to in [
(AppMode::Plan, ApprovalMode::Suggest, false),
(AppMode::Agent, ApprovalMode::Never, false),
] {
let (result, written) = posture_change_during_approval_wait(change_to).await;
let err = result.expect_err("narrowed posture fails the call");
assert!(
err.to_string()
.contains("posture changed before this tool call executed"),
"{change_to:?}: {err}"
);
assert!(!written, "{change_to:?}: the shell must not run");
}
}
#[tokio::test]
#[allow(clippy::await_holding_lock)]
async fn full_access_subagent_handoff_keeps_model_shell_free_of_approval_prompts() {
+40 -17
View File
@@ -3641,6 +3641,10 @@ impl Engine {
questions_allowed: &mut bool,
) -> (Vec<Option<ToolExecOutcome>>, bool) {
let mut authority_changed = false;
// Every plan below was classified under this posture. A narrowing
// applied mid-batch (for example while an earlier call waited on its
// approval) must still stop later plans that assumed the old grant.
let planned_posture = self.applied_runtime_authority();
let collect_fleet_evidence =
tool_registry.is_some_and(|registry| registry.context().tool_authority.is_some());
// --- Intent summary for write tools (#2381) ---
@@ -3713,7 +3717,8 @@ impl Engine {
// changed after this batch was planned, never execute it with
// stale approval or sandbox facts. Return one typed retry to
// the model; the next call is planned under the new posture.
if self.apply_pending_runtime_authority().await {
let changed_now = self.apply_pending_runtime_authority().await;
if changed_now || self.applied_runtime_authority().narrows(&planned_posture) {
authority_changed = true;
*mode = self.current_mode;
*questions_allowed = crate::core::authority::permission_posture_allows_questions(
@@ -4313,10 +4318,13 @@ impl Engine {
(None, None, None)
};
// An approval wait can outlive a posture switch. Do
// not start a tool from the stale plan; the
// model can retry immediately under the newly applied
// authority.
// An approval wait can outlive a posture switch. A
// call the user just approved stays approved when the
// new posture is equal or broader: approving must never
// invalidate the call it approves. Only a narrowing, or
// a change under a call nobody approved, sends it back
// to the model to retry under the new authority.
let posture_before_drain = self.applied_runtime_authority();
let mut result_override = if self.apply_pending_runtime_authority().await {
authority_changed = true;
*mode = self.current_mode;
@@ -4324,12 +4332,20 @@ impl Engine {
crate::core::authority::permission_posture_allows_questions(
self.session.approval_mode,
);
result_override.or_else(|| {
Some(Err(ToolError::permission_denied(
"Runtime permission posture changed before this tool call executed; retry it under the current posture."
.to_string(),
)))
})
let approval_survives = approval_stamp.is_some()
&& !self
.applied_runtime_authority()
.narrows(&posture_before_drain);
if approval_survives {
result_override
} else {
result_override.or_else(|| {
Some(Err(ToolError::permission_denied(
"Runtime permission posture changed before this tool call executed; retry it under the current posture."
.to_string(),
)))
})
}
} else {
result_override
};
@@ -4355,6 +4371,7 @@ impl Engine {
self.emit_pending_snapshot_notices().await;
}
let posture_before_drain = self.applied_runtime_authority();
if self.apply_pending_runtime_authority().await {
authority_changed = true;
*mode = self.current_mode;
@@ -4362,12 +4379,18 @@ impl Engine {
crate::core::authority::permission_posture_allows_questions(
self.session.approval_mode,
);
result_override.get_or_insert_with(|| {
Err(ToolError::permission_denied(
"Runtime permission posture changed before this tool call executed; retry it under the current posture."
.to_string(),
))
});
if approval_stamp.is_none()
|| self
.applied_runtime_authority()
.narrows(&posture_before_drain)
{
result_override.get_or_insert_with(|| {
Err(ToolError::permission_denied(
"Runtime permission posture changed before this tool call executed; retry it under the current posture."
.to_string(),
))
});
}
}
let started_at = Instant::now();