P3:审批路径统一走 resolve_planning_path,并修正瞬时失败的处理
planning_storage 里 M1C-1 新增的六处审批路径绕过了 `resolve_planning_path`, 直接用通用解析器。通用解析器只认 `is_symlink()` 且把结果一律映射成 PLAN_INVALID_PATH;规划解析器认的是 FILE_ATTRIBUTE_REPARSE_POINT 全量重解析 标记,并如实报 PLAN_UNTRUSTED_PATH。审批回执是 GDD 完成门的判据,它的路径 分类必须和 GDD/session 一致。改完后 resolve_local_project_path 在本模块只剩 resolve_planning_path 内部一处调用,成为单一入口。 新增用例用 junction 而不是 symlink_dir 建链接:后者要开发者模式/管理员权限, 普通开发机上建不起来,用例会静默跳过成永远通过的空壳。 delivery.rs 里 user_revision_pending 那条分支是死代码——has_waiting() 已经 把它计入等待,上一道门必然先返回。删掉分支,把语义与跨文件依赖用 debug_assert 钉在使用现场,避免 has_waiting() 日后改动时静默跨过用户修订决策边界。 同时修正上一提交留下的问题:投影恢复失败必须分三路而不是两路。此前把「瞬时」 和「归属不到 run」并成同一个 false,导致瞬时锁争用走了全局上抛,让 resume 这 个可反复调用的恢复入口整轮失败——而锁被占恰恰说明别处正在推进。现在瞬时争用 只跳过本轮投影恢复,其余恢复照常,下一轮重试。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -891,12 +891,19 @@ pub(in crate::agent) fn resume_game_creator_agent_background_tasks_unredacted_at
|
||||
validate_project_root(root)?;
|
||||
// 这是一条只覆盖策划根 Supervisor 的窄投影恢复,却挂在整轮 resume 的最前面。
|
||||
// 原来用 `?` 强传播:一次 Fast GDD 投影失败会掐掉全项目所有 Agent 的恢复——而它
|
||||
// 本身正是 receipt 投影失败后的重试入口,掐掉它等于连兜底一起废掉。改成把
|
||||
// fail-closed 精确收敛到受影响的那个 run,其余 Agent 照常恢复;实在无法归属时
|
||||
// 才退回原来的全局上抛。
|
||||
// 本身正是 receipt 投影失败后的重试入口,掐掉它等于连兜底一起废掉。
|
||||
//
|
||||
// 失败必须分三路,不能两路。此前把「瞬时」和「归属不了」并成同一个 false,
|
||||
// 结果瞬时锁争用走了全局上抛,让这条可反复调用的恢复入口整轮失败——而锁被占
|
||||
// 恰恰说明别处正在推进,是最不该失败的时候。
|
||||
if let Err(error) = reconcile_plan_gdd_approval_projections_at(root) {
|
||||
let error = format!("恢复 GDD approval 投影失败:{error}");
|
||||
if !contain_plan_gdd_approval_recovery_failure_at(root, &error)? {
|
||||
if static_delegate_parent_wake_error_is_transient(&error) {
|
||||
// 瞬时争用(最常见的是 `.agent/project.lock` 正被另一个写操作占用):
|
||||
// 跳过本轮投影恢复,其余恢复照常走,下一轮 resume 重试。与下面拿不到
|
||||
// runtime task 锁时直接跳过的处理是同一套语义,因此同样不落审计。
|
||||
} else if !contain_plan_gdd_approval_recovery_failure_at(root, &error)? {
|
||||
// 持久失败但归属不到具体 run:只能退回原来的全局上抛。
|
||||
return Err(error);
|
||||
}
|
||||
}
|
||||
@@ -2001,12 +2008,11 @@ mod plan_gdd_approval_wait_recovery_tests {
|
||||
|
||||
let held = acquire_project_write_lock(root, "test.hold-project-write-lock")
|
||||
.expect("hold the project write lock");
|
||||
let error = resume_game_creator_agent_background_tasks_at(root)
|
||||
.expect_err("锁争用属于瞬时错误,必须照旧上抛而不是就地收敛");
|
||||
assert!(
|
||||
error.contains("恢复 GDD approval 投影失败"),
|
||||
"unexpected error: {error}"
|
||||
);
|
||||
// 锁被占说明别处正在推进,这恰恰是 resume 最不该失败的时候:本轮跳过投影
|
||||
// 恢复即可,整轮 resume 必须照常成功。上抛会让 resume 这个可反复调用的恢复
|
||||
// 入口在一次普通锁争用下整体失败——真实调用方就是连着调它来断言幂等的。
|
||||
resume_game_creator_agent_background_tasks_at(root)
|
||||
.expect("瞬时锁争用只能跳过本轮投影恢复,不得让整轮 resume 失败");
|
||||
drop(held);
|
||||
|
||||
let contained = read_game_creator_agent_runtime_at(root, &runtime.agent_id)
|
||||
|
||||
+81
-12
@@ -2913,8 +2913,10 @@ pub(crate) fn validate_plan_gdd_index_against_gdds_and_approvals(
|
||||
}
|
||||
|
||||
fn approval_directory_is_present(root: &Path) -> Result<bool, PlanningStorageError> {
|
||||
let path = resolve_local_project_path(root, PLAN_GDD_APPROVAL_DIR)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let path = resolve_planning_path(root, PLAN_GDD_APPROVAL_DIR)?;
|
||||
// 下面这次 stat 仍然必要:`resolve_planning_path` 只保证解析那一刻整条链
|
||||
// 可信,而判定「目录存在」要读的是使用时刻的那一项,顺带还要排掉链接以外
|
||||
// 的另一种不可信形态——普通文件占位。
|
||||
match fs::symlink_metadata(path) {
|
||||
Ok(metadata) => {
|
||||
if planning_metadata_is_link_or_reparse(&metadata) || !metadata.is_dir() {
|
||||
@@ -3166,8 +3168,7 @@ pub(crate) fn read_plan_gdd_approvals(
|
||||
pub(crate) fn read_plan_gdd_approvals_locked(
|
||||
root: &Path,
|
||||
) -> Result<Vec<PlanGddApprovalV1>, PlanningStorageError> {
|
||||
let approvals_root = resolve_local_project_path(root, PLAN_GDD_APPROVAL_DIR)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let approvals_root = resolve_planning_path(root, PLAN_GDD_APPROVAL_DIR)?;
|
||||
let metadata = match fs::symlink_metadata(&approvals_root) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()),
|
||||
@@ -3228,8 +3229,7 @@ pub(crate) fn read_plan_gdd_approval_for_version_locked(
|
||||
return Err(invalid("approval receipt version 越界"));
|
||||
}
|
||||
let relative = format!("{PLAN_GDD_APPROVAL_DIR}/v{version}.json");
|
||||
let path = resolve_local_project_path(root, &relative)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let path = resolve_planning_path(root, &relative)?;
|
||||
match fs::symlink_metadata(&path) {
|
||||
Ok(_) => {
|
||||
let bytes =
|
||||
@@ -4032,8 +4032,7 @@ pub(crate) fn read_plan_gdd_approval_pending(
|
||||
pub(crate) fn read_plan_gdd_approval_pending_locked(
|
||||
root: &Path,
|
||||
) -> Result<Option<PlanGddApprovalPendingV1>, PlanningStorageError> {
|
||||
let path = resolve_local_project_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let path = resolve_planning_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)?;
|
||||
match fs::symlink_metadata(&path) {
|
||||
Ok(_) => {
|
||||
let bytes = read_regular_planning_file(&path, "GDD approval pending")?;
|
||||
@@ -4061,8 +4060,7 @@ pub(crate) fn write_plan_gdd_approval_pending_atomic_locked(
|
||||
value: &PlanGddApprovalPendingV1,
|
||||
) -> Result<(), PlanningStorageError> {
|
||||
let bytes = canonical_plan_gdd_approval_pending_bytes(value)?;
|
||||
let target = resolve_local_project_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let target = resolve_planning_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)?;
|
||||
let parent = ensure_planning_parent(&target)?;
|
||||
if let Ok(_) = fs::symlink_metadata(&target) {
|
||||
verify_regular_planning_file(&target, "现有 GDD approval pending")?;
|
||||
@@ -4094,8 +4092,7 @@ pub(crate) fn write_plan_gdd_approval_pending_atomic_locked(
|
||||
pub(crate) fn remove_plan_gdd_approval_pending_locked(
|
||||
root: &Path,
|
||||
) -> Result<(), PlanningStorageError> {
|
||||
let path = resolve_local_project_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)
|
||||
.map_err(|error| PlanningStorageError::new("PLAN_INVALID_PATH", error))?;
|
||||
let path = resolve_planning_path(root, PLAN_GDD_APPROVAL_PENDING_PATH)?;
|
||||
match fs::symlink_metadata(&path) {
|
||||
Ok(_) => {
|
||||
verify_regular_planning_file(&path, "GDD approval pending")?;
|
||||
@@ -5331,4 +5328,76 @@ mod tests {
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
}
|
||||
|
||||
/// M1C-1 新增的审批路径最初绕过 `resolve_planning_path`,直接用通用解析器。
|
||||
/// 通用解析器只认 `is_symlink()`,把结果一律映射成 `PLAN_INVALID_PATH`;而
|
||||
/// 规划解析器认的是 `FILE_ATTRIBUTE_REPARSE_POINT` 全量重解析标记,并把被
|
||||
/// 篡改的路径如实报成 `PLAN_UNTRUSTED_PATH`。审批回执正是 GDD 完成门的判据,
|
||||
/// 它的路径分类必须和 GDD/session 一致,否则调用方按错误码分流时会把「路径
|
||||
/// 不可信」当成「路径写错了」。
|
||||
#[cfg(any(unix, windows))]
|
||||
#[test]
|
||||
fn approval_paths_classify_a_linked_planning_root_as_untrusted_not_merely_invalid() {
|
||||
let directory = tempfile::tempdir().expect("temp root");
|
||||
let root = directory.path();
|
||||
// 旁路目录留在项目内:真正要挡的是「planning 根被指向别处」,不是「逃出根」。
|
||||
let decoy = root.join("decoy-planning");
|
||||
fs::create_dir_all(decoy.join("approvals")).expect("decoy approvals");
|
||||
fs::create_dir_all(root.join(".agent")).expect("agent dir");
|
||||
let planning_link = {
|
||||
let mut path = root.to_path_buf();
|
||||
for part in PLAN_STORAGE_ROOT.split('/') {
|
||||
path.push(part);
|
||||
}
|
||||
path
|
||||
};
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink(&decoy, &planning_link).expect("planning symlink");
|
||||
// 用 junction 而不是 `symlink_dir`:后者要开发者模式/管理员权限,普通开发
|
||||
// 机上建不起来,用例会静默跳过成永远通过的空壳;junction 无需提权,而且它
|
||||
// 正是本仓各处点名要挡的那种 Windows 重解析点。
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use std::os::windows::process::CommandExt;
|
||||
let status = std::process::Command::new("cmd")
|
||||
.arg("/C")
|
||||
.raw_arg(format!(
|
||||
"mklink /J \"{}\" \"{}\"",
|
||||
planning_link.display(),
|
||||
decoy.display()
|
||||
))
|
||||
.status()
|
||||
.expect("spawn mklink");
|
||||
assert!(status.success(), "junction 建不起来则本用例失去判据");
|
||||
}
|
||||
|
||||
// 写入路径不在列:它的父链早已由 `ensure_planning_parent` 逐段校验,
|
||||
// 本来就会报 PLAN_UNTRUSTED_PATH,不构成这次改动的判据。
|
||||
assert_eq!(
|
||||
approval_directory_is_present(root).unwrap_err().code(),
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
assert_eq!(
|
||||
read_plan_gdd_approvals_locked(root).unwrap_err().code(),
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
assert_eq!(
|
||||
read_plan_gdd_approval_for_version_locked(root, 1)
|
||||
.unwrap_err()
|
||||
.code(),
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
assert_eq!(
|
||||
read_plan_gdd_approval_pending_locked(root)
|
||||
.unwrap_err()
|
||||
.code(),
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
assert_eq!(
|
||||
remove_plan_gdd_approval_pending_locked(root)
|
||||
.unwrap_err()
|
||||
.code(),
|
||||
"PLAN_UNTRUSTED_PATH"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -315,6 +315,14 @@ pub(in crate::agent) fn wake_waiting_static_delegate_parent_run_at(
|
||||
if barrier.has_waiting() {
|
||||
return Ok(false);
|
||||
}
|
||||
// 用户修订是一条显式的 Supervisor 决策边界:在它派出续作之前,父 run 绝不能
|
||||
// 被自动恢复。这条语义由 `has_waiting()` 承担(它把 userRevisionPending 计入
|
||||
// 等待),所以上面那道门已经覆盖。断言把这份跨文件依赖钉在使用现场——若哪天
|
||||
// `has_waiting()` 不再计入该计数,这里会立刻炸而不是静默跨过决策边界。
|
||||
debug_assert_eq!(
|
||||
barrier.user_revision_pending_count, 0,
|
||||
"userRevisionPending 必须已被 has_waiting() 拦下,否则父 run 会越过用户修订边界自动恢复"
|
||||
);
|
||||
let state = read_game_creator_agent_runtime_for_session_at(
|
||||
root,
|
||||
¤t_task.agent_id,
|
||||
@@ -337,11 +345,6 @@ pub(in crate::agent) fn wake_waiting_static_delegate_parent_run_at(
|
||||
ensure_static_delegate_user_input_wait_at(root, &mut state, &deliveries)?;
|
||||
return Ok(true);
|
||||
}
|
||||
if barrier.user_revision_pending_count > 0 {
|
||||
// A user revision is an explicit Supervisor decision boundary. Do
|
||||
// not auto-resume the parent before it dispatches the continuation.
|
||||
return Ok(false);
|
||||
}
|
||||
let state = advance_game_creator_agent_runtime_turn_at(
|
||||
root,
|
||||
state,
|
||||
|
||||
Reference in New Issue
Block a user