From c5e54935b82782382b31ffda22fe43954dd19622 Mon Sep 17 00:00:00 2001 From: Blomios Date: Mon, 22 Jun 2026 23:46:48 +0200 Subject: [PATCH] feat(orchestrator): durcir le rendez-vous inter-agents + backstop no-reply (Finding A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrige le wedge survenant lorsqu'un agent délégué ne rappelle pas idea_reply : le rendez-vous ask_agent ne reste plus bloqué indéfiniment. Lot 1 — durcissement du préambule de délégation : - input/mod.rs : delegation_preamble renforcé (obligation explicite idea_reply). - agent/lifecycle.rs : préambule injecté conditionné à mcp_enabled. Lot 2 — peuplement du prompt_ready_pattern : - agent/catalogue.rs : .with_prompt_ready_pattern("? for shortcuts") sur le profil Claude + tests de seed, pour calibrer la détection de grâce. Lot 3 — backstop timeout serveur : - mcp/jsonrpc.rs : code d'erreur typé RENDEZVOUS_NO_REPLY (-32001). - mcp/server.rs : mapping timeout → erreur typée, with_ask_rendezvous_timeout promu + resolve_ask_rendezvous_timeout (configurable). - app-tauri/state.rs : lecture env IDEA_ASK_RENDEZVOUS_TIMEOUT_MS. - tests/mcp_server.rs mis à jour, fix d'un test input flaky. Propagation overlay (référence des profils) : - agent/catalogue.rs : overlay_reference_defaults + reference_index + tests. - store/profile.rs : ReferenceBackfillProfileStore + tests. - app-tauri/state.rs : wrap au composition root. - exports mod.rs / lib.rs (application + infrastructure). Tests réels verts : application 494, app-tauri 235, infrastructure 248, mcp_server 22/22, 0 échec. Validation e2e LIVE (rebuild AppImage) en attente avant merge vers develop. Co-Authored-By: Claude Opus 4.8 --- crates/app-tauri/src/state.rs | 23 ++- crates/application/src/agent/catalogue.rs | 133 +++++++++++++++++ crates/application/src/agent/lifecycle.rs | 9 +- crates/application/src/agent/mod.rs | 3 +- crates/application/src/lib.rs | 3 +- crates/infrastructure/src/input/mod.rs | 25 +++- crates/infrastructure/src/lib.rs | 7 +- .../src/orchestrator/mcp/jsonrpc.rs | 9 ++ .../src/orchestrator/mcp/mod.rs | 2 +- .../src/orchestrator/mcp/server.rs | 67 +++++++-- crates/infrastructure/src/store/mod.rs | 2 +- crates/infrastructure/src/store/profile.rs | 138 ++++++++++++++++++ crates/infrastructure/tests/mcp_server.rs | 22 ++- 13 files changed, 414 insertions(+), 29 deletions(-) diff --git a/crates/app-tauri/src/state.rs b/crates/app-tauri/src/state.rs index b753015..890c835 100644 --- a/crates/app-tauri/src/state.rs +++ b/crates/app-tauri/src/state.rs @@ -58,7 +58,8 @@ use infrastructure::{ FsProjectStore, FsProviderSessionStore, FsSkillStore, FsTemplateStore, Git2Repository, HeuristicHandoffSummarizer, IdeaiContextStore, InMemoryConversationRegistry, InMemoryMailbox, LocalFileSystem, LocalProcessSpawner, McpServer, MediatedInbox, NaiveMemoryRecall, - OrchestratorWatchHandle, PortablePtyAdapter, RwFileGuard, StructuredSessionFactory, + OrchestratorWatchHandle, PortablePtyAdapter, ReferenceBackfillProfileStore, RwFileGuard, + StructuredSessionFactory, SystemClock, SystemMillisClock, TokioBroadcastEventBus, TokioScheduler, UuidGenerator, VectorMemoryRecall, DEFAULT_OLLAMA_BASE_URL, ONNX_CACHE_SUBDIR, RECOMMENDED_ONNX_MODELS, VECTOR_HTTP_ENABLED, VECTOR_ONNX_ENABLED, @@ -717,7 +718,14 @@ impl AppState { Arc::clone(&fs_port), app_data_dir.to_string_lossy().into_owned(), )); - let profile_store_port = Arc::clone(&profile_store) as Arc; + // Merge-on-read overlay (finding A) : envelopper le store concret UNE SEULE FOIS + // ici, avant toute injection, pour que chaque lecteur (use cases + `.list()` directs + // de l'orchestrateur) voie les défauts de référence backfillés (`prompt_ready_pattern` + // mesuré). `save`/`delete`/`is_configured` délèguent verbatim ⇒ persistance et + // détection first-run inchangées. + let profile_store_port: Arc = Arc::new( + ReferenceBackfillProfileStore::new(Arc::clone(&profile_store) as Arc), + ); let detect_profiles = Arc::new(DetectProfiles::new(Arc::clone(&runtime_port))); let list_profiles = Arc::new(ListProfiles::new(Arc::clone(&profile_store_port))); @@ -1511,10 +1519,19 @@ impl AppState { service_for_ready.release_agent_cold_start(AgentId::from_uuid(uuid)); } }); + // Borne du rendez-vous `idea_ask_agent` (filet serveur) : réglage projet + // optionnel via `IDEA_ASK_RENDEZVOUS_TIMEOUT_MS`. Absent / invalide / `0` ⇒ + // défaut généreux inchangé (24 h), zéro régression sur les tours longs légitimes. + let ask_timeout = infrastructure::resolve_ask_rendezvous_timeout( + std::env::var("IDEA_ASK_RENDEZVOUS_TIMEOUT_MS") + .ok() + .and_then(|v| v.trim().parse::().ok()), + ); let handle = McpServerHandle::start( McpServer::new(Arc::clone(&self.orchestrator_service), project.clone()) .with_events(events) - .with_ready_sink(ready_sink), + .with_ready_sink(ready_sink) + .with_ask_rendezvous_timeout(ask_timeout), endpoint, listener, project_id, diff --git a/crates/application/src/agent/catalogue.rs b/crates/application/src/agent/catalogue.rs index 35d9022..bdd678d 100644 --- a/crates/application/src/agent/catalogue.rs +++ b/crates/application/src/agent/catalogue.rs @@ -17,6 +17,9 @@ //! so re-deriving the catalogue yields the same id for "claude" every time, //! making the reference profiles addressable across runs without a registry. +use std::collections::HashMap; +use std::sync::OnceLock; + use domain::ids::ProfileId; use domain::permission::ProjectorKey; use domain::profile::{ @@ -67,6 +70,14 @@ pub fn reference_profiles() -> Vec { ) .expect("claude reference profile is valid") .with_structured_adapter(StructuredAdapter::Claude) + // Marqueur de **retour au prompt idle** (signal de repli n°3, agents PTY/TUI ; + // arme le watcher dans `MediatedInbox::bind_handle_with_prompt`). Mesuré en réel + // (capture PTY live, finding A) : le footer idle de Claude Code rend la sous-chaîne + // contiguë `? for shortcuts` à chaque retour au prompt (et au démarrage idle). + // Discriminant : pendant un tour, le footer devient `esc to interrupt` (+ spinner), + // donc cette chaîne n'apparaît JAMAIS en milieu de tour ⇒ pas de faux positif. La + // recherche est littérale sur octets bruts : l'ASCII suffit (pas d'ANSI à embarquer). + .with_prompt_ready_pattern("? for shortcuts") .with_projector(ProjectorKey::Claude) .with_mcp(McpCapability::new( McpConfigStrategy::config_file(".mcp.json") @@ -145,6 +156,106 @@ pub fn selectable_reference_profiles() -> Vec { .collect() } +/// Lazy index `ProfileId -> reference profile`, built once from +/// [`reference_profiles`]. Keyed by the deterministic [`reference_id`], so a stored +/// profile created from a reference is addressable across runs without a registry. +fn reference_index() -> &'static HashMap { + static INDEX: OnceLock> = OnceLock::new(); + INDEX.get_or_init(|| reference_profiles().into_iter().map(|p| (p.id, p)).collect()) +} + +/// **Merge-on-read overlay** (pure, no I/O): backfills *internal* reference fields +/// that a stored profile is missing, without ever persisting anything. +/// +/// Motivation: profiles persisted before a reference field existed (e.g. the measured +/// `prompt_ready_pattern`, finding A) are the launch-time source of truth, but the +/// catalogue is never re-merged into the store. This overlay lets every reader see the +/// up-to-date reference defaults while the on-disk `profiles.json` stays untouched. +/// +/// Invariants: +/// - **Match on the deterministic [`reference_id`] only** — a custom profile (random id) +/// is returned **unchanged** (its id is absent from [`reference_index`]). +/// - **Never clobbers a user value** — a field is filled **iff** it is `None`/absent on +/// the stored profile; a present value always wins. +/// - **Strict allowlist** — only *internal, non-wizard-editable* reference fields are +/// overlaid. Today that is **`prompt_ready_pattern` alone** (an internal measured +/// marker, never surfaced in the wizard). User-editable fields are never touched. +/// - **Idempotent**: `overlay(overlay(p)) == overlay(p)` (a filled field is then +/// present, so the second pass is a no-op). +/// - **Pure**: no I/O, safe to call on every `list()`. +#[must_use] +pub fn overlay_reference_defaults(stored: &AgentProfile) -> AgentProfile { + let Some(reference) = reference_index().get(&stored.id) else { + return stored.clone(); + }; + let mut merged = stored.clone(); + // Allowlist STRICTE — un champ par ligne, rempli uniquement s'il est absent. + if merged.prompt_ready_pattern.is_none() { + merged.prompt_ready_pattern = reference.prompt_ready_pattern.clone(); + } + merged +} + +#[cfg(test)] +mod overlay_tests { + use super::*; + + /// Claude reference profile with `prompt_ready_pattern` cleared — stands in for a + /// profile persisted before the field existed (the case this overlay fixes). + fn claude_without_pattern() -> AgentProfile { + let id = reference_id("claude"); + let mut p = reference_profiles() + .into_iter() + .find(|p| p.id == id) + .expect("claude reference exists"); + p.prompt_ready_pattern = None; + p + } + + #[test] + fn reference_id_with_missing_field_is_backfilled() { + let merged = overlay_reference_defaults(&claude_without_pattern()); + assert_eq!( + merged.prompt_ready_pattern.as_deref(), + Some("? for shortcuts"), + "a reference profile missing the internal field gets it backfilled" + ); + } + + #[test] + fn reference_id_never_clobbers_a_present_user_value() { + let mut stored = claude_without_pattern(); + stored.prompt_ready_pattern = Some("USER OVERRIDE".to_owned()); + let merged = overlay_reference_defaults(&stored); + assert_eq!( + merged.prompt_ready_pattern.as_deref(), + Some("USER OVERRIDE"), + "a present value always wins — never clobbered" + ); + } + + #[test] + fn non_reference_id_is_returned_unchanged() { + // A custom profile (random id absent from the reference index) is untouched, + // even when it is missing the field. + let mut custom = claude_without_pattern(); + custom.id = ProfileId::from_uuid(uuid::Uuid::from_u128(0xC0FFEE)); + let merged = overlay_reference_defaults(&custom); + assert_eq!( + merged.prompt_ready_pattern, None, + "a custom profile is never backfilled (id not a reference id)" + ); + assert_eq!(merged, custom, "custom profile returned byte-for-byte unchanged"); + } + + #[test] + fn overlay_is_idempotent() { + let once = overlay_reference_defaults(&claude_without_pattern()); + let twice = overlay_reference_defaults(&once); + assert_eq!(once, twice, "overlay(overlay(p)) == overlay(p)"); + } +} + #[cfg(test)] mod mcp_tests { use super::*; @@ -157,6 +268,28 @@ mod mcp_tests { .unwrap_or_else(|| panic!("reference profile `{slug}` exists")) } + #[test] + fn claude_seed_carries_the_measured_prompt_ready_pattern() { + // Marqueur de retour-au-prompt idle mesuré en réel (capture PTY live, finding A). + // Recherche littérale d'octets côté watcher ⇒ l'ASCII exact suffit. Non vide ⇒ le + // watcher s'arme (`MediatedInbox::bind_handle_with_prompt`). + assert_eq!( + profile("claude").prompt_ready_pattern.as_deref(), + Some("? for shortcuts"), + "Claude seed must carry the measured idle-prompt marker" + ); + } + + #[test] + fn codex_seed_leaves_prompt_ready_pattern_empty_for_now() { + // Capture Codex non concluante (sandbox) + Codex non utilisé en pratique : pattern + // laissé VIDE (le backstop serveur Lot 3 couvre l'absence de détection). Follow-up. + assert!( + profile("codex").prompt_ready_pattern.is_none(), + "Codex prompt_ready_pattern stays empty until a clean live capture" + ); + } + #[test] fn claude_and_codex_expose_mcp_capability() { for slug in ["claude", "codex"] { diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index cc3d767..dafcf67 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -2750,9 +2750,12 @@ pub(crate) fn compose_convention_file( out.push_str( "Quand tu reçois une tâche déléguée par IdeA (un message préfixé \ `[IdeA · tâche de … · ticket …]`), traite-la puis appelle \ - **impérativement** l'outil `idea_reply(result=…)` pour rendre ton \ - résultat. Ne réponds **jamais** uniquement en texte : seul `idea_reply` \ - débloque l'agent qui t'a sollicité.\n\n", + **impérativement** l'outil `idea_reply(ticket=…, result=…)` pour rendre ton \ + résultat — **même pour une réponse triviale** (un simple `pong`). Ne réponds \ + **JAMAIS** uniquement en texte/prose dans le terminal : tant que tu n'as pas \ + appelé `idea_reply`, l'agent qui t'a sollicité reste **bloqué** et sa réponse \ + ne lui parviendra pas. Terminer ton tour sans `idea_reply` est un échec du \ + protocole, pas une réponse.\n\n", ); // Capacités IdeA (feature skill-awareness) : briefing à HAUTE ALTITUDE, // télégraphique, injecté à CHAQUE lancement (coût token → pas d'exemples ni de diff --git a/crates/application/src/agent/mod.rs b/crates/application/src/agent/mod.rs index 95b0d98..84b26d3 100644 --- a/crates/application/src/agent/mod.rs +++ b/crates/application/src/agent/mod.rs @@ -23,7 +23,8 @@ pub use structured::{ }; pub use catalogue::{ - reference_profile_id, reference_profiles, selectable_reference_profiles, CODEX_SUBMIT_DELAY_MS, + overlay_reference_defaults, reference_profile_id, reference_profiles, + selectable_reference_profiles, CODEX_SUBMIT_DELAY_MS, }; pub use inspect::{InspectConversation, InspectConversationInput, InspectConversationOutput}; pub use lifecycle::{ diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index ad6f406..a65d7b1 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -31,7 +31,8 @@ pub mod window; pub mod workstate; pub use agent::{ - drain_with_readiness, drain_with_readiness_outcome, reference_profile_id, reference_profiles, + drain_with_readiness, drain_with_readiness_outcome, overlay_reference_defaults, + reference_profile_id, reference_profiles, selectable_reference_profiles, send_blocking, AgentResumer, ChangeAgentProfile, ChangeAgentProfileInput, ChangeAgentProfileOutput, ConfigureProfiles, ConfigureProfilesInput, ConfigureProfilesOutput, CreateAgentFromScratch, CreateAgentInput, CreateAgentOutput, diff --git a/crates/infrastructure/src/input/mod.rs b/crates/infrastructure/src/input/mod.rs index eaea177..fc120dc 100644 --- a/crates/infrastructure/src/input/mod.rs +++ b/crates/infrastructure/src/input/mod.rs @@ -875,10 +875,21 @@ impl MediatedInbox { /// The `[IdeA · tâche de · ticket ]` header is the protocol signal /// (cadrage B-5, agent context) that marks the message as an IdeA delegation the /// target must answer via `idea_reply` — echoing `ticket` for multi-thread -/// correlation — rather than as a free-text human prompt. The raw `task` follows on -/// the next line so the agent reads the request unchanged. +/// correlation — rather than as a free-text human prompt. +/// +/// A one-line **imperative reminder** follows the header (durcissement +/// comportemental, finding A) : the single most common wedge is a target that ends +/// its turn with a *prose* answer and never calls `idea_reply`, leaving the asker +/// parked. Reminding it inline — on every delegated task, even trivial ones — attacks +/// that behavioural cause at the central point where the turn is composed (no per-agent +/// duplication). The raw `task` then follows so the agent reads the request unchanged. fn delegation_preamble(requester: &str, ticket: TicketId, task: &str) -> String { - format!("[IdeA · tâche de {requester} · ticket {ticket}]\n{task}") + format!( + "[IdeA · tâche de {requester} · ticket {ticket}]\n\ + ⚠️ Termine IMPÉRATIVEMENT ce tour par un appel `idea_reply(ticket=\"{ticket}\", \ + result=…)`, même pour une réponse triviale. Ne réponds JAMAIS uniquement en prose \ + dans le terminal : sans `idea_reply`, l'agent demandeur reste bloqué.\n{task}" + ) } fn write_delegation_chunks( @@ -1425,8 +1436,14 @@ mod tests { inbox.enqueue(a, ticket(10, &task)); + // Attendre le SUBMIT final (`\r`, écrit après le délai), pas un simple seuil de + // writes : le nombre de chunks de contenu dépend de la taille du payload (préambule + // + tâche), donc seul le `\r` terminal est un point d'arrêt stable. assert!( - wait_until(|| pty.writes.lock().unwrap().len() >= 4), + wait_until(|| { + let w = pty.writes.lock().unwrap(); + w.len() >= 4 && w.last().map(Vec::as_slice) == Some(b"\r".as_slice()) + }), "long headless delivery writes multiple chunks followed by submit" ); let writes = pty.writes.lock().unwrap().clone(); diff --git a/crates/infrastructure/src/lib.rs b/crates/infrastructure/src/lib.rs index bdc6105..5da6b91 100644 --- a/crates/infrastructure/src/lib.rs +++ b/crates/infrastructure/src/lib.rs @@ -50,7 +50,9 @@ pub use id::UuidGenerator; pub use input::{MediatedInbox, MillisClock, SystemMillisClock}; pub use inspector::ClaudeTranscriptInspector; pub use mailbox::InMemoryMailbox; -pub use orchestrator::mcp::{McpServer, MemoryTransport, StdioTransport}; +pub use orchestrator::mcp::{ + resolve_ask_rendezvous_timeout, McpServer, MemoryTransport, StdioTransport, +}; pub use orchestrator::{ process_request_file, FsOrchestratorWatcher, OrchestratorResponse, OrchestratorWatchHandle, REQUESTS_SUBDIR, @@ -75,6 +77,7 @@ pub use store::{ AdaptiveMemoryRecall, EmbedderEnvProbe, FsEmbedderProfileStore, FsEmbedderPromptStore, FsLiveStateStore, FsMemoryStore, FsPermissionStore, FsProfileStore, FsProjectStore, FsSkillStore, FsTemplateStore, HashEmbedder, IdeaiContextStore, NaiveMemoryRecall, - OnnxModelInfo, StubEmbedder, VectorMemoryRecall, DEFAULT_OLLAMA_BASE_URL, ONNX_CACHE_SUBDIR, + OnnxModelInfo, ReferenceBackfillProfileStore, StubEmbedder, VectorMemoryRecall, + DEFAULT_OLLAMA_BASE_URL, ONNX_CACHE_SUBDIR, RECOMMENDED_ONNX_MODELS, VECTOR_HTTP_ENABLED, VECTOR_ONNX_ENABLED, }; diff --git a/crates/infrastructure/src/orchestrator/mcp/jsonrpc.rs b/crates/infrastructure/src/orchestrator/mcp/jsonrpc.rs index 68abb9f..babf697 100644 --- a/crates/infrastructure/src/orchestrator/mcp/jsonrpc.rs +++ b/crates/infrastructure/src/orchestrator/mcp/jsonrpc.rs @@ -121,6 +121,15 @@ pub mod error_codes { pub const INVALID_PARAMS: i32 = -32_602; /// Internal server error (a dispatched IdeA command failed). pub const INTERNAL_ERROR: i32 = -32_603; + + /// **Server-defined** (JSON-RPC reserved range −32000..=−32099): the synchronous + /// `idea_ask_agent` rendezvous expired against the server-side safety net without + /// the target ever rendering a reply. **Distinct** from [`INTERNAL_ERROR`] on + /// purpose — it is not an internal fault but a *typed, retryable* outcome + /// (semantics mirror [`application::AppError::TargetReturnedNoReply`]): the caller + /// may re-ask, reminding the target to answer via `idea_reply`. The accompanying + /// `data` carries `{ "code": "TARGET_RETURNED_NO_REPLY", "retryable": true }`. + pub const RENDEZVOUS_NO_REPLY: i32 = -32_001; } /// Errors a [`Transport`] may surface. diff --git a/crates/infrastructure/src/orchestrator/mcp/mod.rs b/crates/infrastructure/src/orchestrator/mcp/mod.rs index 711622a..86b8fa3 100644 --- a/crates/infrastructure/src/orchestrator/mcp/mod.rs +++ b/crates/infrastructure/src/orchestrator/mcp/mod.rs @@ -36,6 +36,6 @@ pub mod transport; pub use jsonrpc::{ JsonRpcError, JsonRpcRequest, JsonRpcResponse, Transport, TransportError, JSONRPC_VERSION, }; -pub use server::McpServer; +pub use server::{resolve_ask_rendezvous_timeout, McpServer}; pub use tools::{catalogue, map_tool_call, tool_returns_reply, ToolDef, ToolMapError}; pub use transport::{MemoryTransport, StdioTransport}; diff --git a/crates/infrastructure/src/orchestrator/mcp/server.rs b/crates/infrastructure/src/orchestrator/mcp/server.rs index 2e23925..60c1181 100644 --- a/crates/infrastructure/src/orchestrator/mcp/server.rs +++ b/crates/infrastructure/src/orchestrator/mcp/server.rs @@ -46,6 +46,20 @@ const MCP_PROTOCOL_VERSION: &str = "2024-11-05"; /// untouched — they don't rendezvous. const ASK_RENDEZVOUS_TIMEOUT: Duration = Duration::from_secs(24 * 60 * 60); +/// Resolves the effective `idea_ask_agent` rendezvous bound from an optional +/// configured override in **milliseconds** (project/profile level), mirroring the +/// application's [`resolve_turn_timeout`](application) shape: `Some(ms)` ⇒ that bound, +/// `None`/absent ⇒ the generous [`ASK_RENDEZVOUS_TIMEOUT`] default (statu quo, zero +/// regression). `Some(0)` is treated as "no override" so a misconfigured zero can never +/// collapse the safety net to an instant expiry. Pure and unit-testable without wiring. +#[must_use] +pub fn resolve_ask_rendezvous_timeout(override_ms: Option) -> Duration { + match override_ms { + Some(ms) if ms > 0 => Duration::from_millis(u64::from(ms)), + _ => ASK_RENDEZVOUS_TIMEOUT, + } +} + /// The IdeA MCP server: an entry adapter over [`OrchestratorService::dispatch`]. /// /// Cheap to clone the dependencies it holds; one instance serves one project's @@ -138,11 +152,15 @@ impl McpServer { } } - /// **Test seam** (doc-hidden): shrinks the `idea_ask_agent` rendezvous bound - /// (see [`ASK_RENDEZVOUS_TIMEOUT`]) so a wedged-target test need not wait the - /// full production timeout. Doc-hidden so it does not widen the documented public - /// surface; production code never calls it. - #[doc(hidden)] + /// Overrides the `idea_ask_agent` rendezvous safety-net bound (default + /// [`ASK_RENDEZVOUS_TIMEOUT`]). + /// + /// A genuine **configuration knob** (project/profile level, modelled on the + /// application's `turn_timeout_ms` / `resolve_turn_timeout`): the composition root + /// resolves an optional override with [`resolve_ask_rendezvous_timeout`] and applies + /// it here. Absent override ⇒ the generous 24 h default is preserved unchanged (zero + /// regression on legitimately long delegated turns). Tests also use it to shrink the + /// bound to milliseconds. #[must_use] pub fn with_ask_rendezvous_timeout(mut self, timeout: Duration) -> Self { self.ask_rendezvous_timeout = timeout; @@ -401,16 +419,20 @@ impl McpServer { match tokio::time::timeout(self.ask_rendezvous_timeout, dispatch).await { Ok(result) => result, Err(_elapsed) => { + // RISQUE #1 (Architect) : on `return` ici ⇒ le futur `dispatch` (déplacé + // dans `timeout`) est **droppé**, jamais détaché dans un `spawn`. C'est ce + // Drop qui libère le `BusyTurnGuard` RAII de `ask_agent` (cancel_head + + // mark_idle) et purge le `Busy`/ticket de la cible — aucune fuite `Busy`. application::diag!( "[mcp] ask EXPIRED requester={requester_label} target={arg_target} \ timeout_ms={} elapsed_ms={}", self.ask_rendezvous_timeout.as_millis(), started.elapsed().as_millis(), ); - return Err(JsonRpcError::new( - error_codes::INTERNAL_ERROR, - "idea_ask_agent : aucune réponse de la cible avant le timeout du rendezvous", - )); + // Filet serveur : ne plus renvoyer un INTERNAL_ERROR opaque, mais la même + // sémantique typée et **retryable** que `AppError::TargetReturnedNoReply`, + // sous un code JSON-RPC DISTINCT (RENDEZVOUS_NO_REPLY) + `data` structuré. + return Err(rendezvous_no_reply_error(&arg_target)); } } } else { @@ -483,6 +505,33 @@ fn tool_result_text(text: &str, is_error: bool) -> Value { }) } +/// Builds the JSON-RPC error returned when the `idea_ask_agent` rendezvous expires +/// against the server-side safety net (the `Elapsed` branch of the outer `timeout`). +/// +/// Mirrors [`application::AppError::TargetReturnedNoReply`] one-for-one — same message +/// and same machine-readable `code` (`"TARGET_RETURNED_NO_REPLY"`), flagged `retryable` +/// — but as a JSON-RPC protocol error under the **distinct** +/// [`error_codes::RENDEZVOUS_NO_REPLY`] code (never the opaque `INTERNAL_ERROR`), since +/// at expiry no [`OrchestratorOutcome`](application::OrchestratorOutcome) exists to fold +/// into a tool result. The `data` object lets a caller branch without parsing the +/// message. +fn rendezvous_no_reply_error(target: &str) -> JsonRpcError { + let message = format!( + "agent {target} returned to its prompt without calling idea_reply (no answer was \ + rendered) before the rendezvous timeout; retry the request and ensure the agent \ + replies via idea_reply" + ); + JsonRpcError { + code: error_codes::RENDEZVOUS_NO_REPLY, + message, + data: Some(json!({ + "code": "TARGET_RETURNED_NO_REPLY", + "retryable": true, + "target": target, + })), + } +} + /// Maps a [`ToolMapError`] to the right JSON-RPC error code. fn map_err_to_jsonrpc(err: ToolMapError) -> JsonRpcError { match err { diff --git a/crates/infrastructure/src/store/mod.rs b/crates/infrastructure/src/store/mod.rs index 3d9204f..55869b6 100644 --- a/crates/infrastructure/src/store/mod.rs +++ b/crates/infrastructure/src/store/mod.rs @@ -28,7 +28,7 @@ pub use embedder::{ pub use live_state::FsLiveStateStore; pub use memory::{index_token_size, FsMemoryStore, NaiveMemoryRecall}; pub use permission::FsPermissionStore; -pub use profile::{FsEmbedderProfileStore, FsProfileStore}; +pub use profile::{FsEmbedderProfileStore, FsProfileStore, ReferenceBackfillProfileStore}; pub use project::FsProjectStore; pub use skill::FsSkillStore; pub use template::FsTemplateStore; diff --git a/crates/infrastructure/src/store/profile.rs b/crates/infrastructure/src/store/profile.rs index 59f454a..24bb002 100644 --- a/crates/infrastructure/src/store/profile.rs +++ b/crates/infrastructure/src/store/profile.rs @@ -148,6 +148,59 @@ impl ProfileStore for FsProfileStore { } } +/// **Merge-on-read decorator** over any [`ProfileStore`]: backfills *internal* +/// reference defaults that a stored profile is missing, without ever persisting +/// anything (finding A — propagate the measured `prompt_ready_pattern` to profiles +/// saved before that field existed). +/// +/// Only [`list`](ProfileStore::list) is transformed — each returned profile is run +/// through [`application::overlay_reference_defaults`] (pure, allowlisted, never +/// clobbers a user value, idempotent). `save`/`delete`/`is_configured`/ +/// `mark_configured` **delegate verbatim** so persistence and first-run detection are +/// byte-for-byte unchanged. Wrap the concrete [`FsProfileStore`] once at the +/// composition root, before injecting the store into use cases and the orchestrator, +/// so every reader (including direct `.list()` calls) sees the overlaid reads. +pub struct ReferenceBackfillProfileStore { + inner: Arc, +} + +impl ReferenceBackfillProfileStore { + /// Wraps an inner [`ProfileStore`]; reads are overlaid, writes pass through. + #[must_use] + pub fn new(inner: Arc) -> Self { + Self { inner } + } +} + +#[async_trait] +impl ProfileStore for ReferenceBackfillProfileStore { + async fn list(&self) -> Result, StoreError> { + Ok(self + .inner + .list() + .await? + .into_iter() + .map(|p| application::overlay_reference_defaults(&p)) + .collect()) + } + + async fn save(&self, profile: &AgentProfile) -> Result<(), StoreError> { + self.inner.save(profile).await + } + + async fn delete(&self, id: ProfileId) -> Result<(), StoreError> { + self.inner.delete(id).await + } + + async fn is_configured(&self) -> Result { + self.inner.is_configured().await + } + + async fn mark_configured(&self) -> Result<(), StoreError> { + self.inner.mark_configured().await + } +} + // --------------------------------------------------------------------------- // FsEmbedderProfileStore — declarative embedder profiles (`embedder.json`, LOT C). // --------------------------------------------------------------------------- @@ -286,3 +339,88 @@ impl EmbedderProfileStore for FsEmbedderProfileStore { FsEmbedderProfileStore::delete(self, id).await } } + +#[cfg(test)] +mod backfill_tests { + use super::*; + use std::sync::Mutex; + + /// In-memory [`ProfileStore`] fake recording mutating/query calls, so we can prove + /// the decorator transforms ONLY `list` and delegates the rest verbatim. + #[derive(Default)] + struct FakeStore { + profiles: Mutex>, + saved: Mutex>, + deleted: Mutex>, + is_configured_calls: Mutex, + mark_configured_calls: Mutex, + } + + #[async_trait] + impl ProfileStore for FakeStore { + async fn list(&self) -> Result, StoreError> { + Ok(self.profiles.lock().unwrap().clone()) + } + async fn save(&self, profile: &AgentProfile) -> Result<(), StoreError> { + self.saved.lock().unwrap().push(profile.clone()); + Ok(()) + } + async fn delete(&self, id: ProfileId) -> Result<(), StoreError> { + self.deleted.lock().unwrap().push(id); + Ok(()) + } + async fn is_configured(&self) -> Result { + *self.is_configured_calls.lock().unwrap() += 1; + Ok(true) + } + async fn mark_configured(&self) -> Result<(), StoreError> { + *self.mark_configured_calls.lock().unwrap() += 1; + Ok(()) + } + } + + /// Claude reference profile with the internal field cleared — the persisted-before + /// shape the overlay backfills. + fn claude_without_pattern() -> AgentProfile { + let id = application::reference_profile_id("claude"); + let mut p = application::reference_profiles() + .into_iter() + .find(|p| p.id == id) + .expect("claude reference exists"); + p.prompt_ready_pattern = None; + p + } + + #[tokio::test] + async fn list_overlays_reference_defaults() { + let fake = Arc::new(FakeStore::default()); + fake.profiles.lock().unwrap().push(claude_without_pattern()); + let store = ReferenceBackfillProfileStore::new(Arc::clone(&fake) as Arc); + + let listed = store.list().await.expect("list ok"); + assert_eq!(listed.len(), 1); + assert_eq!( + listed[0].prompt_ready_pattern.as_deref(), + Some("? for shortcuts"), + "a stored Claude profile WITHOUT the pattern is backfilled on read" + ); + } + + #[tokio::test] + async fn save_delete_and_config_delegate_verbatim() { + let fake = Arc::new(FakeStore::default()); + let store = ReferenceBackfillProfileStore::new(Arc::clone(&fake) as Arc); + + let p = claude_without_pattern(); + store.save(&p).await.expect("save ok"); + store.delete(p.id).await.expect("delete ok"); + assert!(store.is_configured().await.expect("is_configured ok")); + store.mark_configured().await.expect("mark_configured ok"); + + // save passes the profile through UNCHANGED (no overlay on the write path). + assert_eq!(*fake.saved.lock().unwrap(), vec![p.clone()]); + assert_eq!(*fake.deleted.lock().unwrap(), vec![p.id]); + assert_eq!(*fake.is_configured_calls.lock().unwrap(), 1); + assert_eq!(*fake.mark_configured_calls.lock().unwrap(), 1); + } +} diff --git a/crates/infrastructure/tests/mcp_server.rs b/crates/infrastructure/tests/mcp_server.rs index a98d560..69c1379 100644 --- a/crates/infrastructure/tests/mcp_server.rs +++ b/crates/infrastructure/tests/mcp_server.rs @@ -1247,11 +1247,25 @@ async fn ask_agent_rendezvous_times_out_with_a_jsonrpc_error() { .expect("reply owed"); let error = response.error.expect("a JSON-RPC error on timeout"); - assert_eq!(error.code, error_codes::INTERNAL_ERROR, "got {error:?}"); - assert!( - error.message.contains("timeout"), - "explicit timeout message, got {error:?}" + // Filet serveur : code DISTINCT de l'INTERNAL_ERROR opaque, sémantique typée et + // retryable (miroir de `AppError::TargetReturnedNoReply`). + assert_eq!( + error.code, + error_codes::RENDEZVOUS_NO_REPLY, + "rendezvous timeout maps to the distinct typed code, got {error:?}" ); + assert_ne!( + error.code, + error_codes::INTERNAL_ERROR, + "must NOT be the opaque internal error" + ); + assert!( + error.message.contains("idea_reply"), + "message must steer the caller back to idea_reply, got {error:?}" + ); + let data = error.data.expect("typed data on the rendezvous error"); + assert_eq!(data["code"], "TARGET_RETURNED_NO_REPLY", "got {data:?}"); + assert_eq!(data["retryable"], true, "got {data:?}"); assert!(response.result.is_none(), "error responses carry no result"); }