From 895e2344ca49c2f5d9bb4a4e48c52ae81ca7ec0d Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 02:57:06 +0800 Subject: [PATCH 1/6] fix: add activation hints to rlm handle_read text Signed-off-by: asto18089 --- crates/tui/src/tools/rlm.rs | 195 +++++++++++++++++++++++++----------- 1 file changed, 139 insertions(+), 56 deletions(-) diff --git a/crates/tui/src/tools/rlm.rs b/crates/tui/src/tools/rlm.rs index 86e6b89f3a..1599b35fd6 100644 --- a/crates/tui/src/tools/rlm.rs +++ b/crates/tui/src/tools/rlm.rs @@ -28,6 +28,7 @@ use crate::tools::handle::VarHandle; use crate::tools::spec::{ ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, }; +use crate::tools::subagent::HANDLE_READ_ACTIVATION_HINT; const DEFAULT_CHILD_MODEL: &str = "deepseek-v4-flash"; const MAX_INLINE_CONTENT_CHARS: usize = 200_000; @@ -73,19 +74,87 @@ pub struct RlmTool { /// Kept only for replay-compatible explicit RLM sessions. New normal /// agent work uses the session kernel and inherits its route there. root_model: String, + /// Precomposed model-facing description. The `eval` and family surfaces + /// pair stored handles with `handle_read`, so they embed the shared + /// activation hint and cannot stay `&'static` (Pinvou #534). + description: String, } impl RlmTool { - #[must_use] - pub fn new(name: &'static str, client: Option) -> Self { + fn compose( + name: &'static str, + forced_action: Option<&'static str>, + client: Option, + ) -> Self { Self { name, - forced_action: None, + forced_action, client, root_model: DEFAULT_CHILD_MODEL.to_string(), + description: Self::compose_description(forced_action), } } + /// Model-facing description for this surface. Descriptions that pair + /// stored handles with `handle_read` must carry the shared activation + /// hint: `handle_read` is deferred on stock hosts, so model-facing text + /// must teach the `tool_search` activation path instead of commanding a + /// tool absent from the first-turn catalog (Pinvou #490 class, #534). + fn compose_description(forced_action: Option<&str>) -> String { + match forced_action { + Some("session_objects") => { + "List active prompt/history/session symbolic objects as compact cards. \ + Pass one of the returned `id` values to `rlm_open` as \ + `session_object` to inspect it inside an RLM REPL without copying the \ + full prompt or transcript into the parent context." + .to_string() + } + Some("open") => "Open a persistent RLM context. Loads `file_path`, `content`, `url`, \ + or `session_object` into a named Python kernel and returns only \ + metadata: name, length, preview, and sha256. Use this for large or \ + unfamiliar inputs so the parent transcript holds a handle, not the \ + body." + .to_string(), + Some("eval") => format!( + "Run one Python REPL block against a named RLM context. Returns a \ + bounded projection of stdout/stderr plus metadata. If the code calls \ + FINAL/finalize, the final value is stored as a var_handle retrievable \ + with handle_read instead of copied unbounded into the parent context. \ + Large stdout/stderr payloads (>1k chars) are also stored as \ + var_handles (returned in stdout_handle / stderr_handle) to keep the \ + parent transcript lean. Batch child helpers require \ + dependency_mode='independent'; use sub_query_sequence or a \ + sequential loop for dependent work; \ + {HANDLE_READ_ACTIVATION_HINT}." + ), + Some("configure") => { + "Configure a named RLM context: output feedback, child query timeout, \ + recursive sub-RLM depth, and explicit session sharing." + .to_string() + } + Some("close") => "Close a named RLM context, tear down its Python kernel, and return \ + usage/lifecycle metadata." + .to_string(), + _ => format!( + "Persistent RLM sessions over large contexts. Actions: \"session_objects\" \ + (list active prompt/history/session symbolic objects as compact cards), \ + \"open\" (load file_path/content/url/session_object into a named Python \ + kernel; returns only metadata so the parent transcript holds a handle, \ + not the body), \"eval\" (run one bounded Python REPL block against a \ + named context; approval required; FINAL/finalize values and large \ + stdout/stderr become var_handles retrievable with handle_read; \ + {HANDLE_READ_ACTIVATION_HINT}), \ + \"configure\" (output feedback, child timeout, sub-RLM depth, session \ + sharing), \"close\" (tear down the kernel and return usage metadata)." + ), + } + } + + #[must_use] + pub fn new(name: &'static str, client: Option) -> Self { + Self::compose(name, None, client) + } + /// Bind an explicit compatibility session to the active parent route. /// This prevents a saved/manual RLM invocation from silently falling back /// to an unrelated legacy child model. @@ -98,12 +167,7 @@ impl RlmTool { #[cfg(test)] #[must_use] pub fn alias(name: &'static str, action: &'static str, client: Option) -> Self { - Self { - name, - forced_action: Some(action), - client, - root_model: DEFAULT_CHILD_MODEL.to_string(), - } + Self::compose(name, Some(action), client) } fn resolve_action<'a>(&'a self, input: &'a Value) -> Result<&'a str, ToolError> { @@ -171,52 +235,8 @@ impl ToolSpec for RlmTool { false } - fn description(&self) -> &'static str { - match self.forced_action { - Some("session_objects") => { - "List active prompt/history/session symbolic objects as compact cards. \ - Pass one of the returned `id` values to `rlm_open` as \ - `session_object` to inspect it inside an RLM REPL without copying the \ - full prompt or transcript into the parent context." - } - Some("open") => { - "Open a persistent RLM context. Loads `file_path`, `content`, `url`, \ - or `session_object` into a named Python kernel and returns only \ - metadata: name, length, preview, and sha256. Use this for large or \ - unfamiliar inputs so the parent transcript holds a handle, not the \ - body." - } - Some("eval") => { - "Run one Python REPL block against a named RLM context. Returns a \ - bounded projection of stdout/stderr plus metadata. If the code calls \ - FINAL/finalize, the final value is stored as a var_handle retrievable \ - with handle_read instead of copied unbounded into the parent context. \ - Large stdout/stderr payloads (>1k chars) are also stored as \ - var_handles (returned in stdout_handle / stderr_handle) to keep the \ - parent transcript lean. Batch child helpers require \ - dependency_mode='independent'; use sub_query_sequence or a \ - sequential loop for dependent work." - } - Some("configure") => { - "Configure a named RLM context: output feedback, child query timeout, \ - recursive sub-RLM depth, and explicit session sharing." - } - Some("close") => { - "Close a named RLM context, tear down its Python kernel, and return \ - usage/lifecycle metadata." - } - _ => { - "Persistent RLM sessions over large contexts. Actions: \"session_objects\" \ - (list active prompt/history/session symbolic objects as compact cards), \ - \"open\" (load file_path/content/url/session_object into a named Python \ - kernel; returns only metadata so the parent transcript holds a handle, \ - not the body), \"eval\" (run one bounded Python REPL block against a \ - named context; approval required; FINAL/finalize values and large \ - stdout/stderr become var_handles retrievable with handle_read), \ - \"configure\" (output feedback, child timeout, sub-RLM depth, session \ - sharing), \"close\" (tear down the kernel and return usage metadata)." - } - } + fn description(&self) -> &str { + &self.description } fn input_schema(&self) -> Value { @@ -354,7 +374,14 @@ impl RlmTool { "session_object": "session://active/system_prompt" } }, - "redaction": "Large tool results and thinking blocks are represented by compact metadata in transcript objects; use returned handles and handle_read for bounded payload projections." + // #534: this note commands `handle_read`, which is deferred on + // stock hosts, so it must teach the activation path too. + "redaction": format!( + "Large tool results and thinking blocks are represented by compact \ + metadata in transcript objects; use returned handles and handle_read \ + for bounded payload projections; \ + {HANDLE_READ_ACTIVATION_HINT}." + ) })) .map_err(|e| ToolError::execution_failed(e.to_string())) } @@ -948,6 +975,62 @@ mod tests { } } + /// rlm descriptions and the session_objects redaction note pair stored + /// handles with `handle_read`, which is deferred on stock hosts; every + /// such model-facing site must carry the `tool_search` activation hint + /// so the model is never commanded to call a tool it cannot see + /// (Pinvou #490 phantom-tool class, #534). + #[test] + fn rlm_handle_read_pairings_teach_activation_hint() { + assert!( + HANDLE_READ_ACTIVATION_HINT.contains("`tool_search`"), + "shared hint must name the activation tool:\n{HANDLE_READ_ACTIVATION_HINT}" + ); + + let eval = RlmTool::alias("rlm_eval", "eval", None) + .description() + .to_string(); + assert!( + eval.contains("with handle_read") && eval.contains(HANDLE_READ_ACTIVATION_HINT), + "eval description must pair handle_read with the activation hint:\n{eval}" + ); + + let family = RlmTool::new("rlm", None).description().to_string(); + assert!( + family.contains("with handle_read") && family.contains(HANDLE_READ_ACTIVATION_HINT), + "family description must pair handle_read with the activation hint:\n{family}" + ); + + // Surfaces that never name `handle_read` stay hint-free: the hint + // exists only where a pairing would command the deferred tool. + for action in ["session_objects", "open", "configure", "close"] { + let description = RlmTool::alias("rlm_compat", action, None) + .description() + .to_string(); + assert!( + !description.contains("handle_read"), + "{action} description does not pair handle_read and must not name it:\n{description}" + ); + } + } + + #[tokio::test] + async fn rlm_session_objects_redaction_note_teaches_activation_hint() { + let ctx = ctx_with_session_objects(); + let result = RlmTool::alias("rlm_session_objects", "session_objects", None) + .execute(json!({}), &ctx) + .await + .expect("list session objects"); + let body: Value = serde_json::from_str(&result.content).expect("json"); + let redaction = body["redaction"].as_str().expect("redaction note"); + + assert!( + redaction.contains("handle_read") && redaction.contains(HANDLE_READ_ACTIVATION_HINT), + "session_objects redaction note must pair handle_read with the \ + activation hint:\n{redaction}" + ); + } + #[test] fn kernel_failure_result_retains_child_usage_receipt() { let route = crate::cost_status::EffectiveRouteEnvelope::capture( From 9374f63f9881ff060b206c0adbf0f0c47a066af7 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 02:57:06 +0800 Subject: [PATCH 2/6] fix: add the fourth side to the registry cap pin Signed-off-by: asto18089 --- crates/tui/src/core/engine/tool_catalog.rs | 30 ++++++++++++++++++++++ crates/tui/src/tools/mcp_registry.rs | 5 ++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 5a3d6cdd41..7f77577c06 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -205,6 +205,36 @@ pub(super) fn build_model_tool_catalog_with_surface( } 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."; +// The guidance quotes the Registry match cap as a literal "eight", making it +// a fourth side of the compile-time pin in tools/mcp_registry.rs. The pin +// stays local so the private const remains private while drift still fails +// any compile (Pinvou #534). `str::contains` is not const-stable, so the +// substring check is a hand-rolled const byte scan. +const fn str_contains_ascii(haystack: &str, needle: &str) -> bool { + let hay = haystack.as_bytes(); + let pin = needle.as_bytes(); + if pin.is_empty() || hay.len() < pin.len() { + return pin.is_empty(); + } + let mut start = 0; + while start + pin.len() <= hay.len() { + let mut offset = 0; + while offset < pin.len() && hay[start + offset] == pin[offset] { + offset += 1; + } + if offset == pin.len() { + return true; + } + start += 1; + } + false +} + +const _: () = assert!( + str_contains_ascii(REGISTRY_FIRST_SHELL_GUIDANCE, "eight"), + "REGISTRY_FIRST_SHELL_GUIDANCE must keep its \"eight\" wording in sync \ + with MAX_REGISTRY_MATCHES (crates/tui/src/tools/mcp_registry.rs)", +); /// 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 diff --git a/crates/tui/src/tools/mcp_registry.rs b/crates/tui/src/tools/mcp_registry.rs index ef9017bc42..1d8fb4f91b 100644 --- a/crates/tui/src/tools/mcp_registry.rs +++ b/crates/tui/src/tools/mcp_registry.rs @@ -594,8 +594,9 @@ const _: () = assert!( MAX_REGISTRY_MATCHES == 8, "update the \"eight\" wording in \ crates/tui/assets/skills/mcp-discovery/SKILL.md, the registry_sync \ - schema description here, and MCP_REGISTRY_FIRST_INSTRUCTION in \ - core/engine.rs", + schema description here, MCP_REGISTRY_FIRST_INSTRUCTION in \ + core/engine.rs, and REGISTRY_FIRST_SHELL_GUIDANCE in \ + core/engine/tool_catalog.rs", ); #[derive(Serialize)] From ec61720cafd1969767dc7b66318cbbff4413a747 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 02:57:06 +0800 Subject: [PATCH 3/6] test: cover every submission-echo self-start path Signed-off-by: asto18089 --- crates/tui/src/core/engine/tests.rs | 246 +++++++++++++++++++++++++++- 1 file changed, 245 insertions(+), 1 deletion(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 5814a239d2..e607b01466 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -10021,7 +10021,10 @@ async fn forkguard_idle_subagent_completion_self_start_ignores_a_stale_previous_ /// submission started" apart from "an autonomous follow-up overtook it" and /// only ever consume the deferred submit-window stop replay on the former — /// the overtaking order itself is exercised app-side against the forwarder -/// replay gate. +/// replay gate. The three siblings below pin the same `None` echo on the +/// other self-start dispatch paths (composer shell, background shell +/// completion wake, goal continuation), so a refactor cannot swap one of +/// their literal `None`s for a stale token without failing here. #[tokio::test] async fn forkguard_turn_started_echoes_submission_id_self_starts_stay_none() { let workspace = tempdir().expect("tempdir"); @@ -10130,6 +10133,247 @@ async fn forkguard_turn_started_echoes_submission_id_self_starts_stay_none() { task.await.expect("engine task"); } +/// Composer shell commands bypass `handle_send_message` entirely: the engine +/// emits `TurnStarted` directly with a literal `None` because a typed `!` +/// command has no host submission envelope to correlate with. Pins that +/// emission so it cannot learn to echo a stale token (pinvou-agent#254). +#[tokio::test] +async fn forkguard_turn_started_composer_shell_self_start_stays_none() { + let workspace = tempdir().expect("tempdir"); + // The composer shell turn never reaches the model; a client that would + // block forever turns any accidental dispatch into a bounded timeout + // instead of a silent pass. + let client: crate::core::model_client::SharedModelClient = + std::sync::Arc::new(CompleteOnceThenBlockModelClient { + calls: std::sync::atomic::AtomicUsize::new(0), + entered: std::sync::Arc::new(tokio::sync::Notify::new()), + request_dropped: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + }); + let (engine, handle) = Engine::new_with_model_client( + deterministic_engine_config(workspace.path()), + &Config::default(), + client, + ); + let task = tokio::spawn(engine.run()); + handle + .send(Op::RunShellCommand { + command: "echo composer-shell-forkguard".to_string(), + mode: AppMode::Agent, + allow_shell: true, + trust_mode: false, + auto_approve: true, + approval_mode: crate::tui::approval::ApprovalMode::Bypass, + }) + .await + .expect("queue composer shell command"); + { + let mut rx = handle.rx_event.write().await; + loop { + let event = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the composer shell turn") + .expect("engine event"); + if let Event::TurnStarted { submission_id, .. } = event { + assert!( + submission_id.is_none(), + "a composer shell turn has no host submission to echo" + ); + break; + } + } + while let Some(event) = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the composer shell completion") + { + if let Event::TurnComplete { status, error, .. } = event { + assert_eq!(status, TurnOutcomeStatus::Completed, "{error:?}"); + break; + } + } + } + handle.send(Op::Shutdown).await.expect("shutdown engine"); + task.await.expect("engine task"); +} + +/// A finished background shell task wakes the idle engine into a runtime +/// turn (`UserInputProvenance::Runtime`, literal `None` submission id) so the +/// completion reaches the model without waiting for the user to type. Pins +/// the `None` echo on the wake-dispatched turn (pinvou-agent#254). +#[tokio::test] +async fn forkguard_turn_started_background_shell_wake_self_start_stays_none() { + let workspace = tempdir().expect("tempdir"); + // The wake turn is the first provider request, so the canned completion + // lets the whole wake settle without a turn-bound cancel. + let client: crate::core::model_client::SharedModelClient = + std::sync::Arc::new(CompleteOnceThenBlockModelClient { + calls: std::sync::atomic::AtomicUsize::new(0), + entered: std::sync::Arc::new(tokio::sync::Notify::new()), + request_dropped: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + }); + // The wake resolves the route engine-side via `current_runtime_route`, so + // the api config must name a resolvable provider even though the injected + // client answers the request itself. + let api_config = goal_custom_route_config(); + let (engine, handle) = Engine::new_with_model_client( + EngineConfig { + workspace: workspace.path().to_path_buf(), + model: "local-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + ..EngineConfig::default() + }, + &api_config, + client, + ); + let owner_session_id = engine.session.id.clone(); + { + let mut shell = engine.shell_manager.lock().expect("shell manager"); + shell + .execute_with_options_env_for_owner_and_session( + "echo background-shell-wake-forkguard", + None, + 30_000, + true, + None, + false, + None, + std::collections::HashMap::new(), + None, + &owner_session_id, + ) + .expect("start background job"); + } + let deadline = std::time::Instant::now() + Duration::from_secs(30); + loop { + let done = { + let mut shell = engine.shell_manager.lock().expect("shell manager"); + shell.has_finished_unreported_jobs_for_session(&owner_session_id) + }; + if done { + break; + } + assert!( + std::time::Instant::now() < deadline, + "background job never finished" + ); + tokio::time::sleep(Duration::from_millis(25)).await; + } + let task = tokio::spawn(engine.run()); + // The engine idles with finished, unclaimed background work: the wake + // poll self-starts the runtime turn without any host submission, so the + // first TurnStarted of this session is the wake's. + let mut rx = handle.rx_event.write().await; + loop { + let event = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the shell-wake turn") + .expect("engine event"); + if let Event::TurnStarted { submission_id, .. } = event { + assert!( + submission_id.is_none(), + "a background shell completion wake must not present a submission id" + ); + break; + } + } + drop(rx); + handle.send(Op::Shutdown).await.expect("shutdown engine"); + task.await.expect("engine task"); +} + +/// An engine-scheduled or host-injected goal continuation dispatches +/// `handle_send_message` with provenance Runtime and a literal `None` +/// submission id. Pins the `None` echo on the continuation turn so a refactor +/// cannot correlate it with a host submission that never happened +/// (pinvou-agent#254). +#[tokio::test] +async fn forkguard_turn_started_goal_continuation_self_start_stays_none() { + let workspace = tempdir().expect("tempdir"); + // calls starts pre-advanced so every provider request blocks: the + // continuation turn stays parked until the turn-bound cancel lands, + // mirroring the idle sub-agent sibling above. + let client: crate::core::model_client::SharedModelClient = + std::sync::Arc::new(CompleteOnceThenBlockModelClient { + calls: std::sync::atomic::AtomicUsize::new(1), + entered: std::sync::Arc::new(tokio::sync::Notify::new()), + request_dropped: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + }); + let api_config = goal_custom_route_config(); + let (engine, handle) = Engine::new_with_model_client( + EngineConfig { + workspace: workspace.path().to_path_buf(), + model: "local-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + goal_objective: Some("keep the goal continuation warm".to_string()), + goal_continuation_delay_seconds: 0, + ..EngineConfig::default() + }, + &api_config, + client, + ); + engine + .config + .goal_state + .lock() + .expect("goal lock") + .sync_from_host_status( + Some("keep the goal continuation warm"), + None, + crate::tools::goal::GoalStatus::Active, + ); + let task = tokio::spawn(engine.run()); + handle + .send(Op::ContinueGoal { + dynamic_tools: Vec::new(), + engine_schedule_id: None, + }) + .await + .expect("queue goal continuation"); + let continuation_turn_id = { + let mut rx = handle.rx_event.write().await; + loop { + let event = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the goal continuation turn") + .expect("engine event"); + if let Event::TurnStarted { + turn_id, + submission_id, + .. + } = event + { + assert!( + submission_id.is_none(), + "a goal continuation must not present a submission id" + ); + break turn_id; + } + } + }; + // The continuation is parked in its blocked model request; a turn-bound + // cancel by its observed id still lands (unchanged contract) and lets the + // engine task finish. + assert!(handle.cancel_turn( + &continuation_turn_id, + CancelReason::User, + CancelMode::StopDropInbox, + )); + let mut rx = handle.rx_event.write().await; + while let Some(event) = tokio::time::timeout(model_turn_event_timeout(), rx.recv()) + .await + .expect("timed out waiting for the continuation cancellation") + { + if let Event::TurnComplete { status, error, .. } = event { + assert_eq!(status, TurnOutcomeStatus::Interrupted, "{error:?}"); + break; + } + } + drop(rx); + handle.send(Op::Shutdown).await.expect("shutdown engine"); + task.await.expect("engine task"); +} + #[test] fn engine_initial_prompt_includes_configured_goal() { let config = EngineConfig { From c15dac518adcbb88b8ba314d43686760c2d2d390 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 11:21:24 +0800 Subject: [PATCH 4/6] fix(rlm): teach the activation path on the stdout preview and truncation marker The two same-class residuals registered by the first round of this PR are now fixed instead of tracked: the handle-route preview line ("N chars; retrieve via handle_read", emitted on every eval output past the handle threshold) and the preview_output truncation marker both embed the shared HANDLE_READ_ACTIVATION_HINT, each with a runtime pin. Signed-off-by: asto18089 --- crates/tui/src/tools/rlm.rs | 66 +++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/tools/rlm.rs b/crates/tui/src/tools/rlm.rs index 1599b35fd6..4a33dd479c 100644 --- a/crates/tui/src/tools/rlm.rs +++ b/crates/tui/src/tools/rlm.rs @@ -570,7 +570,11 @@ impl RlmTool { let name = format!("{tag}_{}", 0); // single counter is fine let handle = store.insert_text(session_id, name, text); ( - Some(format!("{} chars; retrieve via handle_read", text.len())), + Some(format!( + "{} chars; retrieve via handle_read; \ + {HANDLE_READ_ACTIVATION_HINT}", + text.len() + )), Some(handle), ) } @@ -879,7 +883,8 @@ fn preview_output(text: &str) -> String { .skip(total.saturating_sub(FULL_STDOUT_TAIL_CHARS)) .collect(); format!( - "{head}\n... [{} chars truncated, retrieve via handle_read when returned as a handle] ...\n{tail}", + "{head}\n... [{} chars truncated, retrieve via handle_read when \ + returned as a handle; {HANDLE_READ_ACTIVATION_HINT}] ...\n{tail}", total.saturating_sub(FULL_STDOUT_HEAD_CHARS + FULL_STDOUT_TAIL_CHARS) ) } @@ -1031,6 +1036,63 @@ mod tests { ); } + /// The handle-route preview line ("N chars; retrieve via handle_read") + /// is emitted on every eval output past the handle threshold, so it is + /// the pairing site the model acts on right before calling + /// `handle_read`; it must teach the activation path too (Pinvou #534). + #[tokio::test] + async fn rlm_eval_large_stdout_handle_preview_teaches_activation_hint() { + let ctx = ctx(); + RlmTool::alias("rlm_open", "open", None) + .execute(json!({"name": "wide", "content": "body"}), &ctx) + .await + .expect("open"); + + // 2000 chars of output clears the 1k handle threshold, so the eval + // result carries the handle-route preview line instead of the body. + let eval = RlmTool::alias("rlm_eval", "eval", None) + .execute(json!({"name": "wide", "code": "print('x' * 2000)"}), &ctx) + .await + .expect("eval"); + let eval_json: Value = serde_json::from_str(&eval.content).expect("eval json"); + assert!( + eval_json.get("stdout_handle").is_some(), + "large stdout must be handle-routed: {eval_json}" + ); + let stdout_preview = eval_json["stdout_preview"] + .as_str() + .expect("handle-routed stdout must still carry a preview line") + .replace("\r\n", "\n"); + + assert!( + stdout_preview.contains(" chars; retrieve via handle_read;"), + "handle-route preview line shape:\n{stdout_preview}" + ); + assert!( + stdout_preview.contains(HANDLE_READ_ACTIVATION_HINT), + "preview line must pair handle_read with the activation hint:\n{stdout_preview}" + ); + } + + /// The truncation marker commands `handle_read` the same way; it is + /// currently unreachable through `route_output` (the handle threshold + /// fires first) but the wording must stay correct if that threshold + /// ever rises above head+tail (Pinvou #534). + #[test] + fn rlm_truncation_marker_teaches_activation_hint() { + let body = "x".repeat(FULL_STDOUT_HEAD_CHARS + FULL_STDOUT_TAIL_CHARS + 1); + let preview = preview_output(&body); + + assert!( + preview.contains("chars truncated, retrieve via handle_read"), + "truncation marker shape:\n{preview}" + ); + assert!( + preview.contains(HANDLE_READ_ACTIVATION_HINT), + "truncation marker must pair handle_read with the activation hint:\n{preview}" + ); + } + #[test] fn kernel_failure_result_retains_child_usage_receipt() { let route = crate::cost_status::EffectiveRouteEnvelope::capture( From 5d273b0be5e103b2fa145a115cdb929e3c6d4dc4 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 11:21:59 +0800 Subject: [PATCH 5/6] test(registry): pin the registry_sync description wording and sync the side list The enumeration comment now names all four text sides and both compile-time pins, and the registry_sync tool description gains the missing runtime "eight" pin; the mcp-discovery skill markdown remains the one side reached only by the tripwire message. Signed-off-by: asto18089 --- crates/tui/src/core/engine/tests.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index e607b01466..1c69763ad4 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -450,9 +450,13 @@ fn forkguard_registry_first_instruction_names_registered_tool_specs() { "instruction must keep naming the registered `{start}`" ); // The match cap is quoted as a literal word in the instruction, the - // `registry_sync` schema description, and the bundled mcp-discovery - // skill; `MAX_REGISTRY_MATCHES` pins the constant to it at compile time, - // and these two assertions pin the remaining text sides. + // `registry_sync` schema description and tool description, the bundled + // mcp-discovery skill, and the legacy shell-surface guidance. The + // compile-time pins (`MAX_REGISTRY_MATCHES` in tools/mcp_registry.rs and + // the local "eight" pin in core/engine/tool_catalog.rs) hold the + // constant and the guidance wording, and these assertions pin the + // instruction and registry_sync texts; the skill markdown is only + // reached by the tripwire message, with no mechanical pin. assert!( MCP_REGISTRY_FIRST_INSTRUCTION.contains("eight"), "instruction must keep the match-cap wording in sync with \ @@ -464,6 +468,12 @@ fn forkguard_registry_first_instruction_names_registered_tool_specs() { "registry_sync schema must keep the match-cap wording in sync with \ MAX_REGISTRY_MATCHES: {schema}" ); + let description = ToolSpec::description(&sync); + assert!( + description.contains("eight"), + "registry_sync description must keep the match-cap wording in sync \ + with MAX_REGISTRY_MATCHES: {description}" + ); // Both registry commands are deferred on stock hosts, so the instruction // must teach the `tool_search` activation path instead of only // commanding names the model cannot see yet. From 1c037360dd0b47c541639156139dcdd1a6c15cb1 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 22 Sep 2026 11:22:12 +0800 Subject: [PATCH 6/6] test(forkguard): make the composer-shell accidental-dispatch defense real The composer shell test pre-advances the model-client call counter so every provider request blocks, making the documented bounded-timeout defense true (the first request used to be absorbed by the canned completion); the goal continuation test documents why its host-status sync stays even though the constructor already armed the objective. Signed-off-by: asto18089 --- crates/tui/src/core/engine/tests.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 1c69763ad4..79fa28f852 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -10150,12 +10150,12 @@ async fn forkguard_turn_started_echoes_submission_id_self_starts_stay_none() { #[tokio::test] async fn forkguard_turn_started_composer_shell_self_start_stays_none() { let workspace = tempdir().expect("tempdir"); - // The composer shell turn never reaches the model; a client that would - // block forever turns any accidental dispatch into a bounded timeout - // instead of a silent pass. + // The composer shell turn never reaches the model; calls starts + // pre-advanced so every provider request blocks, turning any accidental + // dispatch into a bounded timeout instead of a silent pass. let client: crate::core::model_client::SharedModelClient = std::sync::Arc::new(CompleteOnceThenBlockModelClient { - calls: std::sync::atomic::AtomicUsize::new(0), + calls: std::sync::atomic::AtomicUsize::new(1), entered: std::sync::Arc::new(tokio::sync::Notify::new()), request_dropped: std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), }); @@ -10322,6 +10322,10 @@ async fn forkguard_turn_started_goal_continuation_self_start_stays_none() { &api_config, client, ); + // Idempotent with the constructor-armed `goal_objective` (same + // objective, already Active); kept because a host-injected continuation + // passes through this sync before the op, and the echo pin must not + // depend on that ordering. engine .config .goal_state