wip(13-08): checkpoint before operator session restart — Task 1 GREEN (28/28), Task 2 in progress
Executor stopped deliberately for a session restart (bypass-permissions relaunch). Executor's final report: 'cargo test assistant confirm-gate suite 28/28 green, individual nonce test passes; committing Task 1 next — first verify the tools.rs/grants.rs/backends diffs are formatting-only.' Task 1 (D-07/D-11 confirm gate, backend) is implemented and test-green but this checkpoint is verbatim-uncommitted-state, NOT the reviewed atomic Task 1 commit: continuation executor should verify diffs, then reset --soft or commit-on-top into proper feat(13-08) task commits. Task 2 (ToolConfirmModal.vue trusted chrome, Chat.vue + contextBroker.ts wiring) is partially built, tests written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
db11c625c8
commit
fc09d7a292
@@ -6,15 +6,17 @@
|
||||
//! Concurrency discipline inherited from `mesh/listener/assist.rs`'s own
|
||||
//! doc comment ("Spawned off the radio loop so it never blocks"): never
|
||||
//! hold a shared lock across a `.await` that can block for human-response
|
||||
//! time. `execute_tool` below holds no lock at all in this tracer — there
|
||||
//! is nothing yet to hold one across (13-08's confirm gate is what
|
||||
//! introduces that discipline requirement for real).
|
||||
//! time. The confirm-gate wait in `execute_tool` below is exactly such an
|
||||
//! await — it can suspend for minutes while a human decides — and it is
|
||||
//! reached holding no lock at all: the gate's own internal lock is scoped
|
||||
//! to map edits inside `confirm.rs`, and nothing here wraps the call in a
|
||||
//! guard. Keep it that way (AI-SPEC §4b.2).
|
||||
|
||||
use anyhow::Result;
|
||||
|
||||
use super::backends::{Backend, BackendTurn};
|
||||
use super::tools::{ChatMessage, Role, ToolCall, ToolResult};
|
||||
use super::tools::ToolDef;
|
||||
use super::tools::{ChatMessage, Role, ToolCall, ToolResult};
|
||||
use super::ToolExecCtx;
|
||||
|
||||
/// Hard stop — a looping model must never spin unbounded (D-05).
|
||||
@@ -63,7 +65,9 @@ pub async fn run_loop(
|
||||
}
|
||||
}
|
||||
}
|
||||
anyhow::bail!("assistant loop exceeded MAX_TURNS without a final answer — stopping, not looping forever")
|
||||
anyhow::bail!(
|
||||
"assistant loop exceeded MAX_TURNS without a final answer — stopping, not looping forever"
|
||||
)
|
||||
}
|
||||
|
||||
/// The single choke point every tool call passes through, regardless of
|
||||
@@ -72,9 +76,9 @@ pub async fn run_loop(
|
||||
/// category grants — re-checked here even though the system prompt already
|
||||
/// omits ungranted tools; never trust that as the only enforcement layer),
|
||||
/// schema validation (never coerce, never guess — AI-SPEC §4b.1), and D-07
|
||||
/// (every destructive tool suspends for confirmation — 13-08 fills that
|
||||
/// branch in; there are no reachable destructive tools yet, so it is
|
||||
/// unreachable today even though the registry now has some).
|
||||
/// (every destructive tool suspends on the confirm gate before execution —
|
||||
/// only a matching human "yes" releases it; a decline or timeout returns a
|
||||
/// declined error result and executes nothing).
|
||||
///
|
||||
/// `pub(crate)` (not private) so `assistant::tools`'s own test module can
|
||||
/// exercise this exact choke point directly for S-05/S-07 — the point of
|
||||
@@ -120,7 +124,9 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu
|
||||
// "not yet implemented" placeholder instead of being refused with the
|
||||
// real reason (13-05 Task 1's `<done>` criterion). This performs no
|
||||
// mutation itself — only a read-only id lookup for the app tools.
|
||||
if let Err(msg) = super::tools::validate_business_rules(&call.name, &args, ctx.handler.as_ref()).await {
|
||||
if let Err(msg) =
|
||||
super::tools::validate_business_rules(&call.name, &args, ctx.handler.as_ref()).await
|
||||
{
|
||||
return ToolResult {
|
||||
call_id: call.id.clone(),
|
||||
is_error: true,
|
||||
@@ -129,11 +135,23 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu
|
||||
}
|
||||
|
||||
if tool.destructive {
|
||||
return ToolResult {
|
||||
call_id: call.id.clone(),
|
||||
is_error: true,
|
||||
content: "destructive tool execution is not yet implemented".to_string(),
|
||||
};
|
||||
// D-07/D-11: the loop suspends here. The gate publishes a
|
||||
// NODE-AUTHORED description (never model text) that neode-ui's
|
||||
// trusted chrome fetches over the authenticated RPC session, and
|
||||
// approval binds to a node-minted nonce over the tool name and the
|
||||
// validated args — so what executes below is exactly what the
|
||||
// human read (S-02). This await is human-speed (up to
|
||||
// CONFIRM_TIMEOUT); no lock is held across it — see the module doc.
|
||||
match ctx.confirm.request(&call.id, tool, &args).await {
|
||||
super::confirm::Confirmed::Yes => {}
|
||||
super::confirm::Confirmed::No | super::confirm::Confirmed::TimedOut => {
|
||||
return ToolResult {
|
||||
call_id: call.id.clone(),
|
||||
is_error: true,
|
||||
content: "the user declined this action — nothing was changed".to_string(),
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// D-06: dispatch is a per-tool, hand-written decision recorded in
|
||||
@@ -156,10 +174,10 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::api::rpc::RpcHandler;
|
||||
use crate::assistant::backends::scripted::ScriptedBackend;
|
||||
use crate::assistant::tools::{registry, system_disk_status_tool};
|
||||
use crate::assistant::{CallerScope, PermissionCategory};
|
||||
use crate::api::rpc::RpcHandler;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -228,7 +246,23 @@ mod tests {
|
||||
};
|
||||
let result = execute_tool(&call, &ctx).await;
|
||||
assert!(!result.is_error, "tool call errored: {}", result.content);
|
||||
assert_eq!(result.content, direct.to_string());
|
||||
// Same handler, same shape — but the two calls sample live statvfs
|
||||
// figures at two different moments, and on a busy node (this test
|
||||
// box hosts a live one) free/used byte counters drift between the
|
||||
// samples. Compare the stable fields byte-for-byte instead of the
|
||||
// whole payload; identical `partition`/`total_bytes`/`encrypted`
|
||||
// still proves this is the SAME handler, not a parallel AI-only
|
||||
// code path.
|
||||
let direct_v: serde_json::Value = direct.clone();
|
||||
let result_v: serde_json::Value =
|
||||
serde_json::from_str(&result.content).expect("tool result is the handler's JSON");
|
||||
for field in ["partition", "total_bytes", "encrypted"] {
|
||||
assert_eq!(
|
||||
result_v.get(field),
|
||||
direct_v.get(field),
|
||||
"field {field} must come from the same handler"
|
||||
);
|
||||
}
|
||||
assert!(result.content.contains("total_bytes"));
|
||||
|
||||
// Exercise the whole loop: a ScriptedBackend that names the tool,
|
||||
@@ -258,7 +292,11 @@ mod tests {
|
||||
};
|
||||
let result = execute_tool(&call, &ctx).await;
|
||||
assert!(result.is_error);
|
||||
assert!(result.content.contains("no such tool"), "{}", result.content);
|
||||
assert!(
|
||||
result.content.contains("no such tool"),
|
||||
"{}",
|
||||
result.content
|
||||
);
|
||||
}
|
||||
|
||||
/// Phase-10 hard constraint: `assistant.*` must never be reachable
|
||||
|
||||
Reference in New Issue
Block a user