mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-11 01:51:34 -10:00
fix: stabilize provider preferences and approval coordination
Preserve exact regional provider identity and saved automatic model selection, round-trip configuration bundles without losing inactive provider preferences, and validate custom defaults against their own route. Make tool-registry lookup an optional fallback and safely upgrade untouched bundled guidance. Give approvals one-use runtime identifiers with tool-call correlation for snapshots and streams. Keep relay polling alive and record bounded provider completion diagnostics without logging prompt content or changing limits. Validation: joined approval/provider selection 350 passed, 0 failed; provider/export/registry selection 96 passed, 0 failed; earlier bounded diagnostics/poll/frame/CLI selection 14 passed, 0 failed. cargo fmt and git diff --check passed. Full workspace gates, hosted CI, final signed artifacts and customer workflows remain unverified on this candidate.
This commit is contained in:
1 parent
e1895bb114
commit
a7c0fb46e4
31 files changed
+1833
-288
No files matched your search
@@ -853,25 +853,10 @@ pub fn export_bundle(
|
||||
|
||||
let mut document = config_document(config)?;
|
||||
if scope == BundleScope::Global {
|
||||
let slot = codewhale_tui::route_preferences::model_slot_for_document(
|
||||
&toml::to_string(&document)?,
|
||||
"model",
|
||||
)?;
|
||||
let has_canonical_model = match slot.as_slice() {
|
||||
[root, provider, field] if root == "providers" => document
|
||||
.get(root)
|
||||
.and_then(|table| table.get(provider.as_str())?.get(field.as_str())?.as_str())
|
||||
.is_some_and(|model| !model.trim().is_empty()),
|
||||
// Literal Custom still owns its root model; it is not shadowed.
|
||||
_ => false,
|
||||
};
|
||||
if has_canonical_model {
|
||||
// These legacy aliases address the active route on import. A
|
||||
// shadowed root must not conflict with the canonical slot in our
|
||||
// own export; inactive providers keep their independent slots.
|
||||
document.remove("model");
|
||||
document.remove("default_text_model");
|
||||
}
|
||||
// Root model aliases are route-relative on import; reconcile them with
|
||||
// the canonical provider slots so the bundle never conflicts with
|
||||
// itself, without reparsing unrelated preserved extras as `Config`.
|
||||
codewhale_tui::route_preferences::scrub_root_model_aliases_for_export(&mut document)?;
|
||||
}
|
||||
for (key, value) in document {
|
||||
if let Some(value) = sanitize_export_value(&key, &value) {
|
||||
@@ -1023,6 +1008,7 @@ pub struct ImportReceipt {
|
||||
/// the prepared candidate is committed through one `ConfigStore::save`, and any failure
|
||||
/// restores the backup before returning the error. The receipt redacts by
|
||||
/// construction: it carries only key paths and counts, never values.
|
||||
#[cfg(test)]
|
||||
pub fn apply_bundle(
|
||||
bundle: &PortableBundle,
|
||||
store: &mut codewhale_config::ConfigStore,
|
||||
@@ -2276,6 +2262,123 @@ verbosity = "verbose"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_reconciles_a_legacy_root_default_model_with_the_deepseek_slot() {
|
||||
// Migration writes the canonical slot but never removes a legacy root
|
||||
// `default_model`; on import that alias also targets the DeepSeek slot,
|
||||
// so a raw export would conflict with itself.
|
||||
let config: ConfigToml = toml::from_str(
|
||||
"provider = 'deepseek'\ndefault_model = 'deepseek-v4-flash'\n[providers.deepseek]\nmodel = 'deepseek-v4-pro'\n",
|
||||
)
|
||||
.unwrap();
|
||||
let bundle =
|
||||
export_bundle(&config, BundleScope::Global, BundleMetadata::default()).unwrap();
|
||||
assert!(!bundle.global.entries.contains_key("default_model"));
|
||||
let providers = bundle
|
||||
.global
|
||||
.entries
|
||||
.get("providers")
|
||||
.and_then(toml::Value::as_table)
|
||||
.expect("providers table");
|
||||
assert_eq!(
|
||||
providers["deepseek"]["model"].as_str(),
|
||||
Some("deepseek-v4-pro")
|
||||
);
|
||||
|
||||
let dir = tempfile::tempdir().expect("config dir");
|
||||
let mut store = ConfigStore::load(Some(dir.path().join("config.toml"))).unwrap();
|
||||
let receipt = apply_bundle(&bundle, &mut store, BundleScope::Global, dir.path())
|
||||
.expect("export must import without alias conflicts");
|
||||
assert!(receipt.plan.conflicting.is_empty(), "{:?}", receipt.plan);
|
||||
assert_eq!(
|
||||
store
|
||||
.config
|
||||
.get_value("providers.deepseek.model")
|
||||
.as_deref(),
|
||||
Some("deepseek-v4-pro")
|
||||
);
|
||||
|
||||
// Without a canonical slot the root alias folds into the DeepSeek slot.
|
||||
let config: ConfigToml = toml::from_str(
|
||||
"provider = 'zai'\ndefault_model = 'deepseek-v4-flash'\n[providers.zai]\nmodel = 'GLM-5.3'\n",
|
||||
)
|
||||
.unwrap();
|
||||
let bundle =
|
||||
export_bundle(&config, BundleScope::Global, BundleMetadata::default()).unwrap();
|
||||
assert!(!bundle.global.entries.contains_key("default_model"));
|
||||
let providers = bundle
|
||||
.global
|
||||
.entries
|
||||
.get("providers")
|
||||
.and_then(toml::Value::as_table)
|
||||
.expect("providers table");
|
||||
assert_eq!(
|
||||
providers["deepseek"]["model"].as_str(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
assert_eq!(providers["zai"]["model"].as_str(), Some("GLM-5.3"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_preserves_the_deepseek_root_fallback_when_another_route_is_active() {
|
||||
// With Z.ai active, a DeepSeek-id root `default_text_model` is
|
||||
// DeepSeek's saved fallback, not shadowed state; the active route's
|
||||
// canonical slot must not cause it to be dropped.
|
||||
let config: ConfigToml = toml::from_str(
|
||||
"provider = 'zai'\ndefault_text_model = 'deepseek-v4-flash'\n[providers.zai]\nmodel = 'GLM-5.3'\n",
|
||||
)
|
||||
.unwrap();
|
||||
let bundle =
|
||||
export_bundle(&config, BundleScope::Global, BundleMetadata::default()).unwrap();
|
||||
assert!(!bundle.global.entries.contains_key("default_text_model"));
|
||||
let providers = bundle
|
||||
.global
|
||||
.entries
|
||||
.get("providers")
|
||||
.and_then(toml::Value::as_table)
|
||||
.expect("providers table");
|
||||
assert_eq!(
|
||||
providers["deepseek"]["model"].as_str(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
assert_eq!(providers["zai"]["model"].as_str(), Some("GLM-5.3"));
|
||||
|
||||
let dir = tempfile::tempdir().expect("config dir");
|
||||
let mut store = ConfigStore::load(Some(dir.path().join("config.toml"))).unwrap();
|
||||
let receipt = apply_bundle(&bundle, &mut store, BundleScope::Global, dir.path())
|
||||
.expect("export must import without alias conflicts");
|
||||
assert!(receipt.plan.conflicting.is_empty(), "{:?}", receipt.plan);
|
||||
assert_eq!(
|
||||
store
|
||||
.config
|
||||
.get_value("providers.deepseek.model")
|
||||
.as_deref(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
|
||||
// A root alias the active route still consumes stays shadowed state and
|
||||
// is dropped; a canonical DeepSeek leaf wins over a duplicate root
|
||||
// fallback.
|
||||
let config: ConfigToml = toml::from_str(
|
||||
"provider = 'zai'\ndefault_text_model = 'deepseek-v4-pro'\nmodel = 'GLM-5.1'\n[providers.zai]\nmodel = 'GLM-5.3'\n[providers.deepseek]\nmodel = 'deepseek-v4-flash'\n",
|
||||
)
|
||||
.unwrap();
|
||||
let bundle =
|
||||
export_bundle(&config, BundleScope::Global, BundleMetadata::default()).unwrap();
|
||||
assert!(!bundle.global.entries.contains_key("model"));
|
||||
assert!(!bundle.global.entries.contains_key("default_text_model"));
|
||||
let providers = bundle
|
||||
.global
|
||||
.entries
|
||||
.get("providers")
|
||||
.and_then(toml::Value::as_table)
|
||||
.expect("providers table");
|
||||
assert_eq!(
|
||||
providers["deepseek"]["model"].as_str(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imports_preserve_exact_builtin_shadowing_custom_provider_identity() {
|
||||
let dir = tempfile::tempdir().expect("config dir");
|
||||
|
||||
@@ -327,7 +327,9 @@ fn runtime_values_cannot_leak_through_a_command_that_saves_the_store() {
|
||||
.unwrap();
|
||||
assert_eq!(saved["provider"], "deepseek");
|
||||
assert_eq!(saved["sandbox_mode"], "workspace-write");
|
||||
assert_eq!(saved["default_text_model"], "deepseek-v4-pro");
|
||||
// Selected models persist in the canonical per-provider slot; root
|
||||
// default_text_model is legacy fallback only.
|
||||
assert_eq!(saved["providers"]["deepseek"]["model"], "deepseek-v4-pro");
|
||||
assert!(
|
||||
!fs::read_to_string(&fixture.config)
|
||||
.unwrap()
|
||||
|
||||
@@ -2736,11 +2736,15 @@ impl ConfigToml {
|
||||
self.selected_provider_id
|
||||
.as_deref()
|
||||
.filter(|id| {
|
||||
if self.providers.extras.contains_key(*id) {
|
||||
self.provider == ProviderKind::Custom
|
||||
if self.provider == ProviderKind::Custom {
|
||||
self.providers.extras.contains_key(*id)
|
||||
} else {
|
||||
self.provider != ProviderKind::Custom
|
||||
&& ProviderKind::parse_config_identity(id) == Some(self.provider)
|
||||
ProviderKind::parse_config_identity(id) == Some(self.provider)
|
||||
&& self.providers.extras.get(*id).is_none_or(|value| {
|
||||
value
|
||||
.as_table()
|
||||
.is_some_and(|table| !table.contains_key("kind"))
|
||||
})
|
||||
}
|
||||
})
|
||||
.unwrap_or_else(|| self.provider.as_str())
|
||||
@@ -2912,8 +2916,17 @@ impl ConfigToml {
|
||||
pub fn bind_persisted_provider_id(&mut self, provider_id: &str) -> Result<()> {
|
||||
let provider_id = provider_id.trim();
|
||||
let parsed = ProviderKind::parse_config_identity(provider_id);
|
||||
// Kindless tables mirroring a built-in alias remain inert. An explicit
|
||||
// custom declaration must validate; never fall back to a different
|
||||
// credential/endpoint authority when its kind is invalid.
|
||||
let custom_table = self.providers.extras.get(provider_id);
|
||||
let kindless_alias = parsed.is_some()
|
||||
&& custom_table
|
||||
.and_then(toml::Value::as_table)
|
||||
.is_some_and(|table| !table.contains_key("kind"));
|
||||
let provider = if parsed != Some(ProviderKind::Antigravity)
|
||||
&& self.providers.extras.contains_key(provider_id)
|
||||
&& custom_table.is_some()
|
||||
&& !kindless_alias
|
||||
{
|
||||
self.named_custom_provider_table(provider_id)?;
|
||||
ProviderKind::Custom
|
||||
|
||||
@@ -4720,6 +4720,62 @@ base_url = "https://acme.example/v1"
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn kindless_table_mirroring_a_builtin_alias_keeps_the_builtin_route() {
|
||||
let _lock = env_lock();
|
||||
let _env = EnvGuard::without_deepseek_runtime_overrides();
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let path = dir.path().join("config.toml");
|
||||
// A kindless `[providers.deepseek-cn]` table merely mirrors the regional
|
||||
// selector spelling. It is not an openai-compatible custom provider, so it
|
||||
// must stay inert: the selector still binds the built-in DeepSeek kind and
|
||||
// the load must not fail.
|
||||
fs::write(
|
||||
&path,
|
||||
"provider = 'deepseek-cn'\n[providers.deepseek-cn]\nmodel = 'deepseek-v4-flash'\n",
|
||||
)
|
||||
.expect("kindless alias fixture");
|
||||
let mut store = ConfigStore::load(Some(path.clone())).expect("kindless alias table must load");
|
||||
assert_eq!(store.config.provider, ProviderKind::Deepseek);
|
||||
assert_eq!(store.config.provider_id(), "deepseek-cn");
|
||||
assert!(store.config.named_custom_provider_id().is_none());
|
||||
// An unrelated typed save leaves the inert extras table untouched.
|
||||
store.config.set_value("verbosity", "quiet").unwrap();
|
||||
store.save().unwrap();
|
||||
let saved: toml::Value = toml::from_str(&fs::read_to_string(&path).unwrap()).unwrap();
|
||||
assert_eq!(saved["provider"].as_str(), Some("deepseek-cn"));
|
||||
assert_eq!(
|
||||
saved["providers"]["deepseek-cn"]["model"].as_str(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
let reloaded = ConfigStore::load(Some(path.clone())).expect("reload kindless alias config");
|
||||
assert_eq!(reloaded.config.provider, ProviderKind::Deepseek);
|
||||
assert_eq!(reloaded.config.provider_id(), "deepseek-cn");
|
||||
|
||||
// A table that does validate as openai-compatible still takes precedence
|
||||
// over the built-in alias.
|
||||
fs::write(
|
||||
&path,
|
||||
"provider = 'deepseek-cn'\n[providers.deepseek-cn]\nkind = 'openai-compatible'\nbase_url = 'https://gateway.example/v1'\nmodel = 'Exact-CN'\n",
|
||||
)
|
||||
.expect("valid custom table fixture");
|
||||
let store = ConfigStore::load(Some(path)).expect("valid custom table takes precedence");
|
||||
assert_eq!(store.config.provider, ProviderKind::Custom);
|
||||
assert_eq!(store.config.provider_id(), "deepseek-cn");
|
||||
assert_eq!(store.config.named_custom_provider_id(), Some("deepseek-cn"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_custom_kind_never_falls_back_to_a_builtin_alias() {
|
||||
for kind in ["'unsupported'", "42", "false"] {
|
||||
let mut config: ConfigToml = toml::from_str(&format!(
|
||||
"provider = 'deepseek-cn'\n[providers.deepseek-cn]\nkind = {kind}\n"
|
||||
))
|
||||
.unwrap();
|
||||
assert!(config.bind_persisted_provider_id("deepseek-cn").is_err());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_kind_accepts_legacy_deepseek_cn_aliases() {
|
||||
for alias in [
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
---
|
||||
name: mcp-discovery
|
||||
description: "Prefer a matching zero-environment local MCP server for specialized tasks, even when built-in shell commands or custom code could also solve them."
|
||||
---
|
||||
|
||||
# mcp-discovery
|
||||
|
||||
Use the Registry as the preferred source of specialized capabilities. Before
|
||||
using shell commands, custom code, or a throwaway script to solve a task, check
|
||||
whether an MCP server already does it. The public MCP Registry ships hundreds
|
||||
of ready-made servers (filesystems, databases, browsers, media processing,
|
||||
developer utilities, cloud APIs, SaaS integrations, …).
|
||||
|
||||
The discovery and structured start tools are available in the active tool
|
||||
surface whenever MCP support is enabled.
|
||||
|
||||
## When to use
|
||||
|
||||
- You are about to fetch data, reach a service, drive a tool, or automate a workflow.
|
||||
- The task needs a capability the current toolset lacks (a database, a
|
||||
browser, a third-party API, …).
|
||||
- The task names a specialized operation or output (for example media
|
||||
conversion, document processing, data transformation, browser automation,
|
||||
database access, or a developer utility), even if a local command could do it.
|
||||
- Skip only ordinary repo-native editing and simple shell/file operations that
|
||||
do not need a specialized capability.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. **Check the registry.** Call `registry_sync {}`. It returns the complete
|
||||
catalog of eligible local stdio packages, including each server's name,
|
||||
description, and required launch arguments. Packages declaring any
|
||||
environment variable (including API keys/tokens) are excluded and never
|
||||
written to the cache.
|
||||
2. **Match from context with a Registry-first bias.** Compare the user's full
|
||||
task against every server name and description. A candidate is a match when
|
||||
it plausibly covers the task's core specialized capability; wording does not
|
||||
need to be exact. When such a candidate exists, you **must start it and inspect
|
||||
its tools before** using `exec_shell`, local programs, custom code, or a manual
|
||||
implementation. The availability or familiarity of a local alternative is
|
||||
not a reason to skip the candidate. Skip Registry use only when every entry is
|
||||
clearly irrelevant, or when a matching server fails to start after the retry
|
||||
described below.
|
||||
3. **Install + run transactionally.** Call
|
||||
`start_registry_mcp_server {registry_name: "<exact name>", arguments: {...}}`.
|
||||
Supply only values listed in `required_args`; omit `arguments` when none
|
||||
are required. Never install or launch the package through `exec_shell`.
|
||||
4. **Solve the task with the new tools.** Their complete schemas are added
|
||||
to the current turn immediately after a successful connection; call the
|
||||
exact names returned by the start result.
|
||||
|
||||
## If a server fails to start
|
||||
|
||||
`start_registry_mcp_server` reports when a package exits before the handshake
|
||||
(often CLI help output = incomplete launch args). Verify the exact
|
||||
required arguments, retry once with the corrected structured values, and if
|
||||
it still fails move on to the next candidate. Failed starts are rolled back,
|
||||
so retrying the same Registry name is safe.
|
||||
|
||||
## Don't
|
||||
|
||||
- Don't attempt to pass env vars or secrets; this flow has no env channel.
|
||||
- Don't reconstruct or edit the Registry-provided package command.
|
||||
@@ -1,61 +1,65 @@
|
||||
---
|
||||
name: mcp-discovery
|
||||
description: "Prefer a matching zero-environment local MCP server for specialized tasks, even when built-in shell commands or custom code could also solve them."
|
||||
description: "Find and start a zero-environment local MCP server when the session lacks a capability that no available tool, project script, or ordinary local code can cover."
|
||||
---
|
||||
|
||||
# mcp-discovery
|
||||
|
||||
Use the Registry as the preferred source of specialized capabilities. Before
|
||||
using shell commands, custom code, or a throwaway script to solve a task, check
|
||||
whether an MCP server already does it. The public MCP Registry ships hundreds
|
||||
of ready-made servers (filesystems, databases, browsers, media processing,
|
||||
developer utilities, cloud APIs, SaaS integrations, …).
|
||||
The public MCP Registry ships hundreds of ready-made stdio servers
|
||||
(filesystems, databases, browsers, media processing, developer utilities, cloud
|
||||
APIs, SaaS integrations, …). This skill is how you reach one when the session
|
||||
is genuinely missing a capability.
|
||||
|
||||
The discovery and structured start tools are available in the active tool
|
||||
surface whenever MCP support is enabled.
|
||||
It is a fallback, not a first step. Discovery costs a `tool_search` to load the
|
||||
deferred tools, a query, an approval, and an install — spend that only when it
|
||||
buys a capability you do not have.
|
||||
|
||||
## When to use
|
||||
|
||||
- You are about to fetch data, reach a service, drive a tool, or automate a workflow.
|
||||
- The task needs a capability the current toolset lacks (a database, a
|
||||
browser, a third-party API, …).
|
||||
- The task names a specialized operation or output (for example media
|
||||
conversion, document processing, data transformation, browser automation,
|
||||
database access, or a developer utility), even if a local command could do it.
|
||||
- Skip only ordinary repo-native editing and simple shell/file operations that
|
||||
do not need a specialized capability.
|
||||
- An available tool, a project script or test, or a few lines of local code
|
||||
cannot do the job.
|
||||
- The task needs an external service, database, or protocol client that this
|
||||
session has no access to.
|
||||
- You would otherwise install a package or reimplement a well-known converter
|
||||
or client by hand.
|
||||
|
||||
## When not to use
|
||||
|
||||
- Ordinary work: writing and editing files, reading fixtures, running repo
|
||||
commands, and checking your own output. Do those directly.
|
||||
- A capability an already-loaded tool covers, even partially — try it first.
|
||||
- A near-miss row in a result list. A scored match is a suggestion; the missing
|
||||
capability is what decides.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. **Check the registry.** Call `registry_sync {}`. It returns the complete
|
||||
catalog of eligible local stdio packages, including each server's name,
|
||||
description, and required launch arguments. Packages declaring any
|
||||
environment variable (including API keys/tokens) are excluded and never
|
||||
written to the cache.
|
||||
2. **Match from context with a Registry-first bias.** Compare the user's full
|
||||
task against every server name and description. A candidate is a match when
|
||||
it plausibly covers the task's core specialized capability; wording does not
|
||||
need to be exact. When such a candidate exists, you **must start it and inspect
|
||||
its tools before** using `exec_shell`, local programs, custom code, or a manual
|
||||
implementation. The availability or familiarity of a local alternative is
|
||||
not a reason to skip the candidate. Skip Registry use only when every entry is
|
||||
clearly irrelevant, or when a matching server fails to start after the retry
|
||||
described below.
|
||||
3. **Install + run transactionally.** Call
|
||||
1. **Name the gap.** State the capability you lack before searching. If you
|
||||
cannot name one, you do not need the Registry.
|
||||
2. **Load and query.** `registry_sync` and `start_registry_mcp_server` are
|
||||
deferred: load one with `tool_search` and use the returned schema. If a call
|
||||
only loads the schema without executing, retry once with that schema. Then call
|
||||
`registry_sync {query: "<the missing capability>"}`. It scores a host-side
|
||||
snapshot and returns at most eight matches; the full index never enters the
|
||||
conversation. Packages declaring any environment variable (including API
|
||||
keys/tokens) are excluded and never written to the cache.
|
||||
3. **Judge the matches.** Take a server when it covers the gap you named. If
|
||||
nothing does, refine the query once, then continue with local tools.
|
||||
4. **Install + run transactionally.** Call
|
||||
`start_registry_mcp_server {registry_name: "<exact name>", arguments: {...}}`.
|
||||
Supply only values listed in `required_args`; omit `arguments` when none
|
||||
are required. Never install or launch the package through `exec_shell`.
|
||||
4. **Solve the task with the new tools.** Their complete schemas are added
|
||||
to the current turn immediately after a successful connection; call the
|
||||
exact names returned by the start result.
|
||||
Supply only values listed in `required_args`; omit `arguments` when none are
|
||||
required. Never install or launch the package through the shell. Starting a
|
||||
server requires approval.
|
||||
5. **Solve the task with the new tools.** Their complete schemas are added to
|
||||
the current turn immediately after a successful connection; call the exact
|
||||
names returned by the start result.
|
||||
|
||||
## If a server fails to start
|
||||
|
||||
`start_registry_mcp_server` reports when a package exits before the handshake
|
||||
(often CLI help output = incomplete launch args). Verify the exact
|
||||
required arguments, retry once with the corrected structured values, and if
|
||||
it still fails move on to the next candidate. Failed starts are rolled back,
|
||||
so retrying the same Registry name is safe.
|
||||
(often CLI help output = incomplete launch args). Verify the exact required
|
||||
arguments, retry once with the corrected structured values, and if it still
|
||||
fails move on to the next candidate or to a local approach. Failed starts are
|
||||
rolled back, so retrying the same Registry name is safe.
|
||||
|
||||
## Don't
|
||||
|
||||
|
||||
@@ -700,7 +700,7 @@
|
||||
"SettingsTuiPrefsQuarantined": "tui.toml keys with no setting were kept in {path}: {keys}",
|
||||
"ClearConversation": "Conversation cleared",
|
||||
"ClearConversationBusy": "Nothing cleared — still busy. Try /clear again in a moment.",
|
||||
"ModelChanged": "Model changed: {old} →, {new}",
|
||||
"ModelChanged": "Operator model changed: {old} → {new}",
|
||||
"LinksProjectTitle": "Codewhale & community:",
|
||||
"LinksDocumentation": "Documentation:",
|
||||
"LinksCommunity": "Community & contribution:",
|
||||
|
||||
@@ -700,7 +700,7 @@
|
||||
"SettingsTuiPrefsQuarantined": "没有对应设置的 tui.toml 键已保留在 {path}: {keys}",
|
||||
"ClearConversation": "对话已清空",
|
||||
"ClearConversationBusy": "未清空任何内容(工作状态或运行时任务正忙;请等待后重试 /clear)",
|
||||
"ModelChanged": "模型已切换:{old} → {new}",
|
||||
"ModelChanged": "主控模型已切换:{old} → {new}",
|
||||
"LinksProjectTitle": "Codewhale 与社区:",
|
||||
"LinksDocumentation": "文档:",
|
||||
"LinksCommunity": "社区与贡献:",
|
||||
|
||||
@@ -1162,7 +1162,7 @@
|
||||
"LinksTip": "提示:使用所顯示提供商的環境變數,或通過 `codewhale auth set --provider <id>` 儲存金鑰。",
|
||||
"LinksTitle": "提供商鏈接:",
|
||||
"ModeAlreadyActiveSavedAsDefault": "已處於 {mode} 模式 — 已儲存為啟動預設值。",
|
||||
"ModelChanged": "模型已切換:{old} → {new}",
|
||||
"ModelChanged": "主控模型已切換:{old} → {new}",
|
||||
"ModelPickerAutoLastRoute": "上次 {provider} · {model}",
|
||||
"ModelPickerAutoLocalHint": "每回合 · 本機啟發式 · 不傳送路由請求",
|
||||
"ModelPickerAutoNetworkActiveProviderHint": "每回合 · 僅目前供應商 · 請求與近期內容 → {provider} / {model}",
|
||||
|
||||
@@ -1907,14 +1907,35 @@ pub fn set_config_value(app: &mut App, key: &str, value: &str, persist: bool) ->
|
||||
let model = if value.eq_ignore_ascii_case("auto") {
|
||||
"auto".to_string()
|
||||
} else {
|
||||
let Some(model) = normalize_model_name_for_provider(ApiProvider::Deepseek, value)
|
||||
else {
|
||||
return CommandResult::error(format!("Invalid DeepSeek model '{value}'."));
|
||||
// Route-aware, matching POST /v1/config: a custom DeepSeek
|
||||
// endpoint owns its model namespace, so an id declared for the
|
||||
// saved DeepSeek route validates verbatim before the catalog
|
||||
// normalization runs.
|
||||
let saved = match load_command_config(app) {
|
||||
Ok(config) => config,
|
||||
Err(err) => return CommandResult::error(err),
|
||||
};
|
||||
if let Err(error) = validate_route(ApiProvider::Deepseek, &model) {
|
||||
return CommandResult::error(error);
|
||||
if crate::provider_lake::configured_model_for_route(
|
||||
&saved,
|
||||
ApiProvider::Deepseek,
|
||||
&saved.provider_identity_for(ApiProvider::Deepseek),
|
||||
&saved.base_url_for_route(ApiProvider::Deepseek),
|
||||
value.trim(),
|
||||
)
|
||||
.is_some()
|
||||
{
|
||||
value.trim().to_string()
|
||||
} else {
|
||||
let Some(model) =
|
||||
normalize_model_name_for_provider(ApiProvider::Deepseek, value)
|
||||
else {
|
||||
return CommandResult::error(format!("Invalid DeepSeek model '{value}'."));
|
||||
};
|
||||
if let Err(error) = validate_route(ApiProvider::Deepseek, &model) {
|
||||
return CommandResult::error(error);
|
||||
}
|
||||
model
|
||||
}
|
||||
model
|
||||
};
|
||||
return match crate::config_persistence::persist_provider_model_key(
|
||||
app.config_path.as_deref(),
|
||||
@@ -1936,7 +1957,8 @@ pub fn set_config_value(app: &mut App, key: &str, value: &str, persist: bool) ->
|
||||
// namespace. Provider-only normalization would reject a non-DeepSeek
|
||||
// id that the live session is already allowed to use via `/model`.
|
||||
// OpenCode Go stays protocol-strict even on a custom host.
|
||||
let model = if value.trim().eq_ignore_ascii_case("auto") {
|
||||
let auto_select = value.trim().eq_ignore_ascii_case("auto");
|
||||
let model = if auto_select {
|
||||
"auto".to_string()
|
||||
} else if app.api_provider == ApiProvider::OpencodeGo {
|
||||
let Some(model) = normalize_model_name_for_provider(app.api_provider, value) else {
|
||||
@@ -4012,6 +4034,103 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saved_automatic_model_selection_round_trips_every_provider_route() {
|
||||
let temp_root = tempfile::tempdir().expect("isolated configuration");
|
||||
let _guard = EnvGuard::new(temp_root.path());
|
||||
let config_path = crate::config_persistence::config_toml_path(None).unwrap();
|
||||
fs::create_dir_all(config_path.parent().unwrap()).unwrap();
|
||||
for (provider, selector, table, initial) in [
|
||||
(
|
||||
ApiProvider::Deepseek,
|
||||
"deepseek",
|
||||
"deepseek",
|
||||
"deepseek-v4-pro",
|
||||
),
|
||||
(
|
||||
ApiProvider::DeepseekCN,
|
||||
"deepseek-cn",
|
||||
"deepseek_cn",
|
||||
"deepseek-v4-pro",
|
||||
),
|
||||
(ApiProvider::Zai, "zai", "zai", "GLM-5.3"),
|
||||
] {
|
||||
fs::write(
|
||||
&config_path,
|
||||
format!("provider = '{selector}'\n[providers.{table}]\nmodel = '{initial}'\n"),
|
||||
)
|
||||
.unwrap();
|
||||
let mut app = create_test_app();
|
||||
app.set_provider_identity(provider, selector);
|
||||
app.model = initial.to_string();
|
||||
app.auto_model = false;
|
||||
let result = set_config_value(&mut app, "model", "auto", true);
|
||||
assert!(!result.is_error, "{:?}", result.message);
|
||||
assert!(app.auto_model);
|
||||
assert!(
|
||||
result
|
||||
.message
|
||||
.as_deref()
|
||||
.is_some_and(|m| m.contains("saved"))
|
||||
);
|
||||
let persisted: toml::Value =
|
||||
toml::from_str(&fs::read_to_string(&config_path).unwrap()).unwrap();
|
||||
assert_eq!(
|
||||
persisted["providers"][table]["model"].as_str(),
|
||||
Some("auto")
|
||||
);
|
||||
let loaded = Config::load(Some(config_path.clone()), None).unwrap();
|
||||
assert_eq!(
|
||||
loaded.default_model(),
|
||||
"auto",
|
||||
"{selector} must consume its saved choice"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_default_model_save_accepts_models_declared_for_the_deepseek_route() {
|
||||
let temp_root = tempfile::tempdir().expect("isolated configuration");
|
||||
let _guard = EnvGuard::new(temp_root.path());
|
||||
// The declaration matches on the saved DeepSeek route's base URL; keep
|
||||
// ambient endpoint overrides out of the comparison.
|
||||
let _base_url_env = [
|
||||
EnvVarGuard::remove("CODEWHALE_BASE_URL"),
|
||||
EnvVarGuard::remove("DEEPSEEK_BASE_URL"),
|
||||
];
|
||||
let config_path = crate::config_persistence::config_toml_path(None).expect("config path");
|
||||
fs::create_dir_all(config_path.parent().expect("config directory")).expect("mkdir");
|
||||
fs::write(
|
||||
&config_path,
|
||||
"provider = 'zai'\n[providers.zai]\nmodel = 'GLM-5.2'\n[providers.deepseek]\nbase_url = 'https://my-gateway.example/v1'\n[[custom_models]]\nprovider = 'deepseek'\nbase_url = 'https://my-gateway.example/v1'\nid = 'my-llm'\n",
|
||||
)
|
||||
.expect("config");
|
||||
|
||||
let mut app = create_test_app();
|
||||
app.api_provider = ApiProvider::Zai;
|
||||
app.model = crate::config::ZAI_GLM_5_2_MODEL.to_string();
|
||||
app.auto_model = false;
|
||||
|
||||
let result = set_config_value(&mut app, "default_model", "my-llm", true);
|
||||
|
||||
assert!(!result.is_error, "{:?}", result.message);
|
||||
let persisted: toml::Value =
|
||||
toml::from_str(&fs::read_to_string(&config_path).expect("saved config")).expect("toml");
|
||||
assert_eq!(
|
||||
persisted["providers"]["deepseek"]["model"].as_str(),
|
||||
Some("my-llm")
|
||||
);
|
||||
// The same id on a different DeepSeek endpoint is not declared for the
|
||||
// saved route and must still be rejected by catalog normalization.
|
||||
fs::write(
|
||||
&config_path,
|
||||
"provider = 'zai'\n[providers.zai]\nmodel = 'GLM-5.2'\n[providers.deepseek]\nbase_url = 'https://other-gateway.example/v1'\n[[custom_models]]\nprovider = 'deepseek'\nbase_url = 'https://my-gateway.example/v1'\nid = 'my-llm'\n",
|
||||
)
|
||||
.expect("config");
|
||||
let rejected = set_config_value(&mut app, "default_model", "my-llm", true);
|
||||
assert!(rejected.is_error, "{:?}", rejected.message);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saved_model_display_uses_the_same_profile_root_precedence_as_startup() {
|
||||
let temp_root = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -4847,6 +4847,7 @@ impl Config {
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn remembered_selection_is_applicable(&self) -> bool {
|
||||
self.remembered_selection_scope.unwrap_or(true)
|
||||
}
|
||||
@@ -6182,6 +6183,11 @@ impl Config {
|
||||
self.provider_config_string_with_runtime_fallback(provider, |entry| entry.model.clone())
|
||||
{
|
||||
let model = model.trim();
|
||||
// Automatic selection is a saved choice on every route, including
|
||||
// DeepSeek. Resolve it before provider model-name normalization.
|
||||
if model.eq_ignore_ascii_case("auto") {
|
||||
return "auto".to_string();
|
||||
}
|
||||
if declared(model)
|
||||
|| provider_passes_model_through(provider)
|
||||
|| self.active_provider_preserves_custom_base_url_model()
|
||||
|
||||
@@ -125,7 +125,7 @@ fn agent_list_event(manager: &SubAgentManager, active_session_id: &str) -> Event
|
||||
}
|
||||
|
||||
const MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE: &str = "runtime:mcp-registry-first";
|
||||
const MCP_REGISTRY_FIRST_INSTRUCTION: &str = "## MCP Registry-first policy\n\nFor any task centered on a specialized capability, including media or document conversion, data transformation, browser automation, database or service access, or a developer utility, you must call `registry_sync` with a `query` describing that capability before `exec_shell`, `fetch_url`, code execution, local programs, custom code, or a manual implementation. It scores the local Registry snapshot host-side and returns at most eight matches; the full catalog never enters the conversation. Treat a returned server as a match when it plausibly covers the core capability; wording need not be exact. If any plausible match exists, you must call `start_registry_mcp_server` with its exact name and inspect its tools before considering a local alternative. If nothing matches, refine the query once; a still-empty refined result means every Registry entry is clearly irrelevant. An installed or familiar shell command is not a reason to skip Registry discovery. Use local tools directly only for ordinary repo-native work and simple file operations, or after the matching server fails to start.";
|
||||
const MCP_REGISTRY_FIRST_INSTRUCTION: &str = "## MCP Registry\n\nThe Registry installs and connects a local MCP server when this session lacks a capability. It is a fallback for a capability you do not have, not a step before ordinary work.\n\nPrefer what is already available, in order: tools already in this catalog, the project's own scripts, tests, and dev tooling, and platform capabilities. Creating a file, reading a fixture, running a repo command, and checking your own output are ordinary work — do them directly.\n\nReach for the Registry once you have identified a specific capability that no available tool covers and that you would otherwise install or reimplement, such as a document or media converter, access to an external database or service, or a protocol client. Then call `registry_sync` with a `query` naming that capability; it scores the local Registry snapshot host-side and returns at most eight matches, so the full index never enters the conversation. When a returned server plausibly covers that capability, call `start_registry_mcp_server` with its exact name rather than installing or running its package command through the shell. If nothing matches, refine the query once, then continue with local tools.\n\nBoth Registry tools are deferred: load one with `tool_search` before its first call, and use the returned schema. If a call instead reports that it only loaded the schema, retry once with that schema. Do not go searching for them for work you can already do.";
|
||||
const ISOLATED_CHAT_ENGINE_PROMPT: &str = "You are Codewhale Chat. Answer the user's request directly and conversationally. This isolated chat-only session has no local workspace, project, memory, skill, account, credential, path, runtime context, or tools.";
|
||||
|
||||
pub(crate) fn sanitize_isolated_chat_attachments(mut text: String) -> String {
|
||||
@@ -1469,10 +1469,15 @@ impl Engine {
|
||||
}
|
||||
|
||||
// Unlike a Skill body, this instruction is visible on the first model
|
||||
// request. Keep selection semantic: the host supplies no keywords or
|
||||
// ranking and the model compares the full user context with the full
|
||||
// Registry catalog. Append it after configured instruction sources so
|
||||
// the Registry-first decision sits close to the current user turn.
|
||||
// request. Registry discovery is a fallback for missing capabilities;
|
||||
// result matching stays with the model and the index stays host-side.
|
||||
//
|
||||
// It describes when discovery is worth a turn; it is not a gate ahead
|
||||
// of ordinary work. The earlier "must call `registry_sync` before a
|
||||
// manual implementation" phrasing named two deferred tools as
|
||||
// mandatory, so a plain "write an HTML page and read a fixture" turn
|
||||
// spent its steps on `tool_search` for `registry_sync` and on starting
|
||||
// a browser server instead of writing the file.
|
||||
if config.features.enabled(Feature::Mcp) && !api_config.runtime_chat_isolated {
|
||||
config
|
||||
.instructions
|
||||
@@ -4661,16 +4666,13 @@ impl Engine {
|
||||
let capability = route.capability_profile();
|
||||
let always_load = self.config.tools_always_load.clone();
|
||||
self.turn_tool_surface_budget = Some(capability.tool_surface_budget);
|
||||
let mut catalog = build_model_tool_catalog_with_surface(
|
||||
let catalog = build_model_tool_catalog_with_surface(
|
||||
tool_registry.to_api_tools_with_cache(true),
|
||||
mcp_tools,
|
||||
input_policy.mode,
|
||||
&always_load,
|
||||
capability.tool_surface_budget,
|
||||
);
|
||||
if self.config.features.enabled(Feature::Mcp) {
|
||||
apply_registry_first_shell_guidance(&mut catalog);
|
||||
}
|
||||
let surface = ToolSurfacePolicy::new(
|
||||
tool_registry,
|
||||
Some(catalog),
|
||||
@@ -8160,9 +8162,9 @@ use self::streaming::{
|
||||
use self::tool_catalog::{
|
||||
CODE_EXECUTION_TOOL_NAME, JS_EXECUTION_TOOL_NAME, MULTI_TOOL_PARALLEL_NAME,
|
||||
REQUEST_USER_INPUT_NAME, ToolSurfacePolicy, active_tools_for_request,
|
||||
apply_registry_first_shell_guidance, build_model_tool_catalog_with_surface,
|
||||
default_synthetic_catalog_tool_names, execute_code_execution_tool, is_tool_search_tool,
|
||||
maybe_hydrate_requested_deferred_tool, missing_tool_error_message,
|
||||
build_model_tool_catalog_with_surface, default_synthetic_catalog_tool_names,
|
||||
execute_code_execution_tool, is_tool_search_tool, maybe_hydrate_requested_deferred_tool,
|
||||
missing_tool_error_message,
|
||||
};
|
||||
#[cfg(test)]
|
||||
use self::tool_catalog::{
|
||||
|
||||
@@ -616,58 +616,80 @@ fn ordinary_engine_default_has_a_finite_step_budget() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn registry_first_scenario() {
|
||||
// Scenario consolidation of: registry_first_policy_is_in_the_initial_prompt_only_when_mcp_is_enabled, registry_first_guidance_is_attached_to_the_shell_fallback_once
|
||||
// from registry_first_policy_is_in_the_initial_prompt_only_when_mcp_is_enabled
|
||||
{
|
||||
let enabled = EngineConfig::default();
|
||||
let (engine, _handle) = Engine::new(enabled, &Config::default());
|
||||
let prompt = crate::prompts::system_prompt_flat_text(
|
||||
engine
|
||||
.session
|
||||
.system_prompt
|
||||
.as_ref()
|
||||
.expect("system prompt"),
|
||||
);
|
||||
assert!(prompt.contains(MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE));
|
||||
fn registry_instruction_is_in_the_initial_prompt_only_when_mcp_is_enabled() {
|
||||
let enabled = EngineConfig::default();
|
||||
let (engine, _handle) = Engine::new(enabled, &Config::default());
|
||||
let prompt = crate::prompts::system_prompt_flat_text(
|
||||
engine
|
||||
.session
|
||||
.system_prompt
|
||||
.as_ref()
|
||||
.expect("system prompt"),
|
||||
);
|
||||
assert!(prompt.contains(MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE));
|
||||
assert!(prompt.contains("registry_sync"));
|
||||
assert!(prompt.contains("start_registry_mcp_server"));
|
||||
|
||||
let mut disabled = EngineConfig::default();
|
||||
disabled.features.disable(Feature::Mcp);
|
||||
let (engine, _handle) = Engine::new(disabled, &Config::default());
|
||||
let prompt = crate::prompts::system_prompt_flat_text(
|
||||
engine
|
||||
.session
|
||||
.system_prompt
|
||||
.as_ref()
|
||||
.expect("system prompt"),
|
||||
);
|
||||
assert!(!prompt.contains(MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE));
|
||||
}
|
||||
|
||||
/// The regression this test exists for. A real DeepSeek turn — "build a
|
||||
/// self-contained HTML focus timer, read a local fixture, verify it" — spent
|
||||
/// its steps on five `tool_search` calls for `registry_sync`, a deferred-schema
|
||||
/// retry, and reasoning about starting a browser MCP server, because the
|
||||
/// always-visible instruction ordered Registry discovery *before* code
|
||||
/// execution or a manual implementation and named two tools that are not in the
|
||||
/// catalog head.
|
||||
///
|
||||
/// Two properties keep that from coming back, and both are about this prompt,
|
||||
/// not about a second policy surface: discovery is never ordered ahead of
|
||||
/// ordinary local work, and the deferred-tool cost of reaching the Registry
|
||||
/// tools is stated where the model reads about them.
|
||||
#[test]
|
||||
fn registry_instruction_does_not_gate_ordinary_local_work() {
|
||||
let (engine, _handle) = Engine::new(EngineConfig::default(), &Config::default());
|
||||
let prompt = crate::prompts::system_prompt_flat_text(
|
||||
engine
|
||||
.session
|
||||
.system_prompt
|
||||
.as_ref()
|
||||
.expect("system prompt"),
|
||||
);
|
||||
assert!(prompt.contains("## MCP Registry"));
|
||||
|
||||
for ordered in [
|
||||
"must call `registry_sync`",
|
||||
"before `exec_shell`",
|
||||
"you must call `start_registry_mcp_server`",
|
||||
"not a reason to skip Registry discovery",
|
||||
] {
|
||||
assert!(
|
||||
prompt.contains("must call `registry_sync` with a `query` describing that capability")
|
||||
!prompt.contains(ordered),
|
||||
"Registry guidance must not order discovery ahead of ordinary work: {ordered:?}"
|
||||
);
|
||||
|
||||
let mut disabled = EngineConfig::default();
|
||||
disabled.features.disable(Feature::Mcp);
|
||||
let (engine, _handle) = Engine::new(disabled, &Config::default());
|
||||
let prompt = crate::prompts::system_prompt_flat_text(
|
||||
engine
|
||||
.session
|
||||
.system_prompt
|
||||
.as_ref()
|
||||
.expect("system prompt"),
|
||||
);
|
||||
assert!(!prompt.contains(MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE));
|
||||
}
|
||||
// from registry_first_guidance_is_attached_to_the_shell_fallback_once
|
||||
{
|
||||
let mut catalog = vec![api_tool("read_file"), api_tool("exec_shell")];
|
||||
|
||||
apply_registry_first_shell_guidance(&mut catalog);
|
||||
let after_first = catalog
|
||||
.iter()
|
||||
.find(|tool| tool.name == "exec_shell")
|
||||
.expect("shell tool")
|
||||
.description
|
||||
.clone();
|
||||
apply_registry_first_shell_guidance(&mut catalog);
|
||||
// Available capability comes first, and the deferred cost is disclosed
|
||||
// where the model reads about the two tools it would have to hunt for.
|
||||
assert!(prompt.contains("Prefer what is already available"));
|
||||
assert!(prompt.contains("not a step before ordinary work"));
|
||||
assert!(prompt.contains("Both Registry tools are deferred"));
|
||||
assert!(prompt.contains("load one with `tool_search`"));
|
||||
|
||||
let after_second = &catalog
|
||||
.iter()
|
||||
.find(|tool| tool.name == "exec_shell")
|
||||
.expect("shell tool")
|
||||
.description;
|
||||
assert_eq!(after_second, &after_first);
|
||||
assert!(after_second.contains("registry_sync"));
|
||||
assert!(after_second.contains("start_registry_mcp_server"));
|
||||
}
|
||||
// The trust boundary the instruction is actually load-bearing for: a
|
||||
// Registry package is started through the approved host path, never
|
||||
// installed or run through the shell.
|
||||
assert!(prompt.contains("rather than installing or running its package command"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -21337,6 +21359,28 @@ async fn reasoning_only_clean_stop_is_retried_and_recovers() {
|
||||
async fn reasoning_only_length_stop_fails_without_retry() {
|
||||
let (model, events) = run_reasoning_only_turn(1, "length").await;
|
||||
|
||||
let diagnostic = events
|
||||
.iter()
|
||||
.find_map(|event| match event {
|
||||
Event::ToolRequestSnapshot { snapshot } => snapshot.terminal.as_ref(),
|
||||
_ => None,
|
||||
})
|
||||
.expect("terminal request diagnostics");
|
||||
assert!(
|
||||
diagnostic
|
||||
.last_prepared_output_limit_tokens
|
||||
.is_some_and(|tokens| tokens > 0)
|
||||
);
|
||||
assert!(
|
||||
events.iter().any(|event| matches!(
|
||||
event,
|
||||
Event::Error { envelope, .. }
|
||||
if envelope.message.contains("response output limit")
|
||||
&& envelope.message.contains("including reasoning")
|
||||
)),
|
||||
"a length stop must explain the actual output constraint"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
model.calls.load(std::sync::atomic::Ordering::SeqCst),
|
||||
1,
|
||||
|
||||
@@ -204,31 +204,13 @@ pub(super) fn build_model_tool_catalog_with_surface(
|
||||
native_tools
|
||||
}
|
||||
|
||||
const REGISTRY_FIRST_SHELL_GUIDANCE: &str = "Before using this tool for a task whose core operation is a specialized capability (for example media or document conversion, data transformation, browser automation, database or service access, or a developer utility), call registry_sync with a query describing that capability; it returns at most eight scored matches from the host-side Registry snapshot. If a returned match plausibly covers the operation, call start_registry_mcp_server and inspect the connected tools before using a shell alternative. Use the shell directly for ordinary repo-native work and simple file operations, or after no match (or one refined query) is plausible or the matching server fails to start.";
|
||||
|
||||
/// Put the Registry-first decision at the point where the model considers its
|
||||
/// strongest fallback. The discovery skill body is lazy-loaded, so relying on
|
||||
/// it alone creates a loop: the model must already prefer discovery before it
|
||||
/// can read the instruction that tells it to prefer discovery.
|
||||
///
|
||||
/// This is applied only while MCP is enabled. It changes no dispatch order and
|
||||
/// performs no task matching in the host; the model still compares the user's
|
||||
/// context against the Registry catalog itself.
|
||||
pub(super) fn apply_registry_first_shell_guidance(catalog: &mut [Tool]) {
|
||||
// The small-contract-shaped lowercase bash schema stays small and direct. This legacy
|
||||
// compatibility hook is intentionally inert unless an old model-visible
|
||||
// exec_shell definition is present.
|
||||
let Some(shell) = catalog.iter_mut().find(|tool| tool.name == "exec_shell") else {
|
||||
return;
|
||||
};
|
||||
if shell.description.contains(REGISTRY_FIRST_SHELL_GUIDANCE) {
|
||||
return;
|
||||
}
|
||||
if !shell.description.ends_with(char::is_whitespace) {
|
||||
shell.description.push(' ');
|
||||
}
|
||||
shell.description.push_str(REGISTRY_FIRST_SHELL_GUIDANCE);
|
||||
}
|
||||
// A second Registry authority used to live here: it appended a "call
|
||||
// registry_sync before this tool" paragraph to a model-visible `exec_shell`
|
||||
// description. The model-visible shell tool is `bash` — `exec_shell` is only a
|
||||
// canonical *action* name (see `tools::canonical_action`) — so the hook never
|
||||
// fired on a live catalog, and its own test pinned that it must not touch
|
||||
// `bash`. The Registry instruction in `Engine::new` is the single prompt
|
||||
// authority for this decision; a per-tool copy of it is not revived here.
|
||||
|
||||
pub(super) fn apply_tool_surface_budget(
|
||||
catalog: &mut [Tool],
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
use super::{
|
||||
CODE_EXECUTION_DESCRIPTION, DEFAULT_ACTIVE_NATIVE_TOOLS,
|
||||
allowlist_is_native_file_and_shell_only, apply_mcp_tool_deferral, apply_native_tool_deferral,
|
||||
apply_registry_first_shell_guidance, build_model_tool_catalog_with_surface,
|
||||
default_synthetic_catalog_tool_names, ensure_advanced_tooling, execute_tool_search_with_cache,
|
||||
initial_active_tools, is_synthetic_catalog_tool, remove_evicted_cache_activations,
|
||||
tool_matches_any_rule, touch_cached_tool_after_execution,
|
||||
build_model_tool_catalog_with_surface, default_synthetic_catalog_tool_names,
|
||||
ensure_advanced_tooling, execute_tool_search_with_cache, initial_active_tools,
|
||||
is_synthetic_catalog_tool, remove_evicted_cache_activations, tool_matches_any_rule,
|
||||
touch_cached_tool_after_execution,
|
||||
};
|
||||
use crate::core::session::ToolActivationCache;
|
||||
use crate::models::Tool;
|
||||
@@ -272,12 +272,24 @@ fn compact_surface_keeps_the_exact_eager_agent_head() {
|
||||
);
|
||||
}
|
||||
|
||||
/// The per-tool Registry paragraph is gone; the catalog builder must leave the
|
||||
/// shell tool's description exactly as the registry produced it, so the KV
|
||||
/// prefix stays byte-stable and there is one Registry authority (the prompt).
|
||||
#[test]
|
||||
fn registry_first_guidance_does_not_expand_contract_bash_schema_text() {
|
||||
let mut catalog = vec![tool("bash")];
|
||||
let description = catalog[0].description.clone();
|
||||
fn catalog_build_does_not_append_registry_guidance_to_the_shell_tool() {
|
||||
let described = tool("bash");
|
||||
let catalog = build_model_tool_catalog_with_surface(
|
||||
vec![described.clone()],
|
||||
Vec::new(),
|
||||
AppMode::Agent,
|
||||
&HashSet::new(),
|
||||
crate::model_profile::ToolSurfaceBudget::Standard,
|
||||
);
|
||||
|
||||
apply_registry_first_shell_guidance(&mut catalog);
|
||||
|
||||
assert_eq!(catalog[0].description, description);
|
||||
let shell = catalog
|
||||
.iter()
|
||||
.find(|definition| definition.name == "bash")
|
||||
.expect("shell tool");
|
||||
assert_eq!(shell.description, described.description);
|
||||
assert!(!shell.description.contains("registry_sync"));
|
||||
}
|
||||
@@ -1469,6 +1469,8 @@ impl Engine {
|
||||
.active_route_limits
|
||||
.and_then(|limits| limits.context_tokens);
|
||||
|
||||
turn.stop_diagnostics.last_prepared_output_limit_tokens = Some(request.max_tokens);
|
||||
|
||||
// Stream the response. Keep the request around (cloned into the
|
||||
// first call) so we can resend it on a transparent retry below
|
||||
// when the wire dies before any content was streamed (#103).
|
||||
@@ -1611,6 +1613,22 @@ impl Engine {
|
||||
turn_error = turn_error.or(stream_error);
|
||||
turn.stop_diagnostics
|
||||
.observe_provider_response(stop_reason.as_deref(), tool_uses.len());
|
||||
// Counts and terminal metadata only: never log messages, tool
|
||||
// arguments, credentials, or raw provider bodies.
|
||||
tracing::debug!(
|
||||
target: "provider_response_diagnostics",
|
||||
model_request = turn.stop_diagnostics.model_requests_started,
|
||||
prepared_output_limit_tokens = stream_request.max_tokens,
|
||||
finish_reason = ?turn.stop_diagnostics.last_provider_finish_reason,
|
||||
reported_usage = usage_reported,
|
||||
input_tokens = usage.input_tokens,
|
||||
output_tokens = usage.output_tokens,
|
||||
cached_input_tokens = ?usage.prompt_cache_hit_tokens,
|
||||
reasoning_tokens = ?usage.reasoning_tokens,
|
||||
decoded_tool_calls = tool_uses.len(),
|
||||
visible_text_chars = current_text_visible.chars().count(),
|
||||
"parent model response settled"
|
||||
);
|
||||
// These belong to post-stream response assembly, not stream
|
||||
// consumption: blocks are built from the completed stream state,
|
||||
// and truncation is derived from its terminal stop reason below.
|
||||
@@ -2577,9 +2595,23 @@ impl Engine {
|
||||
false,
|
||||
)
|
||||
{
|
||||
let message = if has_provider_reasoning {
|
||||
"Model returned reasoning but no answer or tool call; the provider response was incomplete."
|
||||
.to_string()
|
||||
let message = if has_provider_reasoning
|
||||
&& stop_reason_is_output_limit(stop_reason.as_deref())
|
||||
{
|
||||
format!(
|
||||
"Model reached the response output limit with no answer or tool call (requested allowance: {} tokens, including reasoning).",
|
||||
stream_request.max_tokens
|
||||
)
|
||||
} else if has_provider_reasoning {
|
||||
let reason = crate::models::stop_reason_detail(stop_reason.as_deref());
|
||||
format!(
|
||||
"Model returned reasoning but no answer or tool call; the provider response was incomplete (stop reason: {}).",
|
||||
reason
|
||||
.chars()
|
||||
.flat_map(char::escape_default)
|
||||
.take(120)
|
||||
.collect::<String>()
|
||||
)
|
||||
} else if let Some(reason) = stop_reason.as_deref() {
|
||||
format!(
|
||||
"Model returned terminal stop reason `{reason}` with no answer or tool call."
|
||||
|
||||
@@ -18363,9 +18363,13 @@ mod project_config_tests {
|
||||
merge_project_config(&mut explicit, workspace.path());
|
||||
assert_eq!(explicit.default_model(), "gpt-5.6-terra", "{key}");
|
||||
assert_eq!(
|
||||
crate::route_runtime::resolve_runtime_route(&explicit, ApiProvider::Openai, None)
|
||||
.unwrap()
|
||||
.model,
|
||||
crate::route_runtime::resolve_runtime_route(
|
||||
&explicit,
|
||||
crate::config::ApiProvider::Openai,
|
||||
None
|
||||
)
|
||||
.unwrap()
|
||||
.model,
|
||||
"gpt-5.6-terra",
|
||||
"{key}"
|
||||
);
|
||||
|
||||
@@ -3239,6 +3239,11 @@ async fn relay_worker(
|
||||
runtime_upload_tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
||||
let mut runtime_chat_tick = tokio::time::interval(RUNTIME_UPLOAD_RETRY_INTERVAL);
|
||||
runtime_chat_tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
||||
// One deadline across loop iterations. Reconstructing `sleep(SYNC_INTERVAL)`
|
||||
// lets the 250ms Runtime Chat tick reset poll/heartbeat/token refresh.
|
||||
let mut sync_tick =
|
||||
tokio::time::interval_at(tokio::time::Instant::now() + SYNC_INTERVAL, SYNC_INTERVAL);
|
||||
sync_tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip);
|
||||
let mut runtime_retry_delay = RUNTIME_UPLOAD_RETRY_INTERVAL;
|
||||
let mut runtime_retry_not_before = Instant::now();
|
||||
|
||||
@@ -3389,7 +3394,7 @@ async fn relay_worker(
|
||||
.map_err(|_| "The terminal remote-control owner stopped.".to_string())?;
|
||||
}
|
||||
}
|
||||
() = tokio::time::sleep(SYNC_INTERVAL) => {
|
||||
_ = sync_tick.tick() => {
|
||||
if enrollment_needs_refresh(&enrollment) {
|
||||
// Proactive refresh before expiry; reconnect to keep runner lease valid.
|
||||
match refresh_enrollment(&client, enrollment.persisted.clone()).await {
|
||||
@@ -5000,7 +5005,10 @@ fn epoch_seconds() -> u64 {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::models::Role;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::sync::{
|
||||
Arc, Mutex,
|
||||
atomic::{AtomicUsize, Ordering},
|
||||
};
|
||||
use wiremock::{
|
||||
Mock, MockServer, Request, Respond, ResponseTemplate,
|
||||
matchers::{body_json, method, path, query_param},
|
||||
@@ -5011,6 +5019,18 @@ mod tests {
|
||||
bodies: Arc<Mutex<Vec<Value>>>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
struct ControlPollCounter {
|
||||
hits: Arc<AtomicUsize>,
|
||||
}
|
||||
|
||||
impl Respond for ControlPollCounter {
|
||||
fn respond(&self, _request: &Request) -> ResponseTemplate {
|
||||
self.hits.fetch_add(1, Ordering::SeqCst);
|
||||
ResponseTemplate::new(200).set_body_json(json!({ "runs": [] }))
|
||||
}
|
||||
}
|
||||
|
||||
impl Respond for AmbiguousRuntimeResponder {
|
||||
fn respond(&self, request: &Request) -> ResponseTemplate {
|
||||
let body: Value = serde_json::from_slice(&request.body).expect("runtime request JSON");
|
||||
@@ -5731,6 +5751,108 @@ mod tests {
|
||||
assert!(!host.projection_is_claimed_for_tests("thr_claimed", 7));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn idle_runtime_chat_ticks_cannot_starve_the_control_poll() {
|
||||
if !cfg!(debug_assertions) {
|
||||
return;
|
||||
}
|
||||
crate::tls::ensure_rustls_crypto_provider();
|
||||
let _env = crate::test_support::lock_test_env();
|
||||
let secrets_root = tempfile::tempdir().expect("isolated remote-control secrets");
|
||||
let _codewhale_home =
|
||||
crate::test_support::EnvVarGuard::set("CODEWHALE_HOME", secrets_root.path());
|
||||
let server = MockServer::start().await;
|
||||
let plane = format!("{}/", server.uri().trim_end_matches('/'));
|
||||
let _control_plane =
|
||||
crate::test_support::EnvVarGuard::set("CWC_RUNNER_CONTROL_PLANE_BASE", &plane);
|
||||
let base = runner_control_plane_base().expect("loopback control plane");
|
||||
save_persisted_enrollment(&fixture_enrollment(&base).persisted)
|
||||
.expect("persist matching enrollment");
|
||||
|
||||
let access_token = crate::test_support::future_test_jwt(&"a".repeat(40));
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/runner/enrollments/token"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"enrollment": {
|
||||
"id": "enrollment_fixture",
|
||||
"userId": "account_fixture",
|
||||
"deviceId": "device_fixture",
|
||||
"runtimeVersion": "0.9.6",
|
||||
"runtimeCommit": "a".repeat(40),
|
||||
"capabilities": CAPABILITIES,
|
||||
},
|
||||
"credential": { "accessToken": access_token },
|
||||
})))
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/local-runners/connect"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(fixture_connection_response()))
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/local-runners/runner_fixture/heartbeat"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({})))
|
||||
.mount(&server)
|
||||
.await;
|
||||
let polls = ControlPollCounter::default();
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/api/local-runners/runner_fixture/runs"))
|
||||
.respond_with(polls.clone())
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let runtime_root = tempfile::tempdir().expect("idle Runtime Chat host");
|
||||
let host = RuntimeChatRelayHost::open(
|
||||
crate::config::Config::default(),
|
||||
Arc::new(crate::plugins::PluginRegistry::empty(runtime_root.path())),
|
||||
runtime_root.path().to_path_buf(),
|
||||
"target_fixture".to_string(),
|
||||
"session_fixture".to_string(),
|
||||
)
|
||||
.expect("open idle Runtime Chat host");
|
||||
let start = fixture_start();
|
||||
let (_worker_tx, worker_rx) = mpsc::unbounded_channel();
|
||||
let (event_tx, mut event_rx) = mpsc::unbounded_channel();
|
||||
let worker = tokio::spawn(async move {
|
||||
let mut phase = RelayPhase::Enrolling;
|
||||
relay_worker(start, Some(host), worker_rx, event_tx, &mut phase).await
|
||||
});
|
||||
|
||||
tokio::time::timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
match event_rx.recv().await {
|
||||
Some(RemoteEvent::Connected { .. }) => break,
|
||||
Some(RemoteEvent::Notice(_)) => {}
|
||||
Some(RemoteEvent::Failed(error)) => {
|
||||
panic!("relay worker failed before attach: {error}");
|
||||
}
|
||||
Some(RemoteEvent::FailedPreLease(error)) => {
|
||||
panic!("relay worker failed before attach: {error}");
|
||||
}
|
||||
Some(_) => panic!("relay worker emitted an unexpected event before attach"),
|
||||
None => panic!("relay worker stopped before attach"),
|
||||
}
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("relay worker should attach");
|
||||
|
||||
tokio::time::sleep(RUNTIME_UPLOAD_RETRY_INTERVAL.saturating_mul(2)).await;
|
||||
assert_eq!(
|
||||
polls.hits.load(Ordering::SeqCst),
|
||||
0,
|
||||
"the first control poll must still wait SYNC_INTERVAL while idle chat ticks fire"
|
||||
);
|
||||
tokio::time::sleep(SYNC_INTERVAL).await;
|
||||
assert!(
|
||||
polls.hits.load(Ordering::SeqCst) >= 1,
|
||||
"idle Runtime Chat ticks must not reset the control poll"
|
||||
);
|
||||
worker.abort();
|
||||
let _ = worker.await;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preattachment_failure_cannot_release_a_recovered_unsettled_turn() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Durable CLI route edits use the same Config owner as Runtime and the TUI.
|
||||
//! `model` and the legacy `default_text_model` address the saved active route;
|
||||
//! `default_model` addresses DeepSeek. Project root keys retain their scope.
|
||||
//! `default_model` addresses DeepSeek (CN while that route is active). Project root keys retain their scope.
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
@@ -30,7 +30,12 @@ fn parse_config(body: &str) -> Result<Config> {
|
||||
|
||||
fn model_identity(config: &Config, key: &str) -> Result<ProviderIdentity> {
|
||||
if key == "default_model" {
|
||||
config.resolve_provider_pin_identity("deepseek")
|
||||
let provider = if config.api_provider() == ApiProvider::DeepseekCN {
|
||||
ApiProvider::DeepseekCN
|
||||
} else {
|
||||
ApiProvider::Deepseek
|
||||
};
|
||||
config.resolve_provider_pin_identity(provider.as_str())
|
||||
} else if let Some(id) = provider_model_id(key) {
|
||||
// Leaf keys name TOML tables, whose canonical spelling can differ
|
||||
// from the public provider selector. Exact custom tables still win.
|
||||
@@ -92,6 +97,165 @@ pub fn model_slot_for_document(body: &str, key: &str) -> Result<Vec<String>> {
|
||||
.collect())
|
||||
}
|
||||
|
||||
fn document_slot_value(document: &toml::value::Table, slot: &[String]) -> Option<String> {
|
||||
let [root, provider, field] = slot else {
|
||||
return None;
|
||||
};
|
||||
document
|
||||
.get(root)?
|
||||
.as_table()?
|
||||
.get(provider)?
|
||||
.as_table()?
|
||||
.get(field)?
|
||||
.as_str()
|
||||
.map(str::trim)
|
||||
.filter(|model| !model.is_empty())
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
fn insert_document_slot_value(
|
||||
document: &mut toml::value::Table,
|
||||
slot: &[String],
|
||||
value: &str,
|
||||
) -> bool {
|
||||
let [root, provider, field] = slot else {
|
||||
return false;
|
||||
};
|
||||
let Some(providers) = document
|
||||
.entry(root.clone())
|
||||
.or_insert_with(|| toml::Value::Table(toml::value::Table::new()))
|
||||
.as_table_mut()
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
let Some(entry) = providers
|
||||
.entry(provider.clone())
|
||||
.or_insert_with(|| toml::Value::Table(toml::value::Table::new()))
|
||||
.as_table_mut()
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
entry.insert(field.clone(), toml::Value::String(value.to_string()));
|
||||
true
|
||||
}
|
||||
|
||||
/// Scrub root model aliases from a serialized config document for export.
|
||||
///
|
||||
/// Root `model`/`default_text_model` address the *active* route on import and
|
||||
/// `default_model` addresses DeepSeek (CN when that route is active), so a raw root alias exported next to
|
||||
/// the canonical `[providers.<id>].model` leaf fails the importer's replay
|
||||
/// check. This rewrites `document` in place:
|
||||
///
|
||||
/// - A root alias the active route still consumes is shadowed state once the
|
||||
/// route's canonical leaf is exported; it is dropped.
|
||||
/// - A root alias the active route ignores but DeepSeek recognizes is
|
||||
/// DeepSeek's saved fallback; it moves to `providers.deepseek.model` unless
|
||||
/// that slot already carries a value. Any other unrecognized value is dead
|
||||
/// state that would only conflict on import and is dropped.
|
||||
/// - `default_model` folds into the selected DeepSeek region's model slot when
|
||||
/// nothing live occupies that slot, and is dropped otherwise.
|
||||
///
|
||||
/// Only route-selection keys are parsed as `Config` here. Export documents
|
||||
/// intentionally preserve unknown or differently typed local-authority
|
||||
/// extras, and reparsing them merely to locate the model slot would fail the
|
||||
/// whole export.
|
||||
pub fn scrub_root_model_aliases_for_export(document: &mut toml::value::Table) -> Result<()> {
|
||||
let mut scratch = toml::value::Table::new();
|
||||
for key in [
|
||||
"provider",
|
||||
"model",
|
||||
"default_text_model",
|
||||
"defaultTextModel",
|
||||
"base_url",
|
||||
"baseUrl",
|
||||
"providers",
|
||||
] {
|
||||
if let Some(value) = document.get(key) {
|
||||
scratch.insert(key.to_string(), value.clone());
|
||||
}
|
||||
}
|
||||
let body = toml::to_string(&toml::Value::Table(scratch))
|
||||
.context("serializing route selection for export")?;
|
||||
// Slot resolution reuses the exact document identity rules; the extra
|
||||
// parse below only exists so the ownership test can scope a Config clone.
|
||||
let active_slot = model_slot_for_document(&body, "model")?;
|
||||
let deepseek_slot = model_slot_for_document(&body, "default_model")?;
|
||||
let config = parse_config(&body)?;
|
||||
let identity = model_identity(&config, "model")?;
|
||||
|
||||
if document_slot_value(document, &active_slot).is_some() {
|
||||
for root_key in ["model", "default_text_model"] {
|
||||
let Some(value) = document
|
||||
.get(root_key)
|
||||
.and_then(toml::Value::as_str)
|
||||
.map(str::to_owned)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
// Same ownership test as `unset`: scope to the active route with
|
||||
// its canonical leaf cleared and ask whether this root value is
|
||||
// what the route would then resolve. A foreign DeepSeek root
|
||||
// ignored by the active vendor remains DeepSeek's fallback.
|
||||
let mut scoped = config.clone();
|
||||
scoped.scope_to_provider_identity(&identity);
|
||||
scoped.set_provider_model_override(identity.provider, None);
|
||||
scoped.legacy_model = None;
|
||||
scoped.default_text_model = Some(value.to_string());
|
||||
let wire_model = crate::config::wire_model_for_provider_route(
|
||||
identity.provider,
|
||||
&scoped.deepseek_base_url(),
|
||||
&value,
|
||||
);
|
||||
if scoped.default_model() == wire_model {
|
||||
document.remove(root_key);
|
||||
continue;
|
||||
}
|
||||
if crate::config::normalize_model_name(&value).is_none() {
|
||||
document.remove(root_key);
|
||||
continue;
|
||||
}
|
||||
if document_slot_value(document, &deepseek_slot).is_some() {
|
||||
document.remove(root_key);
|
||||
continue;
|
||||
}
|
||||
ensure!(
|
||||
insert_document_slot_value(document, &deepseek_slot, &value),
|
||||
"Cannot export a root model alias into a non-table provider slot"
|
||||
);
|
||||
document.remove(root_key);
|
||||
}
|
||||
}
|
||||
|
||||
match document.get("default_model").and_then(toml::Value::as_str) {
|
||||
Some(value) => {
|
||||
let value = value.trim().to_string();
|
||||
// A root alias the DeepSeek route still consumes lands in this
|
||||
// same slot on import; the live choice wins over the dead alias.
|
||||
let root_covers_slot = active_slot == deepseek_slot
|
||||
&& ["model", "default_text_model"]
|
||||
.iter()
|
||||
.any(|key| document.get(*key).and_then(toml::Value::as_str).is_some());
|
||||
let folded = !value.is_empty()
|
||||
&& !root_covers_slot
|
||||
&& document_slot_value(document, &deepseek_slot).is_none();
|
||||
if folded {
|
||||
ensure!(
|
||||
insert_document_slot_value(document, &deepseek_slot, &value),
|
||||
"Cannot export default_model into a non-table provider slot"
|
||||
);
|
||||
}
|
||||
document.remove("default_model");
|
||||
}
|
||||
None => {
|
||||
ensure!(
|
||||
!document.contains_key("default_model"),
|
||||
"Cannot export a non-string default_model without losing its value"
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn project_root_key<'a>(path: &Path, key: &'a str) -> Option<&'a str> {
|
||||
(codewhale_config::config_path_is_workspace_scoped(path)
|
||||
&& matches!(key, "model" | "default_text_model"))
|
||||
@@ -295,6 +459,9 @@ model = "Other-X"
|
||||
std::fs::write(home.join("settings.toml"), "default_provider = \"zai\"\n")?;
|
||||
let project = root.path().join("project/.codewhale");
|
||||
std::fs::create_dir_all(&project)?;
|
||||
// An absolute config outside the process workspace is project-scoped
|
||||
// only when its parent is a checkout, not merely named `.codewhale`.
|
||||
std::fs::create_dir(root.path().join("project/.git"))?;
|
||||
let path = project.join("config.toml");
|
||||
std::fs::write(&path, "model = \"project-old\"\n")?;
|
||||
set(&path, "model", "project-new")?;
|
||||
|
||||
+114
-12
@@ -52,9 +52,9 @@ use crate::automation_manager::{
|
||||
AutomationManager, AutomationRecord, AutomationRunRecord, AutomationSchedulerConfig,
|
||||
CreateAutomationRequest, SharedAutomationManager, UpdateAutomationRequest, spawn_scheduler,
|
||||
};
|
||||
use crate::config::{
|
||||
ApiProvider, Config, DEFAULT_TEXT_MODEL, normalize_model_name_for_provider, validate_route,
|
||||
};
|
||||
#[cfg(test)]
|
||||
use crate::config::DEFAULT_TEXT_MODEL;
|
||||
use crate::config::{ApiProvider, Config, normalize_model_name_for_provider, validate_route};
|
||||
use crate::fleet::executor::{FleetExecutor, configured_codewhale_binary};
|
||||
use crate::fleet::ledger::{FleetEventReplayError, FleetLedgerState, FleetTaskLedgerStatus};
|
||||
use crate::fleet::manager::{
|
||||
@@ -5701,6 +5701,7 @@ fn map_compat_stream_event(event: &crate::runtime_threads::RuntimeEventRecord) -
|
||||
json!({
|
||||
"id": approval_id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": payload.get("tool_call_id"),
|
||||
"thread_id": event.thread_id,
|
||||
"turn_id": event.turn_id,
|
||||
"tool_name": payload.get("tool_name"),
|
||||
@@ -5719,6 +5720,7 @@ fn map_compat_stream_event(event: &crate::runtime_threads::RuntimeEventRecord) -
|
||||
json!({
|
||||
"id": approval_id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": payload.get("tool_call_id"),
|
||||
"thread_id": event.thread_id,
|
||||
"turn_id": event.turn_id,
|
||||
"decision": payload.get("decision"),
|
||||
@@ -5738,6 +5740,7 @@ fn map_compat_stream_event(event: &crate::runtime_threads::RuntimeEventRecord) -
|
||||
json!({
|
||||
"id": approval_id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": payload.get("tool_call_id"),
|
||||
"thread_id": event.thread_id,
|
||||
"turn_id": event.turn_id,
|
||||
"timeout_secs": payload.get("timeout_secs"),
|
||||
@@ -6995,9 +6998,16 @@ async fn get_config(
|
||||
let cost_currency = settings.cost_currency.clone();
|
||||
let default_mode = settings.default_mode.as_str().to_string();
|
||||
// This field remains the DeepSeek preference even when another provider
|
||||
// is active. The root field is a legacy fallback for unmigrated configs.
|
||||
// is active, and follows the CN slot while the CN route is active — the CN
|
||||
// route resolves `[providers.deepseek_cn].model`, not the primary slot.
|
||||
// The root field is a legacy fallback for unmigrated configs.
|
||||
let default_provider = if config.api_provider() == ApiProvider::DeepseekCN {
|
||||
ApiProvider::DeepseekCN
|
||||
} else {
|
||||
ApiProvider::Deepseek
|
||||
};
|
||||
let identity = config
|
||||
.resolve_provider_pin_identity("deepseek")
|
||||
.resolve_provider_pin_identity(default_provider.as_str())
|
||||
.map_err(ApiError::bad_request)?;
|
||||
let mut deepseek_config = config.clone();
|
||||
deepseek_config.scope_to_provider_identity(&identity);
|
||||
@@ -7102,7 +7112,12 @@ async fn set_config(
|
||||
value = normalize_runtime_config_model(&config, provider, &value)?;
|
||||
}
|
||||
"default_model" => {
|
||||
value = normalize_runtime_config_model(&config, ApiProvider::Deepseek, &value)?;
|
||||
let default_provider = if provider == ApiProvider::DeepseekCN {
|
||||
ApiProvider::DeepseekCN
|
||||
} else {
|
||||
ApiProvider::Deepseek
|
||||
};
|
||||
value = normalize_runtime_config_model(&config, default_provider, &value)?;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
@@ -7134,12 +7149,22 @@ async fn set_config(
|
||||
&active_route.1,
|
||||
&value,
|
||||
),
|
||||
"default_model" => config_persistence::persist_provider_model_key(
|
||||
config_path,
|
||||
ApiProvider::Deepseek,
|
||||
ApiProvider::Deepseek.as_str(),
|
||||
&value,
|
||||
),
|
||||
"default_model" => {
|
||||
// The CN route reads its own `[providers.deepseek_cn]` slot;
|
||||
// writing the primary `deepseek` slot there would be unread.
|
||||
let (default_provider, default_identity) =
|
||||
if active_route.0 == ApiProvider::DeepseekCN {
|
||||
(ApiProvider::DeepseekCN, ApiProvider::DeepseekCN.as_str())
|
||||
} else {
|
||||
(ApiProvider::Deepseek, ApiProvider::Deepseek.as_str())
|
||||
};
|
||||
config_persistence::persist_provider_model_key(
|
||||
config_path,
|
||||
default_provider,
|
||||
default_identity,
|
||||
&value,
|
||||
)
|
||||
}
|
||||
"reasoning_effort" => {
|
||||
config_persistence::persist_root_string_key(config_path, "reasoning_effort", &value)
|
||||
}
|
||||
@@ -8485,4 +8510,81 @@ model = "GLM-5.2"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn runtime_default_model_reads_and_writes_the_deepseek_cn_slot() -> Result<()> {
|
||||
let _env = lock_test_env();
|
||||
let _live = crate::provider_lake::lock_live_snapshot();
|
||||
let root = tempfile::tempdir()?;
|
||||
let _home = EnvVarGuard::set("CODEWHALE_HOME", root.path());
|
||||
let _model_environment = isolate_model_environment();
|
||||
crate::provider_catalog_live::reset_cache_for_test();
|
||||
crate::provider_lake::clear_live_snapshot();
|
||||
let config_path = root.path().join("config.toml");
|
||||
fs::write(
|
||||
&config_path,
|
||||
"provider = 'deepseek-cn'\ntelemetry = false\n[cloud_facts]\nenabled = false\n[providers.deepseek_cn]\nmodel = 'deepseek-v4-pro'\n",
|
||||
)?;
|
||||
let (addr, state, server) = serve_fixture(config_path.clone()).await?;
|
||||
let _shutdown = state.task_manager.shutdown_guard();
|
||||
let client = crate::tls::reqwest_client();
|
||||
// The active CN route resolves `[providers.deepseek_cn].model`; the
|
||||
// runtime default_model surface must report that slot, not the primary.
|
||||
let reported: Value = client
|
||||
.get(format!("http://{addr}/v1/config"))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json()
|
||||
.await?;
|
||||
assert_eq!(reported["default_model"], "deepseek-v4-pro");
|
||||
|
||||
post_json(
|
||||
addr,
|
||||
"/v1/config",
|
||||
json!({ "key": "default_model", "value": "deepseek-v4-flash", "persist": true }),
|
||||
)
|
||||
.await?;
|
||||
let saved: toml::Value = toml::from_str(&fs::read_to_string(&config_path)?)?;
|
||||
assert_eq!(
|
||||
saved["providers"]["deepseek_cn"]["model"].as_str(),
|
||||
Some("deepseek-v4-flash")
|
||||
);
|
||||
assert!(
|
||||
saved
|
||||
.get("providers")
|
||||
.and_then(|providers| providers.get("deepseek"))
|
||||
.and_then(|deepseek| deepseek.get("model"))
|
||||
.is_none(),
|
||||
"the CN write must not create an unread primary slot: {saved}"
|
||||
);
|
||||
|
||||
post_json(addr, "/v1/config/reload", json!({})).await?;
|
||||
let reported: Value = client
|
||||
.get(format!("http://{addr}/v1/config"))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json()
|
||||
.await?;
|
||||
assert_eq!(reported["default_model"], "deepseek-v4-flash");
|
||||
post_json(
|
||||
addr,
|
||||
"/v1/config",
|
||||
json!({ "key": "default_model", "value": "auto", "persist": true }),
|
||||
)
|
||||
.await?;
|
||||
post_json(addr, "/v1/config/reload", json!({})).await?;
|
||||
let reported: Value = client
|
||||
.get(format!("http://{addr}/v1/config"))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json()
|
||||
.await?;
|
||||
assert_eq!(reported["default_model"], "auto");
|
||||
server.abort();
|
||||
state.task_manager.shutdown_and_wait().await?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -4399,6 +4399,105 @@ async fn stream_compat_mapping_handles_expected_runtime_events() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Renders one mapped SSE frame and returns its `data:` payload as JSON.
|
||||
async fn sse_frame_payload(event: SseEvent) -> Result<Value> {
|
||||
let stream = async_stream::stream! {
|
||||
yield Ok::<_, Infallible>(event);
|
||||
};
|
||||
let body =
|
||||
axum::body::to_bytes(Sse::new(stream).into_response().into_body(), usize::MAX).await?;
|
||||
let text = String::from_utf8_lossy(&body).into_owned();
|
||||
let data = text
|
||||
.lines()
|
||||
.find_map(|line| line.strip_prefix("data: "))
|
||||
.context("SSE frame had no data line")?;
|
||||
Ok(serde_json::from_str(data)?)
|
||||
}
|
||||
|
||||
fn approval_event_record(seq: u64, event: &str, payload: Value) -> RuntimeEventRecord {
|
||||
RuntimeEventRecord {
|
||||
schema_version: 1,
|
||||
seq,
|
||||
timestamp: chrono::Utc::now(),
|
||||
thread_id: "thr_test".to_string(),
|
||||
turn_id: Some("turn_test".to_string()),
|
||||
item_id: None,
|
||||
event: event.to_string(),
|
||||
payload,
|
||||
}
|
||||
}
|
||||
|
||||
/// Every approval frame an SSE-only client sees identifies the approval by the
|
||||
/// runtime-minted capability. A provider tool-call ID must never surface in
|
||||
/// `approval_id` or its legacy `id` alias, because those are the fields clients
|
||||
/// echo back to `POST /v1/approvals/{id}`.
|
||||
#[tokio::test]
|
||||
async fn approval_sse_frames_identify_approvals_by_the_opaque_capability() -> Result<()> {
|
||||
const OPAQUE: &str = "approval_0123456789abcdef";
|
||||
const RAW: &str = "call_1";
|
||||
|
||||
// Each arm reads only the identity fields; the rest are the shape a real
|
||||
// payload carries and are ignored by the arms that do not use them.
|
||||
for (seq, event) in ["approval.required", "approval.decided", "approval.timeout"]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let record = approval_event_record(
|
||||
seq as u64 + 1,
|
||||
event,
|
||||
json!({
|
||||
"id": OPAQUE,
|
||||
"approval_id": OPAQUE,
|
||||
"tool_call_id": RAW,
|
||||
"tool_name": "exec_command",
|
||||
"description": "Run tests",
|
||||
"decision": "allow",
|
||||
"timeout_secs": 300,
|
||||
}),
|
||||
);
|
||||
let mapped =
|
||||
map_compat_stream_event(&record).with_context(|| format!("missing {event} frame"))?;
|
||||
let frame = sse_frame_payload(mapped).await?;
|
||||
assert_eq!(frame["approval_id"], OPAQUE, "{event} approval_id");
|
||||
assert_eq!(frame["id"], OPAQUE, "{event} legacy id alias");
|
||||
assert_ne!(frame["approval_id"], RAW, "{event} must not publish raw id");
|
||||
assert_eq!(frame["thread_id"], "thr_test");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Companion to the test above: the raw tool-call ID is the only correlator an
|
||||
/// SSE-only client can use to attach an approval prompt to the tool row it
|
||||
/// gates, so the projection should forward it alongside the capability.
|
||||
#[tokio::test]
|
||||
async fn approval_sse_frames_forward_the_tool_call_correlator() -> Result<()> {
|
||||
const OPAQUE: &str = "approval_0123456789abcdef";
|
||||
const RAW: &str = "call_1";
|
||||
|
||||
for (seq, event) in ["approval.required", "approval.decided", "approval.timeout"]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let record = approval_event_record(
|
||||
seq as u64 + 1,
|
||||
event,
|
||||
json!({
|
||||
"id": OPAQUE,
|
||||
"approval_id": OPAQUE,
|
||||
"tool_call_id": RAW,
|
||||
"tool_name": "exec_command",
|
||||
"decision": "allow",
|
||||
}),
|
||||
);
|
||||
let mapped =
|
||||
map_compat_stream_event(&record).with_context(|| format!("missing {event} frame"))?;
|
||||
let frame = sse_frame_payload(mapped).await?;
|
||||
assert_eq!(frame["tool_call_id"], RAW, "{event} tool_call_id");
|
||||
assert_eq!(frame["approval_id"], OPAQUE, "{event} approval_id");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stream_endpoint_remains_backward_compatible() -> Result<()> {
|
||||
let Some((addr, runtime_threads, handle)) = spawn_test_server().await? else {
|
||||
@@ -7338,16 +7437,28 @@ async fn decide_approval_delivers_to_runtime() -> Result<()> {
|
||||
return Ok(());
|
||||
};
|
||||
let client = crate::tls::reqwest_client();
|
||||
let rx = runtime_threads.register_pending_approval_for_test("ext_id");
|
||||
// The caller never chooses the approval ID; the runtime mints it and the
|
||||
// client echoes it back verbatim.
|
||||
let (approval_id, rx) = runtime_threads.register_pending_approval_for_test("ext_id");
|
||||
assert_ne!(approval_id, "ext_id");
|
||||
|
||||
// A provider-shaped raw ID is not accepted as a fallback for the real one.
|
||||
let raw = client
|
||||
.post(format!("http://{addr}/v1/approvals/ext_id"))
|
||||
.json(&json!({ "decision": "allow", "remember": false }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(raw.status(), StatusCode::NOT_FOUND);
|
||||
|
||||
let resp = client
|
||||
.post(format!("http://{addr}/v1/approvals/ext_id"))
|
||||
.post(format!("http://{addr}/v1/approvals/{approval_id}"))
|
||||
.json(&json!({ "decision": "allow", "remember": false }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
let body: serde_json::Value = resp.json().await?;
|
||||
assert_eq!(body["ok"], true);
|
||||
assert_eq!(body["approval_id"], approval_id);
|
||||
assert_eq!(body["decision"], "allow");
|
||||
assert_eq!(body["delivered"], true);
|
||||
|
||||
@@ -7357,6 +7468,74 @@ async fn decide_approval_delivers_to_runtime() -> Result<()> {
|
||||
ExternalApprovalDecision::Allow { remember: false }
|
||||
);
|
||||
|
||||
// The capability is single-use: a duplicate response (retry, double-click,
|
||||
// replayed request) cannot deliver a second decision.
|
||||
let replay = client
|
||||
.post(format!("http://{addr}/v1/approvals/{approval_id}"))
|
||||
.json(&json!({ "decision": "deny", "remember": true }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(replay.status(), StatusCode::NOT_FOUND);
|
||||
|
||||
handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The snapshot a reconnecting client resumes from must carry both halves of
|
||||
/// the approval identity over the wire: the opaque capability it posts back,
|
||||
/// and the raw tool-call correlator it matches against the tool row. Only the
|
||||
/// first is accepted by `POST /v1/approvals/{id}`.
|
||||
#[tokio::test]
|
||||
async fn thread_snapshot_separates_approval_capability_from_tool_correlator() -> Result<()> {
|
||||
let Some((addr, runtime_threads, handle)) = spawn_test_server().await? else {
|
||||
return Ok(());
|
||||
};
|
||||
let client = crate::tls::reqwest_client();
|
||||
let thread: serde_json::Value = client
|
||||
.post(format!("http://{addr}/v1/threads"))
|
||||
.json(&json!({}))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json()
|
||||
.await?;
|
||||
let thread_id = thread["id"].as_str().context("missing thread id")?;
|
||||
|
||||
let (approval_id, rx) =
|
||||
runtime_threads.register_pending_approval_for_thread_for_test(thread_id, "call_1");
|
||||
|
||||
let detail: serde_json::Value = client
|
||||
.get(format!("http://{addr}/v1/threads/{thread_id}"))
|
||||
.send()
|
||||
.await?
|
||||
.error_for_status()?
|
||||
.json()
|
||||
.await?;
|
||||
let pending = detail["pending_approvals"]
|
||||
.as_array()
|
||||
.context("missing pending_approvals")?;
|
||||
assert_eq!(pending.len(), 1);
|
||||
assert_eq!(pending[0]["id"], approval_id);
|
||||
assert_eq!(pending[0]["tool_call_id"], "call_1");
|
||||
|
||||
// A provider call ID is a correlator, not a credential — even now that a
|
||||
// client can read it straight out of the snapshot.
|
||||
let by_correlator = client
|
||||
.post(format!("http://{addr}/v1/approvals/call_1"))
|
||||
.json(&json!({ "decision": "allow", "remember": false }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(by_correlator.status(), StatusCode::NOT_FOUND);
|
||||
|
||||
let by_capability = client
|
||||
.post(format!("http://{addr}/v1/approvals/{approval_id}"))
|
||||
.json(&json!({ "decision": "deny", "remember": false }))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(by_capability.status(), StatusCode::OK);
|
||||
let received = tokio::time::timeout(ci_scaled(Duration::from_secs(1)), rx).await??;
|
||||
assert_eq!(received, ExternalApprovalDecision::Deny { remember: false });
|
||||
|
||||
handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
@@ -12975,7 +13154,7 @@ async fn native_notification_preparation_authenticates_current_events_and_reuses
|
||||
// request is actionable, even if a stale pending projection survived.
|
||||
turn.status = crate::runtime_threads::RuntimeTurnStatus::InProgress;
|
||||
manager.test_store().save_turn(&turn)?;
|
||||
let _approval =
|
||||
let (approval_id, _approval) =
|
||||
manager.register_pending_approval_for_thread_for_test(&thread.id, "approval-current");
|
||||
manager.register_pending_user_input_for_thread_for_test(&thread.id, "input-current");
|
||||
let approval = manager
|
||||
@@ -12983,7 +13162,7 @@ async fn native_notification_preparation_authenticates_current_events_and_reuses
|
||||
&thread.id,
|
||||
Some(&turn.id),
|
||||
"approval.required",
|
||||
json!({"id":"approval-current","description":"private approval prompt"}),
|
||||
json!({"id": approval_id, "description":"private approval prompt"}),
|
||||
)
|
||||
.await?;
|
||||
let input = manager
|
||||
@@ -13100,12 +13279,18 @@ async fn native_notification_replay_rechecks_requests_settled_during_the_read()
|
||||
manager.register_pending_user_input_for_thread_for_test(&thread.id, "request");
|
||||
None
|
||||
};
|
||||
// The event has to name the same identity the pending projection holds,
|
||||
// which for an approval is the runtime-minted one.
|
||||
let request_id = approval
|
||||
.as_ref()
|
||||
.map(|(approval_id, _)| approval_id.clone())
|
||||
.unwrap_or_else(|| "request".to_string());
|
||||
let event = manager
|
||||
.emit_event_for_test(
|
||||
&thread.id,
|
||||
Some(&turn.id),
|
||||
event_name,
|
||||
json!({"id": "request"}),
|
||||
json!({"id": request_id}),
|
||||
)
|
||||
.await?;
|
||||
let endpoint = format!(
|
||||
@@ -13140,9 +13325,9 @@ async fn native_notification_replay_rechecks_requests_settled_during_the_read()
|
||||
.context("notification preparation did not reach the held durable read")?
|
||||
.context("replay hook closed")?;
|
||||
assert_eq!(point.thread_id, thread.id);
|
||||
if let Some(approval) = approval {
|
||||
if let Some((approval_id, approval)) = approval {
|
||||
assert!(manager.deliver_external_approval(
|
||||
"request",
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
assert_eq!(
|
||||
|
||||
@@ -3310,12 +3310,21 @@ pub struct ThreadDetail {
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct PendingApprovalRequest {
|
||||
/// Runtime-owned, single-use approval ID. Clients echo this value; it is
|
||||
/// independent of the provider's tool-call ID.
|
||||
pub id: String,
|
||||
pub turn_id: String,
|
||||
pub tool_name: String,
|
||||
pub description: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub intent_summary: Option<String>,
|
||||
/// The provider tool-call ID this approval gates, mirroring the
|
||||
/// `tool_call_id` on `approval.required`. A client resuming from a
|
||||
/// snapshot needs it to attach the prompt to the tool row it belongs to.
|
||||
/// It is a correlator, never a capability: `deliver_external_approval`
|
||||
/// matches `id` only, so this value settles nothing.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub tool_call_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
@@ -4765,21 +4774,36 @@ impl RuntimeThreadManager {
|
||||
*self.automations.lock() = Some(automations);
|
||||
}
|
||||
|
||||
/// Mints an approval identifier that no provider can predict or collide
|
||||
/// with. Every `approval_id` an external client ever sees comes from here,
|
||||
/// so `approval_id` never carries a provider tool-call ID and a client can
|
||||
/// echo it without inspecting where it came from.
|
||||
fn mint_approval_id() -> String {
|
||||
format!("approval_{}", Uuid::new_v4().simple())
|
||||
}
|
||||
|
||||
fn register_pending_approval(
|
||||
&self,
|
||||
thread_id: &str,
|
||||
request: PendingApprovalRequest,
|
||||
) -> oneshot::Receiver<ExternalApprovalDecision> {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
self.pending_approvals.lock().insert(
|
||||
request.id.clone(),
|
||||
PendingApprovalEntry {
|
||||
thread_id: thread_id.to_string(),
|
||||
request,
|
||||
sender: tx,
|
||||
},
|
||||
);
|
||||
rx
|
||||
mut request: PendingApprovalRequest,
|
||||
) -> (String, oneshot::Receiver<ExternalApprovalDecision>) {
|
||||
// Provider call IDs can repeat across sessions and responses. Mint the
|
||||
// external capability here so delivery and cleanup can only settle this
|
||||
// registration, including after the provider reuses a previous ID.
|
||||
let mut pending = self.pending_approvals.lock();
|
||||
loop {
|
||||
let id = Self::mint_approval_id();
|
||||
if let std::collections::hash_map::Entry::Vacant(entry) = pending.entry(id.clone()) {
|
||||
request.id = id.clone();
|
||||
let (tx, rx) = oneshot::channel();
|
||||
entry.insert(PendingApprovalEntry {
|
||||
thread_id: thread_id.to_string(),
|
||||
request,
|
||||
sender: tx,
|
||||
});
|
||||
return (id, rx);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn cancel_pending_approval(&self, approval_id: &str) {
|
||||
@@ -5336,28 +5360,35 @@ impl RuntimeThreadManager {
|
||||
self.pending_dynamic_tools.lock().len()
|
||||
}
|
||||
|
||||
/// Registers a pending approval and returns `(minted approval id, waiter)`.
|
||||
/// `label` is only a description and a stand-in provider call ID: the
|
||||
/// caller cannot choose the approval ID, exactly as a provider cannot.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn register_pending_approval_for_test(
|
||||
&self,
|
||||
approval_id: &str,
|
||||
) -> oneshot::Receiver<ExternalApprovalDecision> {
|
||||
self.register_pending_approval_for_thread_for_test("test-thread", approval_id)
|
||||
label: &str,
|
||||
) -> (String, oneshot::Receiver<ExternalApprovalDecision>) {
|
||||
self.register_pending_approval_for_thread_for_test("test-thread", label)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn register_pending_approval_for_thread_for_test(
|
||||
&self,
|
||||
thread_id: &str,
|
||||
approval_id: &str,
|
||||
) -> oneshot::Receiver<ExternalApprovalDecision> {
|
||||
label: &str,
|
||||
) -> (String, oneshot::Receiver<ExternalApprovalDecision>) {
|
||||
self.register_pending_approval(
|
||||
thread_id,
|
||||
PendingApprovalRequest {
|
||||
id: approval_id.to_string(),
|
||||
// Overwritten by the mint; a caller-supplied ID is never honored.
|
||||
id: String::new(),
|
||||
turn_id: "test-turn".to_string(),
|
||||
tool_name: "test-tool".to_string(),
|
||||
description: "test approval".to_string(),
|
||||
description: format!("test approval {label}"),
|
||||
intent_summary: None,
|
||||
// Stands in for the provider's raw call ID so tests can prove
|
||||
// the correlator is visible and still not deliverable.
|
||||
tool_call_id: Some(label.to_string()),
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -11056,22 +11087,34 @@ impl RuntimeThreadManager {
|
||||
let approval_mode = authority.approval_mode;
|
||||
|
||||
let pending_request = PendingApprovalRequest {
|
||||
id: id.clone(),
|
||||
// Replaced by the minted ID at registration. The raw
|
||||
// provider call ID travels in `tool_call_id`, where it
|
||||
// is a correlator a snapshot client can match against
|
||||
// the tool row and never a value the API will accept.
|
||||
id: String::new(),
|
||||
turn_id: turn_id.clone(),
|
||||
tool_name: tool_name.clone(),
|
||||
description: description.clone(),
|
||||
intent_summary: intent_summary.clone(),
|
||||
tool_call_id: Some(id.clone()),
|
||||
};
|
||||
|
||||
if auto_approve {
|
||||
// No waiter is registered on this path, but the emitted
|
||||
// identity still has to obey the one contract clients
|
||||
// read: `approval_id` is ours, `tool_call_id` is the
|
||||
// provider's. Two turns holding the same raw call ID
|
||||
// therefore stay distinguishable in the event stream.
|
||||
let approval_id = Self::mint_approval_id();
|
||||
self.emit_event(
|
||||
&thread_id,
|
||||
Some(&turn_id),
|
||||
None,
|
||||
"approval.required",
|
||||
json!({
|
||||
"id": id,
|
||||
"approval_id": id,
|
||||
"id": approval_id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"tool_name": tool_name,
|
||||
"description": description,
|
||||
"intent_summary": intent_summary,
|
||||
@@ -11096,7 +11139,8 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.decided",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"decision": dec_str,
|
||||
"remember": false,
|
||||
"auto": true,
|
||||
@@ -11124,7 +11168,8 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.decided",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": Self::mint_approval_id(),
|
||||
"tool_call_id": id,
|
||||
"decision": "deny",
|
||||
"remember": false,
|
||||
"auto": true,
|
||||
@@ -11142,7 +11187,8 @@ impl RuntimeThreadManager {
|
||||
// subscribes from an older cursor that will replay it.
|
||||
let projection_lock = self.projection_lock(&thread_id);
|
||||
let projection = projection_lock.lock().await;
|
||||
let rx = self.register_pending_approval(&thread_id, pending_request);
|
||||
let (approval_id, rx) =
|
||||
self.register_pending_approval(&thread_id, pending_request);
|
||||
if let Err(err) = self
|
||||
.emit_event(
|
||||
&thread_id,
|
||||
@@ -11150,8 +11196,9 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.required",
|
||||
json!({
|
||||
"id": id,
|
||||
"approval_id": id,
|
||||
"id": approval_id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"tool_name": tool_name,
|
||||
"description": description,
|
||||
"intent_summary": intent_summary,
|
||||
@@ -11159,7 +11206,7 @@ impl RuntimeThreadManager {
|
||||
)
|
||||
.await
|
||||
{
|
||||
self.cancel_pending_approval(&id);
|
||||
self.cancel_pending_approval(&approval_id);
|
||||
drop(projection);
|
||||
let _ = engine.deny_tool_call(&id).await;
|
||||
return Err(err);
|
||||
@@ -11181,7 +11228,8 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.decided",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"decision": "allow",
|
||||
"remember": remember,
|
||||
}),
|
||||
@@ -11197,7 +11245,8 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.decided",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"decision": "deny",
|
||||
"remember": remember,
|
||||
}),
|
||||
@@ -11207,18 +11256,19 @@ impl RuntimeThreadManager {
|
||||
let _ = engine.deny_tool_call(id).await;
|
||||
}
|
||||
Ok(Err(_recv_err)) => {
|
||||
self.cancel_pending_approval(&id);
|
||||
self.cancel_pending_approval(&approval_id);
|
||||
let _ = engine.deny_tool_call(id).await;
|
||||
}
|
||||
Err(_timeout) => {
|
||||
self.cancel_pending_approval(&id);
|
||||
self.cancel_pending_approval(&approval_id);
|
||||
self.emit_event(
|
||||
&thread_id,
|
||||
Some(&turn_id),
|
||||
None,
|
||||
"approval.timeout",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"timeout_secs": approval_timeout.map(|wait| wait.as_secs()),
|
||||
}),
|
||||
)
|
||||
@@ -11230,7 +11280,8 @@ impl RuntimeThreadManager {
|
||||
None,
|
||||
"approval.decided",
|
||||
json!({
|
||||
"approval_id": id,
|
||||
"approval_id": approval_id,
|
||||
"tool_call_id": id,
|
||||
"decision": "deny",
|
||||
"remember": false,
|
||||
"timeout": true,
|
||||
|
||||
@@ -8058,7 +8058,7 @@ async fn worker_lifecycle_receipts_preserve_owner_outcome_and_durable_replay() -
|
||||
status: "private progress".into(),
|
||||
activity: AgentProgressEventMeta::new(AgentWorkerStatus::RunningTool)
|
||||
.with_step(3)
|
||||
.with_tool("private_tool".into()),
|
||||
.with_tool("private_tool"),
|
||||
parent_run_id: Some("parent".into()),
|
||||
spawn_depth: 2,
|
||||
})
|
||||
@@ -10133,6 +10133,56 @@ async fn approval_required_with_stale_active_turn_is_denied() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Reads the approval identity exactly as an external client does: off the
|
||||
/// `approval.required` event for `raw_call_id`, returning the opaque ID that
|
||||
/// client must echo back. Also pins the two properties every caller below
|
||||
/// depends on — the opaque ID is never the provider's call ID, and the legacy
|
||||
/// `id` field aliases the opaque ID rather than leaking the raw one.
|
||||
async fn await_approval_identity(
|
||||
manager: &RuntimeThreadManager,
|
||||
thread_id: &str,
|
||||
raw_call_id: &str,
|
||||
) -> Result<String> {
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
let found = manager
|
||||
.events_since(thread_id, None)?
|
||||
.into_iter()
|
||||
.rev()
|
||||
.find(|event| {
|
||||
event.event == "approval.required"
|
||||
&& event.payload.get("tool_call_id").and_then(Value::as_str)
|
||||
== Some(raw_call_id)
|
||||
});
|
||||
if let Some(event) = found {
|
||||
let approval_id = event
|
||||
.payload
|
||||
.get("approval_id")
|
||||
.and_then(Value::as_str)
|
||||
.context("approval.required must carry an approval_id")?
|
||||
.to_string();
|
||||
assert_eq!(
|
||||
event.payload.get("id").and_then(Value::as_str),
|
||||
Some(approval_id.as_str()),
|
||||
"the legacy `id` field must alias the opaque approval id"
|
||||
);
|
||||
assert_ne!(
|
||||
approval_id, raw_call_id,
|
||||
"the approval capability must not be the provider's tool-call id"
|
||||
);
|
||||
assert!(
|
||||
approval_id.starts_with("approval_"),
|
||||
"unexpected approval id shape: {approval_id}"
|
||||
);
|
||||
return Ok(approval_id);
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
bail!("no approval.required event for tool call '{raw_call_id}' on '{thread_id}'");
|
||||
}
|
||||
sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn approval_required_awaits_external_decision_allow() -> Result<()> {
|
||||
let manager = test_manager(test_runtime_dir())?;
|
||||
@@ -10192,11 +10242,21 @@ async fn approval_required_awaits_external_decision_allow() -> Result<()> {
|
||||
}
|
||||
assert_eq!(manager.pending_approvals_count(), 1);
|
||||
|
||||
let approval_id = await_approval_identity(&manager, &thread.id, "tool_external_allow").await?;
|
||||
|
||||
// The snapshot a reconnecting client resumes from carries the same opaque
|
||||
// capability as the event, so both routes to the ID agree.
|
||||
let detail = manager.get_thread_detail(&thread.id).await?;
|
||||
assert_eq!(detail.pending_approvals.len(), 1);
|
||||
assert_eq!(detail.pending_approvals[0].id, "tool_external_allow");
|
||||
assert_eq!(detail.pending_approvals[0].id, approval_id);
|
||||
assert_eq!(detail.pending_approvals[0].turn_id, _turn.id);
|
||||
assert_eq!(detail.pending_approvals[0].tool_name, "exec_command");
|
||||
// The snapshot keeps the raw correlator so a client that reloaded past the
|
||||
// event can still attach this prompt to the tool row it gates.
|
||||
assert_eq!(
|
||||
detail.pending_approvals[0].tool_call_id.as_deref(),
|
||||
Some("tool_external_allow")
|
||||
);
|
||||
assert_eq!(detail.pending_user_inputs.len(), 0);
|
||||
|
||||
let events = manager.events_since(&thread.id, None)?;
|
||||
@@ -10213,10 +10273,18 @@ async fn approval_required_awaits_external_decision_allow() -> Result<()> {
|
||||
Some("I will update the config file.")
|
||||
);
|
||||
|
||||
assert!(manager.deliver_external_approval(
|
||||
// The raw provider ID is not a credential: only the minted one settles.
|
||||
assert!(!manager.deliver_external_approval(
|
||||
"tool_external_allow",
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
assert_eq!(manager.pending_approvals_count(), 1);
|
||||
|
||||
assert!(manager.deliver_external_approval(
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
// The engine is still addressed by the provider's own call ID.
|
||||
assert_eq!(
|
||||
harness.recv_approval_event().await,
|
||||
Some(MockApprovalEvent::Approved {
|
||||
@@ -10231,6 +10299,17 @@ async fn approval_required_awaits_external_decision_allow() -> Result<()> {
|
||||
.pending_approvals
|
||||
.is_empty()
|
||||
);
|
||||
assert!(
|
||||
manager.events_since(&thread.id, None)?.iter().any(|event| {
|
||||
event.event == "approval.decided"
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str)
|
||||
== Some(approval_id.as_str())
|
||||
&& event.payload.get("tool_call_id").and_then(Value::as_str)
|
||||
== Some("tool_external_allow")
|
||||
&& event.payload.get("decision").and_then(Value::as_str) == Some("allow")
|
||||
}),
|
||||
"approval.decided must correlate the opaque id with the tool call it settled"
|
||||
);
|
||||
|
||||
harness
|
||||
.tx_event
|
||||
@@ -12543,8 +12622,9 @@ async fn approval_required_external_deny_is_denied() -> Result<()> {
|
||||
}
|
||||
assert_eq!(manager.pending_approvals_count(), 1);
|
||||
|
||||
let approval_id = await_approval_identity(&manager, &thread.id, "tool_external_deny").await?;
|
||||
assert!(manager.deliver_external_approval(
|
||||
"tool_external_deny",
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Deny { remember: false },
|
||||
));
|
||||
assert_eq!(
|
||||
@@ -12554,6 +12634,18 @@ async fn approval_required_external_deny_is_denied() -> Result<()> {
|
||||
})
|
||||
);
|
||||
|
||||
// A settled capability is spent: replaying it neither re-decides nor
|
||||
// reaches the engine a second time.
|
||||
assert!(!manager.deliver_external_approval(
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: true },
|
||||
));
|
||||
assert_eq!(manager.pending_approvals_count(), 0);
|
||||
assert!(
|
||||
!manager.store.load_thread(&thread.id)?.auto_approve,
|
||||
"a replayed decision must not apply its remember flag"
|
||||
);
|
||||
|
||||
harness
|
||||
.tx_event
|
||||
.send(EngineEvent::TurnComplete {
|
||||
@@ -12569,6 +12661,147 @@ async fn approval_required_external_deny_is_denied() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Providers restart their tool-call ID counters per response, so two threads
|
||||
/// can hold gated calls whose raw IDs are byte-equal. While the waiter map was
|
||||
/// keyed by that raw ID, the second registration silently evicted the first:
|
||||
/// one thread's approval could authorize the other thread's tool call, and the
|
||||
/// evicted thread waited out its timeout for a decision that had been made.
|
||||
#[tokio::test]
|
||||
async fn identical_raw_tool_call_ids_on_two_threads_stay_independently_gated() -> Result<()> {
|
||||
const RAW_CALL_ID: &str = "call_1";
|
||||
let manager = test_manager(test_runtime_dir())?;
|
||||
|
||||
let mut gated = Vec::new();
|
||||
for prompt in ["first gated turn", "second gated turn"] {
|
||||
let thread = manager
|
||||
.create_thread(CreateThreadRequest::default())
|
||||
.await?;
|
||||
let mut harness = install_mock_engine(&manager, &thread.id).await;
|
||||
manager
|
||||
.start_turn(
|
||||
&thread.id,
|
||||
StartTurnRequest {
|
||||
prompt: prompt.to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
assert!(matches!(
|
||||
harness.rx_op.recv().await,
|
||||
Some(Op::SendMessage { .. })
|
||||
));
|
||||
harness
|
||||
.tx_event
|
||||
.send(EngineEvent::ApprovalRequired {
|
||||
approval_key: RAW_CALL_ID.to_string(),
|
||||
approval_grouping_key: RAW_CALL_ID.to_string(),
|
||||
id: RAW_CALL_ID.to_string(),
|
||||
tool_name: "exec_command".to_string(),
|
||||
description: format!("gated {prompt}"),
|
||||
input: serde_json::json!({}),
|
||||
intent_summary: None,
|
||||
approval_force_prompt: false,
|
||||
})
|
||||
.await?;
|
||||
let approval_id = await_approval_identity(&manager, &thread.id, RAW_CALL_ID).await?;
|
||||
gated.push((thread, harness, approval_id));
|
||||
}
|
||||
|
||||
// Neither registration evicted the other, and each thread's snapshot shows
|
||||
// only its own capability.
|
||||
assert_eq!(manager.pending_approvals_count(), 2);
|
||||
assert_ne!(
|
||||
gated[0].2, gated[1].2,
|
||||
"each registration must mint its own capability"
|
||||
);
|
||||
for (thread, _, approval_id) in &gated {
|
||||
let pending = manager
|
||||
.get_thread_detail(&thread.id)
|
||||
.await?
|
||||
.pending_approvals;
|
||||
assert_eq!(pending.len(), 1);
|
||||
assert_eq!(&pending[0].id, approval_id);
|
||||
// Both snapshots correlate to the same raw call ID — that is the
|
||||
// provider behavior this fix survives — while the capability that
|
||||
// settles each one is distinct.
|
||||
assert_eq!(pending[0].tool_call_id.as_deref(), Some(RAW_CALL_ID));
|
||||
}
|
||||
|
||||
// The shared correlator is visible to both clients and settles neither.
|
||||
assert!(!manager.deliver_external_approval(
|
||||
RAW_CALL_ID,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
assert_eq!(manager.pending_approvals_count(), 2);
|
||||
|
||||
let mut gated = gated.into_iter();
|
||||
let (thread_a, mut harness_a, approval_a) = gated.next().context("first gated thread")?;
|
||||
let (thread_b, mut harness_b, approval_b) = gated.next().context("second gated thread")?;
|
||||
|
||||
// Deciding the first settles the first, addressed to the engine by the raw
|
||||
// call ID it gated on.
|
||||
assert!(manager.deliver_external_approval(
|
||||
&approval_a,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
assert_eq!(
|
||||
harness_a.recv_approval_event().await,
|
||||
Some(MockApprovalEvent::Approved {
|
||||
id: RAW_CALL_ID.to_string(),
|
||||
})
|
||||
);
|
||||
|
||||
// ...and cross-authorizes nothing: the second thread is still waiting.
|
||||
assert!(
|
||||
tokio::time::timeout(Duration::from_millis(200), harness_b.recv_approval_event())
|
||||
.await
|
||||
.is_err(),
|
||||
"an approval for one thread must not decide another thread's identical call id"
|
||||
);
|
||||
assert_eq!(manager.pending_approvals_count(), 1);
|
||||
let still_pending = manager
|
||||
.get_thread_detail(&thread_b.id)
|
||||
.await?
|
||||
.pending_approvals;
|
||||
assert_eq!(still_pending.len(), 1);
|
||||
assert_eq!(still_pending[0].id, approval_b);
|
||||
assert!(
|
||||
manager
|
||||
.get_thread_detail(&thread_a.id)
|
||||
.await?
|
||||
.pending_approvals
|
||||
.is_empty()
|
||||
);
|
||||
|
||||
assert!(manager.deliver_external_approval(
|
||||
&approval_b,
|
||||
ExternalApprovalDecision::Deny { remember: false },
|
||||
));
|
||||
assert_eq!(
|
||||
harness_b.recv_approval_event().await,
|
||||
Some(MockApprovalEvent::Denied {
|
||||
id: RAW_CALL_ID.to_string(),
|
||||
})
|
||||
);
|
||||
assert_eq!(manager.pending_approvals_count(), 0);
|
||||
|
||||
for harness in [&harness_a, &harness_b] {
|
||||
harness
|
||||
.tx_event
|
||||
.send(EngineEvent::TurnComplete {
|
||||
usage: Usage::default(),
|
||||
parent_route_usage: Usage::default(),
|
||||
routed_usage_dropped_records: 0,
|
||||
status: TurnOutcomeStatus::Completed,
|
||||
error: None,
|
||||
tool_catalog: None,
|
||||
base_url: None,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn auto_review_force_prompt_is_denied_without_opening_a_modal() -> Result<()> {
|
||||
let manager = test_manager(test_runtime_dir())?;
|
||||
@@ -12620,11 +12853,31 @@ async fn auto_review_force_prompt_is_denied_without_opening_a_modal() -> Result<
|
||||
})
|
||||
);
|
||||
assert_eq!(manager.pending_approvals_count(), 0);
|
||||
assert!(manager.events_since(&thread.id, None)?.iter().any(|event| {
|
||||
event.event == "approval.decided"
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str) == Some("tool_auto_hold")
|
||||
&& event.payload.get("posture").and_then(Value::as_str) == Some("auto_review")
|
||||
}));
|
||||
let decided = manager
|
||||
.events_since(&thread.id, None)?
|
||||
.into_iter()
|
||||
.find(|event| {
|
||||
event.event == "approval.decided"
|
||||
&& event.payload.get("posture").and_then(Value::as_str) == Some("auto_review")
|
||||
})
|
||||
.context("auto-review hold must emit approval.decided")?;
|
||||
assert_eq!(
|
||||
decided.payload.get("tool_call_id").and_then(Value::as_str),
|
||||
Some("tool_auto_hold")
|
||||
);
|
||||
// Even on a path with no waiter, `approval_id` stays a runtime-minted
|
||||
// value so a client never learns a raw call ID through that field.
|
||||
let approval_id = decided
|
||||
.payload
|
||||
.get("approval_id")
|
||||
.and_then(Value::as_str)
|
||||
.context("missing approval_id")?;
|
||||
assert_ne!(approval_id, "tool_auto_hold");
|
||||
assert!(approval_id.starts_with("approval_"));
|
||||
assert!(!manager.deliver_external_approval(
|
||||
approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
|
||||
harness
|
||||
.tx_event
|
||||
@@ -12708,23 +12961,34 @@ async fn approval_timeout_denies_clears_ui_and_next_turn_can_start() -> Result<(
|
||||
);
|
||||
assert_eq!(manager.pending_approvals_count(), 0);
|
||||
|
||||
let approval_id = await_approval_identity(&manager, &thread.id, "tool_timeout").await?;
|
||||
let events = manager.events_since(&thread.id, None)?;
|
||||
assert!(
|
||||
events.iter().any(|event| {
|
||||
event.event == "approval.timeout"
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str) == Some("tool_timeout")
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str)
|
||||
== Some(approval_id.as_str())
|
||||
&& event.payload.get("tool_call_id").and_then(Value::as_str) == Some("tool_timeout")
|
||||
}),
|
||||
"timeout event should be persisted"
|
||||
"timeout event should be persisted against the same opaque id it opened with"
|
||||
);
|
||||
assert!(
|
||||
events.iter().any(|event| {
|
||||
event.event == "approval.decided"
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str) == Some("tool_timeout")
|
||||
&& event.payload.get("approval_id").and_then(Value::as_str)
|
||||
== Some(approval_id.as_str())
|
||||
&& event.payload.get("tool_call_id").and_then(Value::as_str) == Some("tool_timeout")
|
||||
&& event.payload.get("decision").and_then(Value::as_str) == Some("deny")
|
||||
&& event.payload.get("timeout").and_then(Value::as_bool) == Some(true)
|
||||
}),
|
||||
"timeout should also emit approval.decided so clients can clear pending UI"
|
||||
);
|
||||
// A decision arriving after the timeout cancelled the waiter must not
|
||||
// resurrect it — the engine already has its denial.
|
||||
assert!(!manager.deliver_external_approval(
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
));
|
||||
|
||||
harness
|
||||
.tx_event
|
||||
@@ -12941,8 +13205,9 @@ async fn approval_required_remember_flips_thread_auto_approve() -> Result<()> {
|
||||
while Instant::now() < deadline && manager.pending_approvals_count() == 0 {
|
||||
sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
let approval_id = await_approval_identity(&manager, &thread.id, "tool_remember").await?;
|
||||
assert!(manager.deliver_external_approval(
|
||||
"tool_remember",
|
||||
&approval_id,
|
||||
ExternalApprovalDecision::Allow { remember: true },
|
||||
));
|
||||
let _ = harness.recv_approval_event().await;
|
||||
|
||||
@@ -2037,6 +2037,7 @@ impl Settings {
|
||||
/// Add a model to a provider's enabled chooser set without removing prior
|
||||
/// choices. IDs are compared case-insensitively but preserve their wire
|
||||
/// spelling on disk.
|
||||
#[cfg(test)]
|
||||
pub fn enable_model_for_provider(&mut self, provider: &str, model: &str) {
|
||||
let provider = provider.trim();
|
||||
let model = model.trim();
|
||||
|
||||
@@ -14,7 +14,13 @@ use std::path::Path;
|
||||
/// continuous operate-mode operations).
|
||||
/// Generation 10 adds the bundled `mcp-discovery` skill (Registry-first
|
||||
/// tool selection).
|
||||
const BUNDLED_SKILL_VERSION: &str = "10";
|
||||
/// Generation 11 rewrites `mcp-discovery` from a Registry-first gate into a
|
||||
/// missing-capability fallback, and corrects two stale facts in its body:
|
||||
/// `registry_sync` requires a `query`, and the discovery tools are deferred
|
||||
/// rather than always present in the active surface. Because the rewrite
|
||||
/// changes an already-installed body, generation 10's exact body is retained in
|
||||
/// `SUPERSEDED_BODIES` so unmodified copies upgrade and edited copies do not.
|
||||
const BUNDLED_SKILL_VERSION: &str = "11";
|
||||
|
||||
// ── system & extension (meta) ───────────────────────────────────────────────
|
||||
const SKILL_CREATOR_BODY: &str = include_str!("../../assets/skills/skill-creator/SKILL.md");
|
||||
@@ -65,6 +71,30 @@ const MCP_DISCOVERY_BODY: &str = include_str!("../../assets/skills/mcp-discovery
|
||||
// Legacy v4 body retained solely for digest-based safe retirement (#4691).
|
||||
const V4_BEST_PRACTICES_BODY: &str = include_str!("../../assets/skills/v4-best-practices/SKILL.md");
|
||||
|
||||
// Generation-10 `mcp-discovery` body, retained solely so an unmodified copy of
|
||||
// it can be recognized and refreshed on upgrade. Same digest discipline as the
|
||||
// v4 retirement above: an exact byte match proves CodeWhale still owns the
|
||||
// file, so replacing it loses no user work.
|
||||
const MCP_DISCOVERY_GENERATION_10_BODY: &str =
|
||||
include_str!("../../assets/skills/mcp-discovery/SKILL.generation-10.md");
|
||||
|
||||
/// Exact bodies a bundled skill shipped in an earlier generation.
|
||||
///
|
||||
/// The installer refuses to overwrite an installed body it does not recognize,
|
||||
/// which is what protects user edits. Without this table that same rule also
|
||||
/// pins every unmodified older copy forever: the on-disk body no longer equals
|
||||
/// the shipped one, so the skill never upgrades. Listing the previous body
|
||||
/// restores the upgrade for exactly the copies CodeWhale wrote itself.
|
||||
const SUPERSEDED_BODIES: &[(&str, &str)] = &[("mcp-discovery", MCP_DISCOVERY_GENERATION_10_BODY)];
|
||||
|
||||
/// Whether `existing` is byte-for-byte a body CodeWhale previously shipped for
|
||||
/// `name` (and therefore safe to replace on upgrade).
|
||||
fn is_superseded_shipped_body(name: &str, existing: &str) -> bool {
|
||||
SUPERSEDED_BODIES
|
||||
.iter()
|
||||
.any(|(skill, body)| *skill == name && *body == existing)
|
||||
}
|
||||
|
||||
struct BundledSkill {
|
||||
name: &'static str,
|
||||
body: &'static str,
|
||||
@@ -373,10 +403,12 @@ fn install_one(
|
||||
|
||||
if should_install {
|
||||
// Never overwrite a user-modified copy that no longer matches a known
|
||||
// shipped body (#4691 non-destructive upgrade table).
|
||||
// shipped body (#4691 non-destructive upgrade table). A body we shipped
|
||||
// in an earlier generation is still a known shipped body, so it may be
|
||||
// refreshed; anything else is the user's.
|
||||
if target_file.exists() {
|
||||
let existing = fs::read_to_string(&target_file).unwrap_or_default();
|
||||
if !existing.is_empty() && existing != skill.body {
|
||||
let existing = fs::read_to_string(&target_file)?;
|
||||
if existing != skill.body && !is_superseded_shipped_body(skill.name, &existing) {
|
||||
// Preserve user/compatible-root content; skip replace-by-name.
|
||||
return Ok(false);
|
||||
}
|
||||
@@ -393,7 +425,9 @@ fn install_one(
|
||||
/// - Fresh install (no marker, no dir): installs every bundled skill, then
|
||||
/// writes the version marker.
|
||||
/// - Version bump (marker present with older version): re-installs any existing
|
||||
/// bundled skill and installs newly introduced bundled skills.
|
||||
/// bundled skill whose body is still one CodeWhale shipped (current or a
|
||||
/// [`SUPERSEDED_BODIES`] entry) and installs newly introduced bundled skills.
|
||||
/// A user-edited body is never replaced.
|
||||
/// - User deleted a skill dir while marker still present at same version: leaves
|
||||
/// it gone.
|
||||
/// - Idempotent: calling twice with no changes is a no-op.
|
||||
|
||||
@@ -76,8 +76,9 @@ fn contributor_onboarding_ships_at_generation_8_and_keeps_its_refusals() {
|
||||
.expect("contributor-onboarding must be bundled");
|
||||
assert_eq!(skill.introduced_in, 8);
|
||||
// The pin tracks the current catalog generation: 9 added handoff,
|
||||
// 10 added mcp-discovery (#5238).
|
||||
assert_eq!(BUNDLED_SKILL_VERSION, "10");
|
||||
// 10 added mcp-discovery (#5238), 11 rewrote mcp-discovery from a
|
||||
// Registry-first gate into a missing-capability fallback.
|
||||
assert_eq!(BUNDLED_SKILL_VERSION, "11");
|
||||
|
||||
let body = skill.body;
|
||||
assert!(body.contains("invocation: explicit-only"));
|
||||
@@ -482,6 +483,93 @@ fn upgrade_preserves_user_modified_bundled_skill_body() {
|
||||
);
|
||||
}
|
||||
|
||||
/// A generation-10 install carries the Registry-first `mcp-discovery` body.
|
||||
/// The generation-11 rewrite is only real for existing users if that untouched
|
||||
/// copy is actually replaced — the digest allowance is what makes the upgrade
|
||||
/// reach them instead of stopping at "body differs, must be the user's".
|
||||
#[test]
|
||||
fn upgrade_from_generation_10_refreshes_untouched_mcp_discovery() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let old = MCP_DISCOVERY_GENERATION_10_BODY;
|
||||
let path = skill_file(&tmp, "mcp-discovery");
|
||||
fs::create_dir_all(skill_dir(&tmp, "mcp-discovery")).unwrap();
|
||||
fs::write(&path, old).unwrap();
|
||||
fs::write(marker_file(&tmp), "10").unwrap();
|
||||
|
||||
install_system_skills(tmp.path()).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
fs::read_to_string(&path).unwrap(),
|
||||
MCP_DISCOVERY_BODY,
|
||||
"an unmodified generation-10 body must upgrade to the shipped body"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(marker_file(&tmp)).unwrap().trim(),
|
||||
BUNDLED_SKILL_VERSION
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upgrade_from_generation_10_preserves_user_edited_mcp_discovery() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let edited = format!("{MCP_DISCOVERY_GENERATION_10_BODY}\n- my own house rule\n");
|
||||
let path = skill_file(&tmp, "mcp-discovery");
|
||||
fs::create_dir_all(skill_dir(&tmp, "mcp-discovery")).unwrap();
|
||||
fs::write(&path, &edited).unwrap();
|
||||
fs::write(marker_file(&tmp), "10").unwrap();
|
||||
|
||||
install_system_skills(tmp.path()).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
fs::read_to_string(&path).unwrap(),
|
||||
edited,
|
||||
"an edited copy stays the user's, even one derived from a shipped body"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upgrade_preserves_an_intentionally_empty_mcp_discovery() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let path = skill_file(&tmp, "mcp-discovery");
|
||||
fs::create_dir_all(skill_dir(&tmp, "mcp-discovery")).unwrap();
|
||||
fs::write(&path, "").unwrap();
|
||||
fs::write(marker_file(&tmp), "10").unwrap();
|
||||
install_system_skills(tmp.path()).unwrap();
|
||||
assert_eq!(fs::read_to_string(path).unwrap(), "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_discovery_deleted_at_generation_10_stays_deleted() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
fs::write(marker_file(&tmp), "10").unwrap();
|
||||
|
||||
install_system_skills(tmp.path()).unwrap();
|
||||
|
||||
assert!(
|
||||
!skill_file(&tmp, "mcp-discovery").exists(),
|
||||
"the digest allowance must not resurrect a skill the user removed"
|
||||
);
|
||||
}
|
||||
|
||||
/// The retained body is evidence, not decoration: it must be the superseded
|
||||
/// text (Registry-first, `registry_sync {}`, always-present tools), never a
|
||||
/// stale copy of the current one, or the allowance would silently do nothing.
|
||||
#[test]
|
||||
fn retained_generation_10_body_is_the_superseded_one() {
|
||||
let old = MCP_DISCOVERY_GENERATION_10_BODY;
|
||||
assert_ne!(old, MCP_DISCOVERY_BODY);
|
||||
assert!(is_superseded_shipped_body("mcp-discovery", old));
|
||||
assert!(!is_superseded_shipped_body(
|
||||
"mcp-discovery",
|
||||
MCP_DISCOVERY_BODY
|
||||
));
|
||||
assert!(!is_superseded_shipped_body("debug", old));
|
||||
// The stale generation-10 facts the rewrite exists to remove.
|
||||
assert!(old.contains("registry_sync {}"));
|
||||
assert!(old.contains("available in the active tool"));
|
||||
assert!(MCP_DISCOVERY_BODY.contains("registry_sync {query:"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn end_user_pack_skills_parse_for_discovery() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
@@ -83,6 +83,9 @@ pub struct TurnStopDiagnostics {
|
||||
/// Last parent response's reported input tokens, not cumulative billing.
|
||||
pub last_reported_input_tokens: Option<u32>,
|
||||
pub route_context_window_tokens: Option<u64>,
|
||||
/// Engine-prepared output allowance. A transport may omit the field;
|
||||
/// this is budget evidence, not a provider-published capability ceiling.
|
||||
pub last_prepared_output_limit_tokens: Option<u32>,
|
||||
pub automatic_compaction_attempts: u32,
|
||||
pub emergency_compaction_attempts: u32,
|
||||
}
|
||||
|
||||
@@ -561,18 +561,26 @@ fn server_to_entry(server: RegistryServer) -> Option<McpRegistryServerEntry> {
|
||||
})
|
||||
}
|
||||
|
||||
/// Prompt attached to every `registry_sync` result (Registry-first policy).
|
||||
/// Prompt attached to every `registry_sync` result.
|
||||
///
|
||||
/// A scored list is not a verdict. This used to read "you must call
|
||||
/// start_registry_mcp_server ... before using shell commands, local programs,
|
||||
/// custom code, or a manual implementation", which turned any near-miss row
|
||||
/// into an obligation: a turn that only had to write an HTML file and read a
|
||||
/// fixture matched a browser-automation server and spent itself on starting
|
||||
/// that server. The result now describes the list and leaves the choice with
|
||||
/// the model, while keeping the one instruction the host actually depends on —
|
||||
/// start an entry through `start_registry_mcp_server`, never by running its
|
||||
/// package command through the shell.
|
||||
const REGISTRY_FIRST_PROMPT: &str = concat!(
|
||||
"REGISTRY-FIRST POLICY: These are the top scored matches for your ",
|
||||
"query from the local Registry snapshot; the full catalog stays on the ",
|
||||
"host. Treat a server as a match when it plausibly covers the task's ",
|
||||
"core specialized capability; wording need not be exact. If a returned ",
|
||||
"match is plausible, you must call start_registry_mcp_server with its ",
|
||||
"exact name and inspect its tools before using shell commands, local ",
|
||||
"programs, custom code, or a manual implementation. When no returned ",
|
||||
"match plausibly covers the capability, refine the query once; if the ",
|
||||
"refined query still returns nothing plausible, fall back to local ",
|
||||
"tools.",
|
||||
"These are the top scored matches for your query from the local Registry ",
|
||||
"snapshot; the full catalog stays on the host. A match is only worth ",
|
||||
"starting when it covers a capability you do not already have — if an ",
|
||||
"available tool, a project script, or a few lines of local code already ",
|
||||
"does the job, use that and ignore these results. To start one, call ",
|
||||
"start_registry_mcp_server with its exact name; never install or run its ",
|
||||
"package command through the shell. If nothing here covers the missing ",
|
||||
"capability, refine the query once, then continue with local tools.",
|
||||
);
|
||||
|
||||
/// Host-side cap on model-visible Registry matches. The complete catalog
|
||||
@@ -870,14 +878,14 @@ impl ToolSpec for McpSyncRegistry {
|
||||
}
|
||||
|
||||
fn description(&self) -> &str {
|
||||
"Search installable local MCP servers for a specialized capability \
|
||||
and return at most eight scored matches; the full Registry index \
|
||||
stays host-side. Describe the capability you need in the query. \
|
||||
The index contains only stdio packages that declare no environment \
|
||||
variables or API keys. If a match plausibly covers the task's core \
|
||||
specialized capability, call start_registry_mcp_server with its \
|
||||
exact name and inspect its tools before choosing a local \
|
||||
alternative; do not run its package command through exec_shell."
|
||||
"Search installable local MCP servers for a capability this session \
|
||||
does not already have, and return at most eight scored matches; the \
|
||||
full Registry index stays host-side. Describe the missing capability \
|
||||
in the query. The index contains only stdio packages that declare no \
|
||||
environment variables or API keys. Use this when an available tool, a \
|
||||
project script, or ordinary local code cannot do the job — not before \
|
||||
ordinary work. To use a match, call start_registry_mcp_server with its \
|
||||
exact name; do not run its package command through the shell."
|
||||
}
|
||||
|
||||
fn input_schema(&self) -> Value {
|
||||
@@ -1425,6 +1433,28 @@ mod tests {
|
||||
assert!(miss.servers.is_empty());
|
||||
}
|
||||
|
||||
/// A scored row must not read as an obligation. The prompt this result
|
||||
/// carries used to say the model "must call start_registry_mcp_server ...
|
||||
/// before using shell commands, local programs, custom code, or a manual
|
||||
/// implementation", which is how a turn that only needed to write an HTML
|
||||
/// file and read a fixture ended up trying to start a browser server.
|
||||
#[test]
|
||||
fn result_prompt_does_not_oblige_starting_a_match() {
|
||||
let catalog = catalog_from_cache(&make_test_cache(), "filesystem");
|
||||
let instruction = catalog.instruction;
|
||||
|
||||
assert!(!instruction.contains("must call"));
|
||||
assert!(!instruction.contains("custom code"));
|
||||
assert!(!instruction.contains("manual implementation"));
|
||||
|
||||
// What the result is still responsible for saying: the matches are
|
||||
// only worth starting for a capability the session lacks, and a
|
||||
// package is started through the approved host path, never the shell.
|
||||
assert!(instruction.contains("a capability you do not already have"));
|
||||
assert!(instruction.contains("start_registry_mcp_server with its exact name"));
|
||||
assert!(instruction.contains("never install or run its package command through the shell"));
|
||||
}
|
||||
|
||||
/// Parse one `ServerResponse` JSON object the way the upstream list
|
||||
/// endpoint ships it. Lifecycle status travels in registry-managed
|
||||
/// `_meta` (`io.modelcontextprotocol.registry/official`), as a
|
||||
|
||||
@@ -134,13 +134,12 @@ use crate::tui::workspace_context;
|
||||
use super::key_actions;
|
||||
|
||||
use super::app::{
|
||||
ActiveCompaction, ActiveTurnMetadata, AgentCurrentActivity, AgentCurrentActivityStatus, App,
|
||||
AppAction, AppMode, ComposerSubmitAction, ComposerSubmitChord, EffectiveReasoningEffort,
|
||||
GoalControlIntent, OnboardingState, PendingGoalControl, PendingProviderSwitch, QueuedMessage,
|
||||
ReasoningEffort, RedactionGateNotice, ScreenMode, StatusToast, StatusToastLevel,
|
||||
SubmitDisposition, TaskPanelEntry, TaskPanelEntryKind, ToolEvidence, TuiOptions,
|
||||
bound_agent_activity_text, is_stop_word, looks_like_slash_command_input,
|
||||
shell_command_from_bang_input,
|
||||
ActiveCompaction, ActiveTurnMetadata, AgentCurrentActivity, App, AppAction, AppMode,
|
||||
ComposerSubmitAction, ComposerSubmitChord, EffectiveReasoningEffort, GoalControlIntent,
|
||||
OnboardingState, PendingGoalControl, PendingProviderSwitch, QueuedMessage, ReasoningEffort,
|
||||
RedactionGateNotice, ScreenMode, StatusToast, StatusToastLevel, SubmitDisposition,
|
||||
TaskPanelEntry, TaskPanelEntryKind, ToolEvidence, TuiOptions, bound_agent_activity_text,
|
||||
is_stop_word, looks_like_slash_command_input, shell_command_from_bang_input,
|
||||
};
|
||||
use super::approval::{
|
||||
ApprovalMode, ApprovalRequest, ApprovalView, ElevationRequest, ElevationView, ReviewDecision,
|
||||
|
||||
@@ -537,11 +537,17 @@ fn statusline_full_frame_presets_preserve_transcript_composer_and_hitboxes() {
|
||||
terminal
|
||||
.backend_mut()
|
||||
.assert_cursor_position(Position::from(cursor));
|
||||
let painted_input = (text.x..cursor.0)
|
||||
.map(|x| terminal.backend().buffer()[(x, cursor.1)].symbol())
|
||||
.collect::<String>();
|
||||
// Ratatui represents each CJK continuation cell as a blank cell.
|
||||
assert_eq!(painted_input.replace(' ', ""), "ab中文", "{evidence}");
|
||||
// Terminal backends skip continuation cells covered by a wide
|
||||
// glyph. TestBackend can retain a prior border in those cells;
|
||||
// it is not visible terminal text after the wide glyph is drawn.
|
||||
let mut painted_input = String::new();
|
||||
let mut x = text.x;
|
||||
while x < cursor.0 {
|
||||
let symbol = terminal.backend().buffer()[(x, cursor.1)].symbol();
|
||||
painted_input.push_str(symbol);
|
||||
x += unicode_width::UnicodeWidthStr::width(symbol).max(1) as u16;
|
||||
}
|
||||
assert_eq!(painted_input, "ab中文", "{evidence}");
|
||||
app.viewport.composer_click_trace = None;
|
||||
assert!(crate::tui::mouse_ui::handle_composer_mouse(
|
||||
&mut app,
|
||||
@@ -602,11 +608,23 @@ fn statusline_full_frame_presets_preserve_transcript_composer_and_hitboxes() {
|
||||
}
|
||||
}
|
||||
if name == "full-restored" {
|
||||
assert_eq!(
|
||||
terminal.backend().buffer(),
|
||||
&full_buffer,
|
||||
"restoration leaves no stale painted row or style"
|
||||
);
|
||||
let restored = terminal.backend().buffer();
|
||||
assert_eq!(restored.area, full_buffer.area);
|
||||
for y in full_buffer.area.y..full_buffer.area.bottom() {
|
||||
let mut x = full_buffer.area.x;
|
||||
while x < full_buffer.area.right() {
|
||||
let expected = &full_buffer[(x, y)];
|
||||
assert_eq!(
|
||||
&restored[(x, y)],
|
||||
expected,
|
||||
"restoration leaves no stale visible cell or style at ({x}, {y}): {evidence}"
|
||||
);
|
||||
// Covered continuation cells are not rendered by a
|
||||
// terminal, so TestBackend's retained contents there
|
||||
// are not part of visible restoration.
|
||||
x += unicode_width::UnicodeWidthStr::width(expected.symbol()).max(1) as u16;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -781,6 +781,28 @@ an incomplete admission before a later lookup, but GET itself never does so.
|
||||
- `POST /v1/approvals/{approval_id}` with body
|
||||
`{ "decision": "allow" | "deny", "remember": false }`
|
||||
|
||||
`approval_id` is minted by the Runtime, not by the model or the provider. It is
|
||||
an opaque `approval_<32 hex>` capability, unique per prompt, bound to the thread
|
||||
that raised it, and single-use: the Runtime removes it when the decision is
|
||||
delivered, when the prompt times out, or when the turn abandons it. Clients echo
|
||||
the value they were given and must not construct, derive, or guess one.
|
||||
|
||||
It is deliberately **not** the provider's tool-call ID. Providers restart their
|
||||
call-ID counters per response, so two threads can gate calls whose raw IDs are
|
||||
byte-equal; keying approvals by that value let one thread's decision settle
|
||||
another thread's call. The endpoint therefore performs one exact match on the
|
||||
minted ID and has no fallback: a raw tool-call ID, an expired ID, or a replayed
|
||||
ID that has already been settled all return `404` and reach no engine. A `404`
|
||||
means the capability is not pending — it is not evidence about how the approval
|
||||
was resolved; read `approval.decided` for that.
|
||||
|
||||
The raw provider call ID travels separately as `tool_call_id` on
|
||||
`pending_approvals[]` and on the approval events. It is a correlator for
|
||||
attaching a prompt to the tool row it gates, and never accepted as a decision.
|
||||
Each thread-detail `pending_approvals[]` entry is
|
||||
`{ "id", "turn_id", "tool_name", "description", "intent_summary"?, "tool_call_id"? }`,
|
||||
where `id` is the capability above.
|
||||
|
||||
**User input**
|
||||
- `POST /v1/user-input/{thread_id}/{input_id}` with body
|
||||
`{ "answers": [{ "id": "question-id", "label": "Choice", "value": "Choice" }] }`
|
||||
@@ -1239,6 +1261,22 @@ cursor include the same materialized prefix.
|
||||
execution-policy rule caused the prompt. This field is explanatory metadata for
|
||||
clients and does not grant or persist permissions.
|
||||
|
||||
`approval.required`, `approval.decided`, and `approval.timeout` carry two
|
||||
distinct identifiers. `approval_id` is the Runtime-minted, single-use capability
|
||||
described under **Approvals** — the only value `POST /v1/approvals/{id}` accepts
|
||||
— and `approval.required` also repeats it in the legacy `id` field for older
|
||||
clients. `tool_call_id` is the provider's raw tool-call ID, present for
|
||||
correlation only. Automatically resolved prompts (thread `auto_approve`, and the
|
||||
Auto-Review posture, which never opens a modal) mint an `approval_id` as well, so
|
||||
the field has one meaning on every path; those IDs register no waiter and are
|
||||
inert against the endpoint. Clients must never treat `tool_call_id` as an
|
||||
approval capability or assume it is unique across threads.
|
||||
|
||||
The thread event stream forwards these payloads intact. The compatibility turn
|
||||
stream carries `approval_id`, its `id` alias and `tool_call_id`; the pending
|
||||
snapshot carries the same capability and correlator so reconnecting clients can
|
||||
attach an approval prompt to its tool row.
|
||||
|
||||
## Security boundary
|
||||
|
||||
- **Localhost by default**. The server binds to `127.0.0.1` by default.
|
||||
|
||||
Reference in new issue
Block a user