DirectProject 审批拒绝原因留痕
Project CI / AI game creator shell Rust smoke (pull_request) Successful in 1m43s
Project CI / Backend tests (pull_request) Failing after 17s
Project CI / AI game creator shell Rust crates (pull_request) Successful in 1m13s
Project CI / Frontend tests (pull_request) Successful in 2m12s
Project CI / Repository checks (pull_request) Failing after 13s
Project CI / AI game creator shell web tests (pull_request) Successful in 1m57s
Project CI / Native shell tests (pull_request) Successful in 5m54s
Project CI / AI game creator shell Rust lane 2/2 (pull_request) Successful in 9m41s
Project CI / AI game creator shell Rust lane 1/2 (pull_request) Successful in 10m49s
Project CI / AI game creator shell Rust smoke (pull_request) Successful in 1m43s
Project CI / Backend tests (pull_request) Failing after 17s
Project CI / AI game creator shell Rust crates (pull_request) Successful in 1m13s
Project CI / Frontend tests (pull_request) Successful in 2m12s
Project CI / Repository checks (pull_request) Failing after 13s
Project CI / AI game creator shell web tests (pull_request) Successful in 1m57s
Project CI / Native shell tests (pull_request) Successful in 5m54s
Project CI / AI game creator shell Rust lane 2/2 (pull_request) Successful in 9m41s
Project CI / AI game creator shell Rust lane 1/2 (pull_request) Successful in 10m49s
宿主拒绝 app-server 的审批 / 交互请求时,原因必须落 AppData `RUST` 日志。稳定键 `agent.direct_codex.approval.denied`,字段为 `thread_id` / `method` / `reason` / `distinct_reasons`;未绑定宿主执行器时 `reason=no-bound-host-execution-adapter`,回包写入失败另记 `agent.direct_codex.approval.response_write_failed`。
This commit is contained in:
@@ -15,6 +15,10 @@ use tokio::sync::{watch, Notify};
|
||||
|
||||
const MAX_PROTOCOL_ITEMS: usize = 2048;
|
||||
const MAX_REQUEST_CACHE: usize = 512;
|
||||
/// 拒绝原因留痕的条数上限(按 `(method, reason)` 去重后的上限)。
|
||||
///
|
||||
/// 原因基本是固定的静态分类,上限只是防止宿主错误文本意外发散时无界增长。
|
||||
const MAX_DENIED_REASON_LOGS: usize = 64;
|
||||
|
||||
/// 逐次审批协议的版本门禁:发行构建只接受捆绑侧车的固定版本;开发构建用宿主自带的 Codex
|
||||
/// (Linux 与未 stage 侧车时没有固定版本可用),按 profile 直接跳过该门禁。
|
||||
@@ -38,6 +42,11 @@ pub(super) fn validate_approval_version(version: &str) -> Result<(), String> {
|
||||
}
|
||||
|
||||
pub(super) fn denied_response(id: u64, method: &str) -> Value {
|
||||
// 没绑定宿主执行器时一律拒绝。这是「回合没接单 / 适配器已释放」的唯一表现,
|
||||
// 不留痕线上就只剩一个没有原因的 decline。
|
||||
app_log!(
|
||||
"agent.direct_codex.approval.denied reason=no-bound-host-execution-adapter request_id={id} method={method}"
|
||||
);
|
||||
denied(id, method)
|
||||
}
|
||||
|
||||
@@ -195,6 +204,8 @@ pub(super) struct ExecutionAdapter {
|
||||
turn_failure: Mutex<Option<DirectTurnError>>,
|
||||
/// 用户/宿主是否主动要求终止这一轮(界面的「终止」按钮)。用户主动终止不是失败。
|
||||
host_stop_requested: AtomicBool,
|
||||
/// 已留痕的拒绝原因(`method\u{1}reason`)。同一回合内同因只记一次,避免模型重试刷屏。
|
||||
denied_reasons_logged: Mutex<HashSet<String>>,
|
||||
}
|
||||
|
||||
fn identity(value: Option<&Value>) -> Option<&str> {
|
||||
@@ -313,9 +324,40 @@ impl ExecutionAdapter {
|
||||
outcome,
|
||||
turn_failure: Mutex::new(None),
|
||||
host_stop_requested: AtomicBool::new(false),
|
||||
denied_reasons_logged: Mutex::new(HashSet::new()),
|
||||
})
|
||||
}
|
||||
|
||||
/// 审批 / 交互被拒的原因留痕。
|
||||
///
|
||||
/// 宿主拒绝 app-server 的请求时,线上只回一个 `decline`,用户和 Codex 都看不到是哪一道闸门
|
||||
/// 拦下的;原因只在这里落 AppData 日志。`reason` 只接受本模块的静态分类或宿主自己的错误
|
||||
/// 文本,绝不带请求参数、上游正文、路径或凭据。
|
||||
///
|
||||
/// 同一回合内 `(method, reason)` 只记一次:模型被拒后常反复重试同一动作,逐次记录会把真正
|
||||
/// 有用的诊断刷掉;"这一轮被拒过哪些原因"仍然完整。
|
||||
fn log_denied_reason(&self, method: &str, reason: &str) {
|
||||
let key = format!("{method}\u{1}{reason}");
|
||||
// 拒绝原因留痕不得反过来影响放行判定:锁不可用就放弃记录。
|
||||
let Ok(mut logged) = self.denied_reasons_logged.lock() else {
|
||||
return;
|
||||
};
|
||||
if logged.len() >= MAX_DENIED_REASON_LOGS || !logged.insert(key) {
|
||||
return;
|
||||
}
|
||||
let distinct = logged.len();
|
||||
drop(logged);
|
||||
app_log!(
|
||||
"agent.direct_codex.approval.denied thread_id={} method={method} reason={reason} distinct_reasons={distinct}",
|
||||
self.thread_id
|
||||
);
|
||||
}
|
||||
|
||||
fn denied(&self, id: u64, method: &str, reason: &str) -> Value {
|
||||
self.log_denied_reason(method, reason);
|
||||
denied(id, method)
|
||||
}
|
||||
|
||||
pub(super) fn bind_turn(&self, turn_id: &str) -> bool {
|
||||
if turn_id.is_empty() || turn_id.len() > 256 {
|
||||
return false;
|
||||
@@ -454,38 +496,38 @@ impl ExecutionAdapter {
|
||||
|
||||
pub(super) async fn respond(self: &Arc<Self>, id: u64, method: &str, params: &Value) -> Value {
|
||||
if self.closed.load(Ordering::Acquire) || self.is_host_ending() {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "session-closed-or-host-ending");
|
||||
}
|
||||
let fingerprint = cohort_key(method, params);
|
||||
let target = {
|
||||
let Ok(mut state) = self.state.lock() else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "state-lock-poisoned");
|
||||
};
|
||||
if !self.matches_scope(&state, params) {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "request-out-of-turn-scope");
|
||||
}
|
||||
if let Some(ticket) = state.tickets.get(&id) {
|
||||
return if ticket.fingerprint == fingerprint {
|
||||
ticket
|
||||
.response
|
||||
.clone()
|
||||
.unwrap_or_else(|| denied(id, method))
|
||||
.unwrap_or_else(|| self.denied(id, method, "ticket-without-response"))
|
||||
} else {
|
||||
denied(id, method)
|
||||
self.denied(id, method, "request-id-fingerprint-mismatch")
|
||||
};
|
||||
}
|
||||
// Approval tasks run concurrently; numeric request IDs need not enter
|
||||
// this mutex in order. Retain seen IDs rather than rejecting by high-water.
|
||||
if state.seen_request_ids.len() >= 8192 || !state.seen_request_ids.insert(id) {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "request-id-replayed-or-capacity");
|
||||
}
|
||||
let target = match method {
|
||||
"item/commandExecution/requestApproval" | "item/fileChange/requestApproval" => {
|
||||
let Some(item_id) = identity(params.get("itemId")) else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "approval-without-item-id");
|
||||
};
|
||||
let Some(item) = state.items.get(item_id) else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "approval-for-unknown-item");
|
||||
};
|
||||
let expected = if method == "item/fileChange/requestApproval" {
|
||||
ItemKind::Patch
|
||||
@@ -493,10 +535,10 @@ impl ExecutionAdapter {
|
||||
ItemKind::Command
|
||||
};
|
||||
if item.kind != expected || item.terminal.is_some() || item.lease.is_some() {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "item-not-pending-for-approval");
|
||||
}
|
||||
if state.tickets.values().any(|ticket| matches!(&ticket.target, Target::Item(existing, _) if existing == item_id)) {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "item-approval-already-pending");
|
||||
}
|
||||
Target::Item(
|
||||
item_id.to_string(),
|
||||
@@ -514,20 +556,20 @@ impl ExecutionAdapter {
|
||||
!= Some("mcp_tool_call")
|
||||
|| params.get("mode").and_then(Value::as_str) != Some("form")
|
||||
{
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "unsupported-mcp-elicitation-shape");
|
||||
}
|
||||
let Some(server) = params.get("serverName").and_then(Value::as_str) else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "mcp-elicitation-without-server-name");
|
||||
};
|
||||
if server == "agc_tools" || !self.third_party_servers.contains(server) {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "mcp-server-not-eligible-for-elicitation");
|
||||
}
|
||||
let Some(arguments) = params.pointer("/_meta/tool_params") else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "mcp-elicitation-without-tool-params");
|
||||
};
|
||||
let key = cohort_key(server, arguments);
|
||||
let Some(cohort) = state.cohorts.get(&key) else {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "mcp-cohort-not-registered");
|
||||
};
|
||||
let pending_members = cohort
|
||||
.members
|
||||
@@ -541,12 +583,12 @@ impl ExecutionAdapter {
|
||||
.count();
|
||||
let seen_tickets = state.tickets.values().filter(|ticket| matches!(&ticket.target, Target::Cohort(existing) if existing == &key)).count();
|
||||
if pending_members == 0 || cohort.members.len() <= seen_tickets {
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "mcp-cohort-already-covered");
|
||||
}
|
||||
state.cohorts.get_mut(&key).unwrap().admissions += 1;
|
||||
Target::Cohort(key)
|
||||
}
|
||||
_ => return denied(id, method),
|
||||
_ => return self.denied(id, method, "unsupported-approval-method"),
|
||||
};
|
||||
if state.tickets.len() >= MAX_REQUEST_CACHE {
|
||||
state.tickets.retain(|_, ticket| ticket.response.is_none());
|
||||
@@ -592,13 +634,17 @@ impl ExecutionAdapter {
|
||||
})
|
||||
.await
|
||||
.unwrap_or_else(|_| Err("执行许可任务中断".into()));
|
||||
// 闸门拒绝(合同缺失 / 预算耗尽 / 阶段已收束 / 同一输入重复受理…)的真实原因只在
|
||||
// `admitted` 里;它随后会被 move 掉,先取副本,供最后统一留痕。
|
||||
let admit_failure = admitted.as_ref().err().cloned();
|
||||
let mut allowed = false;
|
||||
let mut rejected_lease = None;
|
||||
let mut denied_reason = None;
|
||||
let entries = {
|
||||
let Ok(mut state) = self.state.lock() else {
|
||||
self.settling.fetch_sub(1, Ordering::AcqRel);
|
||||
self.changed.notify_waiters();
|
||||
return denied(id, method);
|
||||
return self.denied(id, method, "state-lock-poisoned-after-admit");
|
||||
};
|
||||
if let Target::Cohort(key) = &target {
|
||||
if let Some(cohort) = state.cohorts.get_mut(key) {
|
||||
@@ -617,6 +663,7 @@ impl ExecutionAdapter {
|
||||
item.lease = Some(entry);
|
||||
allowed = true;
|
||||
} else {
|
||||
denied_reason = Some("item-left-pending-before-lease-attach");
|
||||
rejected_lease = Some(entry);
|
||||
}
|
||||
}
|
||||
@@ -625,11 +672,13 @@ impl ExecutionAdapter {
|
||||
cohort.leases.push(entry);
|
||||
allowed = true;
|
||||
} else {
|
||||
denied_reason = Some("cohort-gone-before-lease-attach");
|
||||
rejected_lease = Some(entry);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
denied_reason = Some("session-closed-before-lease-attach");
|
||||
rejected_lease = Some(entry);
|
||||
}
|
||||
}
|
||||
@@ -652,7 +701,13 @@ impl ExecutionAdapter {
|
||||
if allowed {
|
||||
accepted(id, method)
|
||||
} else {
|
||||
denied(id, method)
|
||||
// 每个请求只留一条:admit 失败优先用宿主的真实错误文本,其次是租约附着阶段的分类。
|
||||
let reason = match (admit_failure, denied_reason) {
|
||||
(Some(error), _) => format!("lease-admit-failed: {error}"),
|
||||
(None, Some(reason)) => reason.to_string(),
|
||||
(None, None) => "lease-not-granted".to_string(),
|
||||
};
|
||||
self.denied(id, method, &reason)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -695,6 +750,11 @@ impl ExecutionAdapter {
|
||||
}
|
||||
|
||||
pub(super) fn response_write_failed(self: &Arc<Self>) {
|
||||
// 回包写不出去时 Codex 侧等不到 decision,表现为「审批没有回应」;与主动 decline 分开留痕。
|
||||
app_log!(
|
||||
"agent.direct_codex.approval.response_write_failed thread_id={}",
|
||||
self.thread_id
|
||||
);
|
||||
let adapter = Arc::clone(self);
|
||||
tokio::spawn(async move {
|
||||
adapter
|
||||
@@ -1689,4 +1749,35 @@ mod tests {
|
||||
);
|
||||
assert!(denied(5, "item/tool/call").get("error").is_some());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn denied_approval_records_its_reason_once_per_turn() {
|
||||
let (_temp, adapter) = fixture();
|
||||
let method = "item/commandExecution/requestApproval";
|
||||
assert_eq!(
|
||||
adapter.respond(1, method, &approval("missing")).await["result"]["decision"],
|
||||
"decline"
|
||||
);
|
||||
// 线上只回 decline;原因必须留在适配器上,供 AppData 日志回查是哪一道闸门拦下的。
|
||||
let key = format!("{method}\u{1}approval-for-unknown-item");
|
||||
assert!(adapter.denied_reasons_logged.lock().unwrap().contains(&key));
|
||||
// 模型重试同一动作会换新的 request id,但同一回合同因只留一条。
|
||||
assert_eq!(
|
||||
adapter.respond(2, method, &approval("missing")).await["result"]["decision"],
|
||||
"decline"
|
||||
);
|
||||
assert_eq!(adapter.denied_reasons_logged.lock().unwrap().len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_reason_logging_is_bounded() {
|
||||
let (_temp, adapter) = fixture();
|
||||
for index in 0..(MAX_DENIED_REASON_LOGS + 8) {
|
||||
adapter.log_denied_reason("unsupported-approval-method", &format!("reason-{index}"));
|
||||
}
|
||||
assert_eq!(
|
||||
adapter.denied_reasons_logged.lock().unwrap().len(),
|
||||
MAX_DENIED_REASON_LOGS
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user