diff --git a/.ideai/briefs/orchestration-v5-transport-bind-cadrage.md b/.ideai/briefs/orchestration-v5-transport-bind-cadrage.md new file mode 100644 index 0000000..21630cd --- /dev/null +++ b/.ideai/briefs/orchestration-v5-transport-bind-cadrage.md @@ -0,0 +1,215 @@ +# Orchestration v5 — Bind transport S-MCP + fix registre session (cadrage) + +> **Agent Architecture.** Ce document tranche le **dernier kilomètre** de l'orchestration native : (1) le **bind transport** entre une CLI MCP réellement lancée et le serveur MCP par projet (verrou §S-MCP, resté ouvert depuis M3), et (2) le **fix de robustesse du registre de session** (mémoire `session-registry-agent-ambiguity`). Aucun code de production ici : décisions + contrats + découpage en lots testables. +> +> **État du terrain (lu, pas présumé)** : +> - `infrastructure/src/orchestrator/mcp/{server,transport,jsonrpc,tools}.rs` : `McpServer::serve(&mut transport)` boucle ligne-à-ligne sur `Transport::{recv,send}` ; `StdioTransport` (JSON Lines générique), `MemoryTransport` (tests). **`serve` n'est jamais appelé en prod.** +> - `app-tauri/src/state.rs` : `ensure_mcp_server` crée un `McpServer` par projet et le **parke** sur un signal d'arrêt (`McpServerHandle::start` ⇒ `let _server = server; stop_rx.recv().await;`). **Aucun transport, aucun pair.** +> - `application/src/agent/lifecycle.rs` : `apply_mcp_config` matérialise la conf MCP (`ConfigFile`/`Flag`/`Env`) dans le run dir isolé, **après** `apply_injection`, **avant** spawn/`factory.start`. `mcp_server_declaration` écrit un placeholder `{"command":"idea","args":["mcp-server"],"transport":"stdio|socket"}`. +> - `application/src/terminal/registry.rs` : `TerminalSessions` (PTY) + `StructuredSessions` (IA) + agrégateur `LiveSessions`. Invariant **« 1 session vivante/agent »**. +> - `application/src/error.rs` : `AppError::AgentAlreadyRunning { agent_id, node_id }` + code `AGENT_ALREADY_RUNNING` — **défini mais jamais levé** (le garde de `LaunchAgent` rebind/idempotent au lieu d'échouer). +> - `app-tauri/src/commands.rs::list_live_agents` lit **seulement** `terminal_sessions.live_agents()` — **aveugle aux sessions structurées**. + +--- + +## 0. Synthèse exécutive (décisions tranchées) + +1. **Transport S-MCP = `stdio-spawn`** : la CLI **spawn elle-même** un process serveur MCP fourni par IdeA. Ce sous-process est un **client de boucle de retour** (loopback) vers le process Tauri, pas un second `OrchestratorService`. Justification : c'est le **seul** modèle réellement supporté par Claude Code / Codex (déclaration `.mcp.json` `{command,args}`), **cross-OS sans port réseau** (compatible AppImage / Windows / SSH-remote), et il **résout le point dur** « comment le process serveur retrouve le bon projet » par **injection d'identité à l'`args`/`env`** au `LaunchAgent` (le projet est connu à ce moment-là). Le socket est **rejeté** comme défaut (ports/permissions/cross-OS) mais **gardé en TODO** derrière le même trait `Transport`. + +2. **Le binaire `idea mcp-server` est un sous-commande du binaire app-tauri existant** (pas un nouveau crate/binaire à distribuer séparément) : `main.rs` route `argv[1] == "mcp-server"` vers un mode **headless loopback** qui lit stdin/stdout (JSON Lines = `StdioTransport`) et **relaye** chaque message JSON-RPC au process IdeA principal via un **canal de loopback local** (Unix socket / Windows named pipe **par projet**, créé par `ensure_mcp_server`, chemin passé en `--endpoint`). Le `McpServer` (qui tient l'`OrchestratorService`) **vit dans le process Tauri** ; le sous-process `mcp-server` n'est qu'un **pont stdio↔loopback** ultraléger. Un seul binaire à livrer (AppImage/setup.exe). + +3. **Contrat conf↔serveur de bout en bout** rendu **cohérent** : `apply_mcp_config` (M1) écrit une déclaration qui pointe **exactement** vers ce que `ensure_mcp_server` (M3) a mis à l'écoute — `command = `, `args = ["mcp-server", "--endpoint", , "--project", ]`. Fin du placeholder. + +4. **Fix registre session = lot PRIORITAIRE et INDÉPENDANT du bind** (peut/doit partir en premier) : l'invariant correct est **« 1 session vivante par agent »** (décision produit verrouillée, mémoire `session-registry-agent-ambiguity` : un agent est un **singleton**, pas N instances). Le fix n'invente **pas** d'identité par cellule ; il **durcit l'invariant** sur les **deux** registres et **réconcilie les `layouts.json` à doublons**. Trois trous concrets à boucher (cf. §3). + +5. **Robustesse `ask`** : cible morte ⇒ lancement structuré puis envoi ; cible PTY brut ⇒ `Invalid` explicite (déjà fait) ; **cible occupée par un autre tour** ⇒ sérialisation **FIFO par agent** (nouveau, §4) ; timeout 300 s ⇒ cible **vivante**, erreur typée (déjà fait). Deux `ask` simultanés sur la même cible ⇒ file, **jamais** d'entrelacement de tours. + +6. **Frontières** : le pilotage de `serve` vit dans **l'adapter infra** (`McpServer` + une boucle **par connexion** sur le loopback du projet), supervisé par `McpServerHandle` (app-tauri) qui ne fait qu'**accepter les connexions** et spawn une tâche `serve` par pair. Aucune logique applicative ne fuit : le sous-process `mcp-server` ne connaît que des octets JSON-RPC ; `OrchestratorService` ignore tout du transport. + +--- + +## 1. Décision 1 — Modèle de transport S-MCP + +### 1.1 Le point dur, posé proprement + +Une CLI MCP (Claude Code, Codex) attend une déclaration de serveur de forme **`{ "command": "...", "args": [...] }`** : à l'`initialize`, **elle spawn ce process** et parle **JSON-RPC sur son stdin/stdout**. Donc le « serveur » qu'elle voit est **un process enfant à elle**, distinct du process Tauri. C'est le cœur du problème : ce process enfant **n'a pas** l'`Arc` du projet (use cases, registres de sessions, event bus vivent dans Tauri). + +Deux familles de solutions : + +| | **stdio-spawn (RETENU)** | socket (rejeté en défaut) | +|---|---|---| +| Ce que la CLI spawn | un **pont** `idea mcp-server` (stdio↔loopback) | rien — elle se connecte à un serveur déjà à l'écoute | +| Où vit l'`OrchestratorService` | process Tauri (le pont relaie) | process Tauri (écoute directe) | +| Cross-OS / AppImage | ✅ pipes stdio + loopback local (Unix socket / named pipe) | ⚠️ port TCP (firewall/permissions) ou socket — déclaration CLI variable | +| Retrouver le bon projet | **`--endpoint`/`--project` à l'`args`**, fixés au `LaunchAgent` (projet connu) | l'adresse encode le projet, mais la CLI doit la connaître | +| Compat Claude/Codex | ✅ `{command,args}` natif | ❓ support socket inégal selon CLI/version | +| Cycle de vie | la CLI **possède** le pont (meurt avec elle) | serveur long-vécu, connexions multiplexées | + +### 1.2 Pourquoi stdio-spawn, malgré le sous-process en plus + +- **Compatibilité réelle** : `{command,args}` est le **dénominateur commun** des CLIs MCP. Le socket n'est pas universellement déclarable. +- **Cross-OS sans port réseau** : le **loopback local** entre le pont et Tauri est un **Unix domain socket** (Linux/macOS) ou un **named pipe** (Windows) — déjà la techno la plus portable pour de l'IPC local, **sans firewall ni permission réseau**, donc **AppImage-safe** et **SSH-remote-safe** (le pont tourne côté machine de l'agent). +- **Résolution du point dur par injection d'identité** : le projet est **connu** au moment du `LaunchAgent` (c'est lui qui écrit la conf MCP). On **encode** `--endpoint ` (+ `--project ` en garde-fou) dans les `args` de la déclaration. Le pont n'a **rien à deviner** : il se connecte à l'endpoint du **bon** projet. Le `McpServer` côté Tauri, lui, **est** déjà attaché à cet `OrchestratorService`/`Project` (créé par `ensure_mcp_server`). + +### 1.3 Le pont `idea mcp-server` (sous-commande du binaire existant) + +- **Pas de nouveau binaire distribué** : `main.rs` détecte `argv[1] == "mcp-server"` **avant** d'initialiser Tauri/WebKit, et bascule en **mode headless pont**. Un seul exécutable livré (AppImage / setup.exe). +- **Rôle du pont** : `StdioTransport(stdin, stdout)` côté CLI ; un client de loopback côté Tauri. Boucle : lire une ligne JSON-RPC de la CLI → l'écrire sur le loopback → lire la réponse du loopback → l'écrire sur stdout. **Zéro logique métier** : c'est un tube transparent. (Optionnellement, le pont peut **directement** porter le `McpServer` si l'`OrchestratorService` était accessible — il ne l'est pas inter-process — d'où le relais.) +- **Côté Tauri** : `ensure_mcp_server` crée **l'endpoint loopback du projet** (socket/pipe), et `McpServerHandle` **accepte** les connexions ; **chaque connexion** (= un pont = un agent) ⇒ une **tâche `McpServer::serve(&mut conn_transport)`** où `conn_transport` enveloppe le flux loopback. `McpServer` est **déjà** branché à l'`OrchestratorService` du projet. + +### 1.4 Identité de l'appelant (lève le `requester_id = "mcp"` figé) + +`server.rs::publish_processed` tague aujourd'hui `requester_id: "mcp"` (placeholder). Avec stdio-spawn, le pont **connaît l'agent** (le `LaunchAgent` peut injecter `--requester ` dans les `args` de la déclaration, comme `--project`). Le pont passe cette identité dans la **poignée de connexion** (premier message de handshake loopback, hors JSON-RPC MCP), et `McpServer::serve` la porte dans son contexte de connexion ⇒ `OrchestratorRequestProcessed.requester_id` devient l'**agent réel**. Observabilité UI exacte (qui a délégué à qui). + +### 1.5 Socket = TODO derrière le même trait + +Le trait `Transport` (jsonrpc.rs) **isole** déjà le serveur du transport. Un `SocketTransport` (TCP/HTTP-stream) reste un **ajout sans toucher `McpServer`** si une CLI l'exige. Non requis pour Claude/Codex ⇒ **hors périmètre v5**, documenté. + +--- + +## 2. Décision 2 — Contrat conf injectée (M1) ↔ serveur écouté (M3/v5) + +Bout en bout, **un seul chemin** : + +``` +profil.mcp = Some(McpCapability{ config: ConfigFile{".mcp.json"} | Flag{..} | Env{..}, transport }) + │ (LaunchAgent, run dir isolé .ideai/run// ; après apply_injection, avant spawn) + ▼ +apply_mcp_config écrit la DÉCLARATION RÉELLE (fin du placeholder) : + { + "mcpServers": { + "idea": { + "command": "", ← std::env::current_exe() + "args": ["mcp-server", + "--endpoint", "", ← fourni par ensure_mcp_server + "--project", "", + "--requester",""] ← identité (§1.4) + } + } + } + │ (la CLI lit .mcp.json à l'initialize) + ▼ +la CLI SPAWN mcp-server --endpoint … --project … --requester … + │ (pont stdio↔loopback) + ▼ +le pont se connecte au LOOPBACK DU PROJET (créé par ensure_mcp_server) + │ (handshake : project id + requester id) + ▼ +McpServerHandle ACCEPTE ⇒ tâche McpServer::serve(conn) [McpServer tient l'OrchestratorService du projet] + │ tools/call → map_tool_call → OrchestratorCommand → OrchestratorService::dispatch(&project, cmd) + ▼ +réponse inline (idea_ask_agent ⇒ outcome.reply) renvoyée verbatim à la CLI +``` + +**Invariant de cohérence à tester** : le **chemin de l'endpoint** et l'**exe** écrits par `apply_mcp_config` sont **exactement** ceux que `ensure_mcp_server` met à l'écoute pour ce projet. Source de vérité **unique** : une fonction (app-tauri) calcule l'endpoint d'un `ProjectId` ; M1 (qui écrit la conf) et M3/v5 (qui écoute) l'appellent tous deux. **Pas** de chaîne dupliquée. + +**Le `transport` du profil** reste surfacé dans la déclaration pour la voie socket future, mais en stdio-spawn il est **implicite** (la CLI spawn = stdio). `McpConfigStrategy` inchangé : `ConfigFile` écrit le fichier, `Flag`/`Env` passent le **chemin de la conf** (run dir) — sémantique déjà en place, on ne fait que **remplir** la déclaration de vrai contenu. + +--- + +## 3. Décision 3 — Fix du registre de session (lot PRIORITAIRE, indépendant) + +### 3.1 Invariant correct (verrouillé) + +**1 session vivante par agent** (un agent = singleton : un `.md`, une conversation, un run dir, une mémoire). On **ne** modélise **pas** une identité par cellule. La **cellule est une vue** rebindable (§17.6). `session_for_agent` est donc **déterministe par construction** — à condition que l'invariant soit **réellement enforced** et que les **deux registres** soient considérés. Aujourd'hui il y a **trois fuites** : + +### 3.2 Trou A — le garde de `LaunchAgent` ne lève jamais `AgentAlreadyRunning` + +`LaunchAgent::execute` (lifecycle.rs ~877-920) : si l'agent a déjà une session vivante (PTY **ou** structurée) et qu'un `node_id` est demandé, il **rebind silencieusement** ; sans node, il **rend la session existante** (idempotent). C'est juste pour **réattacher une vue**, mais cela **masque** un vrai second lancement (deux cellules distinctes voulant le **lancer** chacune). `AppError::AgentAlreadyRunning` est **défini mais jamais levé**. + +**Décision** : distinguer **réattache de vue** (légitime, rebind) de **second lancement** (à refuser). Le signal de discrimination existe déjà dans le flux : un `LaunchAgentInput` issu d'une **réattache** (la cellule sait que l'agent tournait : `agent_was_running`/`conversation_id` présents) vs un **lancement neuf**. Le garde lève `AgentAlreadyRunning { agent_id, node_id_hôte }` quand un lancement **neuf** vise un agent **déjà vivant sur un autre node**, et **rebind** seulement quand le `node_id` demandé **est** le node hôte (ou réattache explicite). L'orchestrateur `spawn_agent` (`Visible{node_id}`) suit la **même** règle. + +### 3.3 Trou B — `list_live_agents` est aveugle aux sessions structurées + +`commands.rs::list_live_agents` ⇒ `state.terminal_sessions.live_agents()` **seulement**. Un agent **chat** (structuré) vivant n'apparaît **pas** ⇒ l'UI ne le désactive pas dans le dropdown ⇒ on peut tenter de le relancer ailleurs. + +**Décision** : la commande lit l'**agrégateur** `LiveSessions::live_agents()` (PTY **+** structuré), déjà présent dans le registre. Un seul point de vérité de liveness pour l'UI. + +### 3.4 Trou C — `layouts.json` à doublons (deux feuilles, même `agent`) + +Les layouts persistés **contiennent déjà** des feuilles en double sur le même `agent` id (constaté). À l'ouverture, **ne pas auto-lancer la 2ᵉ** ; **réconcilier** : une seule feuille reste « hôte vivant », les autres sont des vues mortes (pas de relance, pas de bannière fraîche). C'est précisément ce qui causait le **symptôme** (« une cellule reset au retour d'onglet »). + +**Décision** : étape de **réconciliation à l'ouverture du projet** (app-tauri, jumelle de `SnapshotRunningAgents`) : pour chaque agent apparaissant sur N feuilles, **garder une** hôte, **dé-flagger** `agent_was_running`/`conversation_id` sur les autres feuilles dupliquées. La reprise (B, §15.2) ne relance alors qu'**une** session/agent. + +### 3.5 Pourquoi indépendant du bind + +Aucun de ces trois trous ne touche MCP/transport : ils vivent dans `LaunchAgent`, `list_live_agents`, et l'ouverture de projet. Le fix **stabilise le routage de `ask`** (qui s'appuie sur `session_for_agent`) **avant** d'ouvrir la vanne MCP ⇒ on évite de débugger un `ask` mal routé **et** un transport neuf en même temps. **⇒ Lot R0, livré en premier.** + +--- + +## 4. Décision 4 — Robustesse `ask` (sérialisation FIFO par agent) + +Sémantique cible (au-dessus de l'existant) : + +| Situation | Comportement | +|---|---| +| Cible inconnue | `AppError::NotFound` (fait) | +| Cible morte | lancement structuré (background) puis envoi (fait) | +| Cible vivante en **PTY brut** | `AppError::Invalid` explicite, jamais d'ACK trompeur (fait) | +| Cible vivante structurée, **libre** | rendez-vous direct `send_blocking` (fait) | +| Cible vivante structurée, **déjà en tour** (autre `ask`) | **file FIFO par agent** : le 2ᵉ `ask` attend son tour, **pas** d'entrelacement (NOUVEAU) | +| Timeout 300 s | cible **vivante**, erreur typée `Timeout`, retry possible (fait) | + +**Le seul vrai manque = la concurrence.** Deux `ask` simultanés sur la même cible appelleraient `send_blocking` **en parallèle** sur la **même** `AgentSession` ⇒ deux tours entrelacés sur un moteur qui pilote **une** conversation. Inacceptable (cf. bug accents = writes non sérialisés). + +**Décision** : **sérialiser les tours par agent** dans `OrchestratorService::ask_agent` via un **verrou par `agent_id`** (un `Mutex`/sémaphore d'unité, registre `HashMap>>` détenu par le service, ou porté par l'entrée de `StructuredSessions`). Un `ask` **acquiert** le verrou de l'agent avant `send_blocking`, le **relâche** après le `Final`/timeout. Les `ask` concurrents forment une **file naturelle** (ordre d'acquisition). Le timeout s'applique **au tour** (pas à l'attente du verrou) — ou un timeout global borné l'attente totale (décision : timeout **par tour** ; l'attente en file ne consomme pas le budget, mais un plafond d'attente évite l'inanition). Cohérent avec « 1 conversation déterministe/agent ». + +**Erreurs typées remontées aux deux portes** : MCP ⇒ `tool_result_text(.., isError=true)` (déjà) ; fichier ⇒ `*.response.json` avec le champ d'erreur (déjà via `OrchestratorOutcome`/watcher). Le verrou n'ajoute pas de nouveau type d'erreur ; un timeout d'attente en file ⇒ `Timeout` (même type). + +--- + +## 5. Décision 5 — Frontières hexagonales (où vit `serve`) + +- **`McpServer::serve` = adapter infra**, piloté **par connexion** : `McpServerHandle` (app-tauri) **accepte** sur l'endpoint loopback du projet et, **par pair connecté** (= un agent), **spawn une tâche** `serve(conn)`. Le serveur reste **sans état de connexion** au-delà de l'identité du pair (portée par le contexte de connexion, §1.4). +- **`McpServerHandle` évolue** : aujourd'hui il **parke** (`let _server; stop_rx.recv()`). Demain il **ouvre l'endpoint** + boucle d'`accept` ; à l'arrêt, ferme l'endpoint (les ponts enfants meurent avec leur CLI). **Toujours** non-bloquant pour open/close projet (l'`accept` est async, parqué sur l'absence de pair). +- **Le sous-process `mcp-server`** vit dans `app-tauri` (route `main.rs`), mais ne connaît que **stdio + loopback + JSON brut** : **zéro** `OrchestratorService`, zéro use case. Frontière nette. +- **Aucune fuite applicative dans l'infra** : `OrchestratorService::dispatch` est appelé **à l'identique** par les trois portes (fichier, MCP, UI). Le verrou par agent (§4) est une **règle applicative** ⇒ il vit dans `OrchestratorService` (ou le registre `StructuredSessions`), **pas** dans l'adapter MCP. + +--- + +## 6. Découpage en LOTS testables (méthode §3) + +> Ordre global : **R0 (fix registre) d'abord** — indépendant, débloque la robustesse de `ask`. Puis **bind transport M5x**. Front (M5-UI) optionnel en fin. + +### Bloc R — Fix registre session (PRIORITAIRE, indépendant du transport) + +| Lot | Côté | Périmètre | Critères de test | +|---|---|---|---| +| **R0a** | back (application) | Garde `LaunchAgent` : lever `AgentAlreadyRunning{agent_id,node_hôte}` pour un **lancement neuf** ciblant un agent déjà vivant sur **un autre node** (PTY **ou** structuré) ; **rebind** seulement si node demandé = node hôte / réattache. Même règle dans `OrchestratorService::spawn_agent`. | lancement neuf d'un agent vivant ailleurs ⇒ `AgentAlreadyRunning` (code `AGENT_ALREADY_RUNNING`) ; réattache même node ⇒ rebind sans respawn ; idempotence background inchangée ; `ask` d'une cible morte ⇒ lancement OK (pas de faux positif). | +| **R0b** | back (app-tauri) | `list_live_agents` lit `LiveSessions::live_agents()` (PTY **+** structuré). | un agent **chat** vivant apparaît dans la liste ; un agent PTY aussi ; aucun doublon ; sans session ⇒ vide. | +| **R0c** | back (app-tauri) | Réconciliation à l'**ouverture projet** : pour un agent sur N feuilles, garder **une** hôte, dé-flagger `agent_was_running`/`conversation_id` sur les autres. | layout à doublons ⇒ après ouverture, **une seule** feuille « était en cours » pour l'agent ; layout sans doublon **inchangé** ; persistance idempotente (2ᵉ ouverture = no-op). | +| **R0d** | front | Dropdown agent du leaf : désactiver/"déjà placé" via la liste R0b (PTY+chat) ; gérer le retour `AGENT_ALREADY_RUNNING` (aller-à / déplacer). | Vitest : agent vivant ailleurs ⇒ option désactivée + action « aller à la cellule » ; erreur backend mappée à un message clair. | + +### Bloc M5 — Bind transport S-MCP (stdio-spawn) + +| Lot | Côté | Périmètre | Critères de test | +|---|---|---|---| +| **M5a** | back (app-tauri) | **Endpoint loopback par projet** : fonction unique `mcp_endpoint(project_id)` (Unix socket / Windows named pipe) ; `ensure_mcp_server` l'ouvre, `stop_orchestrator_watch` le ferme. | endpoint créé à l'open, supprimé au close ; idempotent (1/projet) ; chemin déterministe par `ProjectId` ; pas de collision inter-projets. | +| **M5b** | back (app-tauri) | **Sous-commande `mcp-server`** dans `main.rs` (avant init Tauri) : pont `StdioTransport(stdin,stdout)` ↔ loopback (`--endpoint`), handshake (`--project`,`--requester`). | `argv mcp-server` ⇒ mode headless, ne lance pas la webview ; relaie une requête JSON-RPC ligne→loopback→réponse→stdout ; EOF stdin ⇒ sortie propre ; endpoint absent ⇒ erreur non-zéro, jamais de hang. | +| **M5c** | back (infra/app-tauri) | **`McpServerHandle` accepte** sur l'endpoint et **spawn `McpServer::serve(conn)` par pair** ; identité du pair (requester) portée au contexte de connexion ⇒ `OrchestratorRequestProcessed.requester_id` = agent réel (fin du `"mcp"` figé). | une connexion ⇒ une tâche serve ; `initialize`/`tools/list`/`tools/call` OK de bout en bout via le loopback (test d'intégration local, hors réseau) ; `requester_id` = l'agent ; déconnexion d'un pair n'affecte pas les autres ; arrêt ferme l'endpoint + termine les serve. | +| **M5d** | back (application) | **`apply_mcp_config` écrit la déclaration RÉELLE** (fin placeholder) : `command = current_exe`, `args = ["mcp-server","--endpoint",mcp_endpoint(project),"--project",id,"--requester",agent]`. `ConfigFile` non-clobbering ; `Flag`/`Env` portent le chemin de conf. **Source d'endpoint partagée avec M5a.** | `mcp=None` ⇒ aucune écriture (inchangé) ; `ConfigFile` ⇒ `.mcp.json` pointe l'exe + endpoint **exacts** du projet ; endpoint identique à `ensure_mcp_server` (test de cohérence M1↔M3) ; non-clobbering. | +| **M5e** | back (intégration) | **Smoke end-to-end loopback** (sans CLI réelle) : un faux pont écrit `tools/call idea_list_agents`/`idea_ask_agent` sur le loopback d'un projet et reçoit la réponse inline du `dispatch` réel. | `idea_list_agents` ⇒ JSON des agents ; `idea_ask_agent` vers une cible structurée ⇒ `reply` inline ; cible PTY ⇒ erreur typée ; JSON-RPC malformé ⇒ erreur, jamais panic ; **hors réseau**. | + +### Bloc A — Robustesse `ask` (concurrence) + +| Lot | Côté | Périmètre | Critères de test | +|---|---|---|---| +| **A0** | back (application) | **Sérialisation FIFO par agent** dans `ask_agent` : verrou par `agent_id` autour de `send_blocking` ; timeout **par tour** ; plafond d'attente en file. | deux `ask` concurrents sur la même cible ⇒ tours **séquentiels** (pas d'entrelacement), ordre FIFO ; un `ask` sur agent A et un sur agent B ⇒ **parallèles** ; timeout d'un tour laisse la cible vivante et **libère** la file ; plafond d'attente ⇒ `Timeout` typé. | + +### Optionnel + +| Lot | Côté | Périmètre | Critères de test | +|---|---|---|---| +| **M5-UI** | front | Badge **source** (`mcp`/`file`) + **requester réel** sur une délégation (réutilise `OrchestratorRequestProcessed`). | badge source correct ; requester = agent réel (plus « mcp ») ; pas de régression sans event. | + +**Ordre recommandé** : **R0a→R0b→R0c→R0d** (stabilise le routage), puis **A0** (concurrence `ask`, ne dépend pas du transport), puis **M5a→M5b→M5c→M5d→M5e** (bind), puis **M5-UI**. R0 et A0 sont livrables **sans** toucher MCP ; M5 ne doit partir qu'**après** R0 (sinon on débugge `ask` mal routé + transport neuf ensemble). + +--- + +## 7. Conformité hexagonale & SOLID (rappel) + +- **Règle de dépendance** : JSON-RPC, stdio, loopback socket/pipe, sous-process `mcp-server` = **infra/app-tauri exclusivement**. `domain`/`application` ignorent MCP et le transport. Le verrou FIFO par agent est une **règle applicative** (vit dans `OrchestratorService`/registre), pas dans l'adapter. +- **S** : `McpServer` = traduire JSON-RPC → `dispatch` ; le **pont** = relayer des octets ; `McpServerHandle` = cycle de vie + `accept` ; le verrou = sérialiser les tours. Aucune responsabilité fourre-tout. +- **O** : un `SocketTransport` futur = un impl de `Transport` en plus, **zéro** modif de `McpServer`. Une CLI MCP de plus = un profil avec `mcp = Some(..)`, zéro code. +- **L** : les trois portes (fichier, MCP, UI) sont substituables derrière `OrchestratorService::dispatch` — **un** comportement applicatif. +- **I** : `McpServerHandle` ne voit que « accepter + servir » ; `OrchestratorService` ne voit que `dispatch` + registres ; l'UI ne voit que `LiveSessions::live_agents`. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a7537fc..dcd7742 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -770,6 +770,40 @@ Exemple `skill.create` : **Ordre** : **M0 → M1 → M2 → M3** (→ M4 optionnel). Chantiers adjacents **déjà livrés** (non prérequis) : hot-swap profil (A, §15.1) et reprise auto (B, §15.2) — au relance, `LaunchAgent` (ré)injecte/retire la conf MCP automatiquement puisque la surface suit le profil courant. +**État réel post-M3 (2026-06-10)** : M0→M3 livrés, **mais deux verrous restants** empêchent le « IdeA-only natif » et sont tranchés en **§14.3.2 (orchestration v5)** : (1) le **bind transport S-MCP** n'est pas câblé — `McpServer::serve` n'est jamais piloté, le serveur par projet est juste *parqué* (`state.rs::ensure_mcp_server`), donc aucune CLI lancée n'est réellement connectée aux outils `idea_*` ; (2) un **bug de robustesse du registre de session** (mémoire `session-registry-agent-ambiguity`) doit être corrigé pour fiabiliser le routage de `ask`. + +#### 14.3.2 Orchestration v5 — bind transport S-MCP + fix registre session + +> Cadrage complet : `.ideai/briefs/orchestration-v5-transport-bind-cadrage.md`. Cette sous-section fige le **dernier kilomètre** (transport réellement vivant) et le **fix de robustesse** prérequis. Elle ne réécrit ni le domaine ni l'application : elle **remplit** le placeholder de conf MCP, **pilote `serve`** par connexion, et **durcit** un invariant existant. + +**Décision V5-1 — Transport S-MCP = `stdio-spawn` (loopback), socket = TODO.** Une CLI MCP (Claude/Codex) attend une déclaration `{command,args}` et **spawn elle-même** ce process à l'`initialize`. IdeA fournit donc une **sous-commande `mcp-server` du binaire app-tauri existant** (route dans `main.rs` avant init Tauri, **un seul exécutable livré** AppImage/setup.exe) : un **pont** ultraléger `StdioTransport(stdin,stdout)` ↔ **endpoint loopback du projet** (Unix domain socket / Windows named pipe, **sans port réseau** ⇒ AppImage/Windows/SSH-safe). Le `McpServer` (qui tient l'`OrchestratorService`/`Project`) **reste dans le process Tauri** ; `McpServerHandle` **accepte** sur l'endpoint et **spawn une tâche `McpServer::serve(conn)` par pair**. Le **point dur** « comment le process serveur retrouve le bon projet » est résolu par **injection d'identité aux `args`** (`--endpoint`/`--project`/`--requester`), fixée au `LaunchAgent` (projet connu à ce moment). Le socket direct est **rejeté en défaut** (ports/permissions/cross-OS, support CLI inégal) mais reste un **ajout sans toucher `McpServer`** derrière le trait `Transport`. + +**Décision V5-2 — Cohérence conf↔serveur, source d'endpoint unique.** `apply_mcp_config` (M1) écrit la **déclaration réelle** (fin du placeholder `mcp_server_declaration`) : `command = current_exe()`, `args = ["mcp-server","--endpoint",mcp_endpoint(project),"--project",id,"--requester",agent]`. Le **chemin d'endpoint** vient d'une **fonction unique** `mcp_endpoint(project_id)` partagée par celui qui **écrit** la conf (M1/M5d) et celui qui **écoute** (`ensure_mcp_server`/M5a) ⇒ **zéro chaîne dupliquée**, invariant de cohérence testable. `McpConfigStrategy` inchangé (`ConfigFile` écrit le fichier non-clobbering ; `Flag`/`Env` portent le chemin de conf). L'identité du pair (`--requester`) lève le `requester_id = "mcp"` figé ⇒ observabilité UI exacte (qui délègue à qui). + +**Décision V5-3 — Fix registre session = lot PRIORITAIRE et indépendant du transport.** Invariant correct = **« 1 session vivante par agent »** (décision produit verrouillée : un agent est un **singleton**, la cellule est une **vue** §17.6 — *pas* d'identité par cellule à inventer). `session_for_agent` est déterministe **à condition** d'enforcer l'invariant sur **les deux** registres. **Trois fuites** à boucher : (A) le garde de `LaunchAgent` ne lève **jamais** `AgentAlreadyRunning` (rebind/idempotent silencieux qui masque un second lancement) ⇒ distinguer **réattache de vue** (rebind) de **lancement neuf** (refus typé) ; (B) `list_live_agents` est **aveugle aux sessions structurées** (lit seulement `terminal_sessions`) ⇒ lire l'agrégateur `LiveSessions` (PTY+chat) ; (C) les `layouts.json` **à doublons** (N feuilles, même agent) ⇒ **réconciliation à l'ouverture** (garder une hôte, dé-flagger les autres), ce qui supprime le symptôme « une cellule reset au retour d'onglet ». + +**Décision V5-4 — Robustesse `ask` : sérialisation FIFO par agent.** Au-dessus de l'existant (cible morte ⇒ lancement structuré ; PTY brut ⇒ `Invalid` ; timeout 300 s ⇒ cible vivante + erreur typée), le seul manque est la **concurrence** : deux `ask` simultanés sur la même cible appelleraient `send_blocking` en parallèle sur **une** `AgentSession` ⇒ tours entrelacés (cf. bug accents = writes non sérialisés). `OrchestratorService::ask_agent` **sérialise les tours par `agent_id`** (verrou par agent) : file FIFO naturelle, timeout **par tour**, plafond d'attente borné. Règle **applicative** (vit dans le service/registre, pas dans l'adapter MCP). + +**Décision V5-5 — Frontières.** `McpServer::serve` est piloté **par connexion** dans l'adapter infra ; `McpServerHandle` (app-tauri) évolue de « parker » à « ouvrir l'endpoint + boucle d'`accept` + spawn serve par pair » (toujours non-bloquant pour open/close projet). Le sous-process `mcp-server` ne connaît que **stdio + loopback + JSON brut** (zéro `OrchestratorService`). `dispatch` est appelé **à l'identique** par les trois portes (fichier, MCP, UI). + +**Découpage en LOTS (méthode §3)** — **R0 d'abord** (fix registre, indépendant), puis **A0** (concurrence `ask`), puis **M5x** (bind), puis front optionnel : + +| Lot | Côté | Périmètre | Tests attendus | +|---|---|---|---| +| **R0a** | back | Garde `LaunchAgent`/`spawn_agent` : lever `AgentAlreadyRunning` pour un lancement **neuf** d'un agent vivant sur un **autre** node ; rebind si node = hôte. | neuf+ailleurs ⇒ `AGENT_ALREADY_RUNNING` ; réattache même node ⇒ rebind sans respawn ; idempotence inchangée. | +| **R0b** | back | `list_live_agents` lit `LiveSessions::live_agents()` (PTY+structuré). | un agent **chat** vivant apparaît ; PTY aussi ; pas de doublon. | +| **R0c** | back | Réconciliation à l'ouverture : agent sur N feuilles ⇒ garder une hôte, dé-flagger les autres. | layout à doublons ⇒ une seule feuille « en cours » ; sans doublon inchangé ; 2ᵉ ouverture = no-op. | +| **R0d** | front | Dropdown leaf : désactiver via R0b (PTY+chat) ; gérer `AGENT_ALREADY_RUNNING` (aller-à/déplacer). | option désactivée + « aller à la cellule » ; erreur mappée clairement. | +| **A0** | back | Sérialisation FIFO par agent dans `ask_agent` (verrou par `agent_id`, timeout par tour, plafond d'attente). | 2 `ask` même cible ⇒ séquentiels FIFO ; cibles différentes ⇒ parallèles ; timeout libère la file ; plafond ⇒ `Timeout`. | +| **M5a** | back | Endpoint loopback par projet `mcp_endpoint(project_id)` ; ouvert à l'open, fermé au close. | endpoint créé/supprimé ; idempotent (1/projet) ; déterministe ; pas de collision. | +| **M5b** | back | Sous-commande `mcp-server` dans `main.rs` : pont stdio↔loopback, handshake `--project`/`--requester`. | mode headless (pas de webview) ; relai requête↔réponse ; EOF ⇒ sortie propre ; endpoint absent ⇒ erreur, jamais de hang. | +| **M5c** | back | `McpServerHandle` accepte + `serve(conn)` par pair ; `requester_id` = agent réel (fin du `"mcp"`). | bout-en-bout local `initialize`/`tools/*` ; requester = agent ; déconnexion isolée ; arrêt ferme l'endpoint. | +| **M5d** | back | `apply_mcp_config` écrit la déclaration réelle (exe + `mcp_endpoint` partagé) ; non-clobbering. | `mcp=None` inchangé ; `.mcp.json` pointe exe+endpoint exacts ; endpoint identique à `ensure_mcp_server` (cohérence M1↔M3). | +| **M5e** | back | Smoke end-to-end loopback (faux pont, sans CLI) : `idea_list_agents`/`idea_ask_agent` → `dispatch` réel. | liste JSON ; `ask` structuré ⇒ `reply` inline ; PTY ⇒ erreur typée ; JSON-RPC malformé ⇒ erreur, jamais panic ; hors réseau. | +| **M5-UI** *(opt.)* | front | Badge source (`mcp`/`file`) + requester réel sur une délégation. | badge correct ; requester = agent réel ; pas de régression sans event. | + +**Ordre** : **R0a→R0b→R0c→R0d → A0 → M5a→M5b→M5c→M5d→M5e → M5-UI**. R0 et A0 sont livrables **sans** toucher MCP ; M5 ne part qu'**après** R0 (sinon on débugge `ask` mal routé + transport neuf simultanément). + --- ### 14.4 Git = intégration optionnelle, zéro dépendance fonctionnelle diff --git a/crates/app-tauri/src/commands.rs b/crates/app-tauri/src/commands.rs index 32fcfe8..63cbcf4 100644 --- a/crates/app-tauri/src/commands.rs +++ b/crates/app-tauri/src/commands.rs @@ -15,11 +15,12 @@ use application::{ DeleteLayoutInput, DeleteMemoryInput, DeleteSkillInput, DeleteTemplateInput, DetectAgentDriftInput, GetMemoryInput, GitBranchesInput, GitCheckoutInput, GitCommitInput, GitGraphInput, GitInitInput, GitLogInput, GitStagePathInput, GitStatusInput, - InspectConversationInput, LaunchAgentInput, ListAgentsInput, ListLayoutsInput, + InspectConversationInput, LaunchAgentInput, ListAgentsInput, ListLayoutsInput, LiveSessions, ListMemoriesInput, ListResumableAgentsInput, ListSkillsInput, LoadLayoutInput, MutateLayoutInput, OpenProjectInput, ReadAgentContextInput, ReadMemoryIndexInput, ReadProjectContextInput, RecallMemoryInput, - RenameLayoutInput, ResolveMemoryLinksInput, SetActiveLayoutInput, SnapshotRunningAgentsInput, + ReconcileLayoutsInput, RenameLayoutInput, ResolveMemoryLinksInput, SetActiveLayoutInput, + SnapshotRunningAgentsInput, SyncAgentWithTemplateInput, UnassignSkillFromAgentInput, UpdateAgentContextInput, UpdateMemoryInput, UpdateProjectContextInput, UpdateSkillInput, }; @@ -108,6 +109,15 @@ pub async fn open_project( .execute(OpenProjectInput { project_id: id }) .await .map_err(ErrorDto::from)?; + // R0c (§3.4 « Trou C ») : dé-doublonne les `layouts.json` portant plusieurs + // feuilles sur le même agent AVANT toute reprise (`list_resumable_agents` + // relit la version persistée), de sorte qu'on ne propose / ne relance qu'une + // session par agent. Idempotent (no-op sans doublon) et best-effort : un échec + // ne doit pas bloquer l'ouverture. + let _ = state + .reconcile_layouts + .execute(ReconcileLayoutsInput { project_id: id }) + .await; // (Re)start the orchestrator watcher for this project (idempotent, §14.3). state.ensure_orchestrator_watch(&output.project); Ok(ProjectDto::from(output)) @@ -835,9 +845,18 @@ pub async fn list_agents( .map_err(ErrorDto::from) } -/// `list_live_agents` — list the agents that currently own a live PTY session -/// and the cell hosting each, so the UI can disable an agent already running in -/// another cell (the "one live session per agent" invariant). +/// `list_live_agents` — list every agent that currently owns a live session +/// (raw PTY **or** structured/chat) and the cell hosting each, so the UI can +/// disable an agent already running in another cell (the "one live session per +/// agent" invariant). +/// +/// Reads the **aggregator** [`LiveSessions::live_agents`], the single source of +/// truth for liveness across both registries (PTY + structured). Reading only +/// `terminal_sessions` here was blind to structured chat agents, so a live chat +/// agent would not be disabled in the UI and could be relaunched elsewhere +/// (cadrage v5 §3.3, Trou B). `LiveSessions` is built on the fly from the two +/// `Arc` registries already held by [`AppState`]; the aggregation logic itself +/// is not duplicated. /// /// `project_id` is accepted for API symmetry and future per-project scoping; the /// session registry is process-wide today, so the full live set is returned (a @@ -854,9 +873,11 @@ pub fn list_live_agents( // Validate the id shape for a consistent contract, even though the registry // is not project-scoped yet. let _ = parse_project_id(&project_id)?; - Ok(LiveAgentListDto::from_pairs( - state.terminal_sessions.live_agents(), - )) + let live = LiveSessions::new( + std::sync::Arc::clone(&state.terminal_sessions), + std::sync::Arc::clone(&state.structured_sessions), + ); + Ok(LiveAgentListDto::from_pairs(live.live_agents())) } /// `attach_live_agent` — rebind an already-running agent session to a visible diff --git a/crates/app-tauri/src/dto.rs b/crates/app-tauri/src/dto.rs index 7ea972c..8a30daa 100644 --- a/crates/app-tauri/src/dto.rs +++ b/crates/app-tauri/src/dto.rs @@ -1349,11 +1349,19 @@ pub struct LiveAgentListDto(pub Vec); impl LiveAgentListDto { /// Builds the wire list from the registry's `(AgentId, NodeId, SessionId)` tuples. + /// + /// De-duplicates by `agent_id`: the "one live session per agent" invariant + /// guarantees an agent is live in at most one registry (PTY **or** + /// structured), but the aggregated input could in theory carry the same agent + /// twice; the UI contract is a dup-free set (each agent appears once), so we + /// keep the first occurrence and drop any later one for the same agent. #[must_use] pub fn from_pairs(pairs: Vec<(AgentId, NodeId, domain::SessionId)>) -> Self { + let mut seen = std::collections::HashSet::new(); Self( pairs .into_iter() + .filter(|(agent_id, _, _)| seen.insert(*agent_id)) .map(|(agent_id, node_id, session_id)| LiveAgentDto { agent_id: agent_id.to_string(), node_id: node_id.to_string(), diff --git a/crates/app-tauri/src/state.rs b/crates/app-tauri/src/state.rs index c3ffaa4..758a9ea 100644 --- a/crates/app-tauri/src/state.rs +++ b/crates/app-tauri/src/state.rs @@ -22,7 +22,8 @@ use application::{ ListTemplates, LiveAgentRegistry, LoadLayout, MoveTabToNewWindow, MutateLayout, OnnxModelView, OpenProject, OpenTerminal, OrchestratorService, ReadAgentContext, ReadMemoryIndex, ReadProjectContext, - RecallMemory, ReferenceProfiles, RenameLayout, ResizeTerminal, ResolveMemoryLinks, + RecallMemory, ReconcileLayouts, ReferenceProfiles, RenameLayout, ResizeTerminal, + ResolveMemoryLinks, SaveEmbedderProfile, SaveProfile, SetActiveLayout, SnapshotRunningAgents, StructuredSessions, SuggestedThisSession, SyncAgentWithTemplate, TerminalSessions, UnassignSkillFromAgent, UpdateAgentContext, @@ -98,6 +99,9 @@ pub struct AppState { pub set_active_layout: Arc, /// Freeze `agent_was_running` on every agent leaf before a PTY kill (T5). pub snapshot_running_agents: Arc, + /// Dé-doublonne, à l'ouverture, les feuilles d'agent en double d'un même + /// agent dans `layouts.json` (R0c). Idempotent : no-op sans doublon. + pub reconcile_layouts: Arc, /// Detect which candidate profiles' CLIs are installed (first-run). pub detect_profiles: Arc, /// List configured profiles. @@ -380,6 +384,14 @@ impl AppState { Arc::clone(&terminal_sessions) as Arc, )); + // Twin of the snapshot above, but at *open* time: dé-doublonne les + // `layouts.json` portant plusieurs feuilles sur le même agent (R0c, §3.4 + // « Trou C »). Idempotent : aucun doublon ⇒ aucune écriture. + let reconcile_layouts = Arc::new(ReconcileLayouts::new( + Arc::clone(&store_port), + Arc::clone(&fs_port), + )); + // --- Profiles & AI runtime (L5) --- // One generic, profile-driven runtime adapter (Open/Closed): it holds the // process spawner used for detection. The profile store persists @@ -761,6 +773,7 @@ impl AppState { delete_layout, set_active_layout, snapshot_running_agents, + reconcile_layouts, detect_profiles, list_profiles, save_profile, diff --git a/crates/app-tauri/tests/list_live_agents_r0b.rs b/crates/app-tauri/tests/list_live_agents_r0b.rs new file mode 100644 index 0000000..aa95222 --- /dev/null +++ b/crates/app-tauri/tests/list_live_agents_r0b.rs @@ -0,0 +1,180 @@ +//! LOT R0b (cadrage orchestration v5 §3.3, Trou B) — `list_live_agents` lit +//! l'agrégateur `LiveSessions` (PTY **+** structuré), plus seulement le registre +//! PTY. Un agent chat (structuré) vivant doit apparaître dans la liste, comme un +//! agent PTY ; les deux ensemble sans doublon ; aucune session ⇒ liste vide. +//! +//! Ces tests reproduisent **exactement** ce que fait la commande +//! `list_live_agents` : construire un `LiveSessions` à partir des deux registres +//! partagés et passer `live_agents()` à `LiveAgentListDto::from_pairs`. Ils +//! exercent donc le câblage de l'agrégateur **et** la dé-duplication par agent +//! portée par le DTO (contrat de liveness pour l'UI). 100 % fakes, sans process. + +use std::sync::Arc; + +use async_trait::async_trait; + +use app_tauri_lib::dto::LiveAgentListDto; +use application::{LiveSessions, StructuredSessions, TerminalSessions}; +use domain::ports::{AgentSession, AgentSessionError, PtyHandle, ReplyStream}; +use domain::{ + AgentId, NodeId, ProjectPath, PtySize, SessionId, SessionKind, TerminalSession, +}; +use uuid::Uuid; + +// --- petits constructeurs déterministes ------------------------------------ + +fn sid(n: u128) -> SessionId { + SessionId::from_uuid(Uuid::from_u128(n)) +} +fn aid(n: u128) -> AgentId { + AgentId::from_uuid(Uuid::from_u128(n)) +} +fn nid(n: u128) -> NodeId { + NodeId::from_uuid(Uuid::from_u128(n)) +} + +/// Fake minimal d'`AgentSession` : porte juste l'id (ce que le registre clé). +struct FakeSession { + id: SessionId, +} + +#[async_trait] +impl AgentSession for FakeSession { + fn id(&self) -> SessionId { + self.id + } + fn conversation_id(&self) -> Option { + None + } + async fn send(&self, _prompt: &str) -> Result { + Ok(Box::new(std::iter::empty())) + } + async fn shutdown(&self) -> Result<(), AgentSessionError> { + Ok(()) + } +} + +fn fake(id: SessionId) -> Arc { + Arc::new(FakeSession { id }) +} + +/// Insère un agent PTY dans `TerminalSessions` (jumeau du helper de D1). +fn insert_pty(pty: &TerminalSessions, s: SessionId, a: AgentId, n: NodeId) { + let session = TerminalSession::starting( + s, + n, + ProjectPath::new("/p").unwrap(), + SessionKind::Agent { agent_id: a }, + PtySize::new(24, 80).unwrap(), + ); + pty.insert(PtyHandle { session_id: s }, session); +} + +/// Reproduit le corps de la commande `list_live_agents` (hors validation d'id). +fn dto_for(pty: &Arc, structured: &Arc) -> LiveAgentListDto { + let live = LiveSessions::new(Arc::clone(pty), Arc::clone(structured)); + LiveAgentListDto::from_pairs(live.live_agents()) +} + +// =========================================================================== +// 1. Un agent PTY vivant apparaît. +// =========================================================================== +#[test] +fn pty_live_agent_is_listed() { + let pty = Arc::new(TerminalSessions::new()); + let structured = Arc::new(StructuredSessions::new()); + + let a = aid(10); + insert_pty(&pty, sid(1), a, nid(100)); + + let dto = dto_for(&pty, &structured); + assert_eq!(dto.0.len(), 1); + assert_eq!(dto.0[0].agent_id, a.to_string()); + assert_eq!(dto.0[0].node_id, nid(100).to_string()); + assert_eq!(dto.0[0].session_id, sid(1).to_string()); +} + +// =========================================================================== +// 2. Un agent structuré/chat vivant apparaît (le cas qui régressait — Trou B). +// =========================================================================== +#[test] +fn structured_live_agent_is_listed() { + let pty = Arc::new(TerminalSessions::new()); + let structured = Arc::new(StructuredSessions::new()); + + let a = aid(20); + structured.insert(fake(sid(2)), a, nid(200)); + + let dto = dto_for(&pty, &structured); + assert_eq!( + dto.0.len(), + 1, + "un agent chat vivant doit apparaître (il était invisible avant R0b)" + ); + assert_eq!(dto.0[0].agent_id, a.to_string()); + assert_eq!(dto.0[0].node_id, nid(200).to_string()); + assert_eq!(dto.0[0].session_id, sid(2).to_string()); +} + +// =========================================================================== +// 3. Les deux types vivants en même temps ⇒ présents, sans doublon. +// =========================================================================== +#[test] +fn both_kinds_live_listed_without_duplicates() { + let pty = Arc::new(TerminalSessions::new()); + let structured = Arc::new(StructuredSessions::new()); + + let pty_agent = aid(10); + let chat_agent = aid(20); + insert_pty(&pty, sid(1), pty_agent, nid(100)); + structured.insert(fake(sid(2)), chat_agent, nid(200)); + + let dto = dto_for(&pty, &structured); + assert_eq!(dto.0.len(), 2, "les deux registres contribuent"); + + let mut ids: Vec<&str> = dto.0.iter().map(|d| d.agent_id.as_str()).collect(); + ids.sort_unstable(); + let mut expected = vec![pty_agent.to_string(), chat_agent.to_string()]; + expected.sort_unstable(); + assert_eq!(ids, expected); + + // Aucun agent répété (contrat de liveness pour l'UI). + let mut uniq = ids.clone(); + uniq.dedup(); + assert_eq!(uniq.len(), dto.0.len(), "aucun doublon d'agent"); +} + +// =========================================================================== +// 3bis. Même agent présent dans les DEUX registres ⇒ une seule entrée. +// +// L'invariant « 1 session vivante/agent » l'interdit normalement, mais le DTO +// doit rester dup-free par défense en profondeur : un seul `agent_id` sur le fil. +// =========================================================================== +#[test] +fn same_agent_in_both_registries_is_deduplicated() { + let pty = Arc::new(TerminalSessions::new()); + let structured = Arc::new(StructuredSessions::new()); + + let a = aid(30); + insert_pty(&pty, sid(1), a, nid(100)); + structured.insert(fake(sid(2)), a, nid(200)); + + let dto = dto_for(&pty, &structured); + assert_eq!(dto.0.len(), 1, "un même agent ne doit apparaître qu'une fois"); + assert_eq!(dto.0[0].agent_id, a.to_string()); + // On garde la première occurrence (PTY, listé en premier par l'agrégateur). + assert_eq!(dto.0[0].node_id, nid(100).to_string()); + assert_eq!(dto.0[0].session_id, sid(1).to_string()); +} + +// =========================================================================== +// 4. Aucune session ⇒ liste vide. +// =========================================================================== +#[test] +fn no_sessions_yields_empty_list() { + let pty = Arc::new(TerminalSessions::new()); + let structured = Arc::new(StructuredSessions::new()); + + let dto = dto_for(&pty, &structured); + assert!(dto.0.is_empty(), "aucune session ⇒ liste vide"); +} diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index 37ac57b..2715d53 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -874,18 +874,40 @@ impl LaunchAgent { // (§17.4) : un agent est vivant s'il a une session PTY *ou* structurée. // On consulte d'abord le registre PTY (chemin historique), puis le // registre structuré (le cas échéant). + // + // R0a — discrimination réattache-de-vue vs second lancement (cadrage v5 + // §3.2, Trou A). Un agent est un **singleton** : une seule session vivante. + // Quand l'agent est déjà vivant, on distingue (cf. [`Self::reattach_decision`]) : + // - **réattache de vue** (rebind sans respawn) : le `node_id` demandé est + // le node hôte vivant, OU la cellule porte une `conversation_id` + // (réattache explicite — la cellule sait que l'agent tournait) ; + // - **lancement background/idempotent** : ni node, ni conversation ⇒ no-op + // (rend la session existante, pas de respawn) ; + // - **second lancement neuf** : on vise un **autre** node, sans signal de + // réattache ⇒ refus [`AppError::AgentAlreadyRunning`] (node hôte rapporté). if let Some(existing_id) = self.sessions.session_for_agent(&input.agent_id) { - if let Some(node_id) = input.node_id { - if let Some(session) = self.sessions.rebind_agent_node(&input.agent_id, node_id) { - return Ok(LaunchAgentOutput { - session, - assigned_conversation_id: None, - structured: None, + let host_node = self.sessions.node_for_agent(&input.agent_id); + match reattach_decision(input.node_id, host_node, input.conversation_id.as_deref()) { + ReattachDecision::Rebind { node_id } => { + if let Some(session) = self.sessions.rebind_agent_node(&input.agent_id, node_id) + { + return Ok(LaunchAgentOutput { + session, + assigned_conversation_id: None, + structured: None, + }); + } + } + ReattachDecision::Idempotent => {} + ReattachDecision::Refuse { node_id } => { + return Err(AppError::AgentAlreadyRunning { + agent_id: input.agent_id, + node_id, }); } } - // Idempotent — hand back the already-registered session, no respawn, - // nothing new to persist. + // Idempotent (or a rebind that found no entry to move) — hand back the + // already-registered session, no respawn, nothing new to persist. if let Some(session) = self.sessions.session(&existing_id) { return Ok(LaunchAgentOutput { session, @@ -894,18 +916,30 @@ impl LaunchAgent { }); } } - // Garde structurée (§17.4) : même sémantique côté registre IA. Rebind de la - // cellule-vue si un node est demandé, sinon idempotence (pas de redémarrage). + // Garde structurée (§17.4) : même sémantique côté registre IA. R0a appliqué de + // façon identique — rebind de la cellule-vue pour une réattache légitime, + // idempotence sans node/conversation, refus d'un second lancement neuf ailleurs. if let Some(structured) = &self.structured { if let Some(existing) = structured.session_for_agent(&input.agent_id) { - // Cellule-vue : le node demandé, sinon le node hôte courant, sinon un - // node neuf (rebind sans redémarrer le process, « la cellule est une - // vue »). - let node_id = input.node_id.or_else(|| structured.node_for_agent(&input.agent_id)); - if let Some(node_id) = node_id { - let _ = structured.rebind_agent_node(&input.agent_id, node_id); - } - let node_id = node_id.unwrap_or_else(NodeId::new_random); + let host_node = structured.node_for_agent(&input.agent_id); + let node_id = match reattach_decision( + input.node_id, + host_node, + input.conversation_id.as_deref(), + ) { + ReattachDecision::Rebind { node_id } => { + let _ = structured.rebind_agent_node(&input.agent_id, node_id); + node_id + } + // Idempotent — garder le node hôte courant, sinon un node neuf. + ReattachDecision::Idempotent => host_node.unwrap_or_else(NodeId::new_random), + ReattachDecision::Refuse { node_id } => { + return Err(AppError::AgentAlreadyRunning { + agent_id: input.agent_id, + node_id, + }); + } + }; return Ok(LaunchAgentOutput { session: structured_snapshot(&existing, input.agent_id, node_id, size), assigned_conversation_id: None, @@ -1339,6 +1373,80 @@ impl LaunchAgent { } } +/// Outcome of the R0a discrimination between a legitimate **view reattach** and a +/// **fresh second launch** of an already-live agent (cadrage v5 §3.2, Trou A). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ReattachDecision { + /// Rebind the live session's view to `node_id` without respawning (the request + /// targets the live host node, or carries an explicit reattach signal). + Rebind { node_id: NodeId }, + /// Background / idempotent no-op: hand back the existing session unchanged + /// (neither a node nor a conversation id was supplied). + Idempotent, + /// Refuse the launch — a genuine second launch of a singleton agent already + /// live on `node_id` (the host node, reported in `AgentAlreadyRunning`). + Refuse { node_id: NodeId }, +} + +impl ReattachDecision { + /// Decides reattach vs refuse for an already-live agent. See [`reattach_decision`]. + #[must_use] + pub(crate) fn resolve( + requested_node: Option, + host_node: Option, + conversation_id: Option<&str>, + ) -> Self { + reattach_decision(requested_node, host_node, conversation_id) + } +} + +/// Decides whether a launch hitting an already-live agent is a legitimate view +/// **reattach** (rebind), a **background/idempotent** no-op, or a **refused** second +/// launch (cadrage v5 §3.2, Trou A). Pure (no I/O), unit-testable. +/// +/// Signals (both already present in the launch flow): +/// - `requested_node` — the hosting leaf the caller targets (`None` ⇒ no cell, e.g. +/// a background launch); +/// - `host_node` — the node currently hosting the agent's live session, if known; +/// - `conversation_id` — the conversation id recorded on the hosting cell. Its +/// presence means the **cell knows the agent was running** ⇒ it is a reattach of a +/// view onto an in-progress session, not a brand-new launch. +/// +/// Rules: +/// - requested node **is** the host node ⇒ `Rebind` (re-open of the same cell); +/// - a `conversation_id` is present ⇒ `Rebind` (explicit reattach, even on another +/// node — the cell is a rebindable view, §17.6); +/// - no node **and** no conversation ⇒ `Idempotent` (background/no-op relaunch); +/// - a different node, no reattach signal ⇒ `Refuse` (genuine second launch). +fn reattach_decision( + requested_node: Option, + host_node: Option, + conversation_id: Option<&str>, +) -> ReattachDecision { + // Explicit reattach: the cell carries a conversation id, so it is re-binding a + // view onto an already-running session. Rebind to the requested node, or keep the + // current host node when none was supplied. + if conversation_id.is_some() { + if let Some(node_id) = requested_node.or(host_node) { + return ReattachDecision::Rebind { node_id }; + } + return ReattachDecision::Idempotent; + } + + match requested_node { + // Same cell re-opened ⇒ idempotent rebind (no respawn). + Some(node) if host_node == Some(node) => ReattachDecision::Rebind { node_id: node }, + // A different cell with no reattach signal ⇒ genuine second launch: refuse, + // reporting the live host node (falling back to the requested node only if the + // host is somehow unknown, so the error always carries a meaningful cell). + Some(node) => ReattachDecision::Refuse { + node_id: host_node.unwrap_or(node), + }, + // No node and no reattach signal ⇒ background/idempotent no-op. + None => ReattachDecision::Idempotent, + } +} + /// Builds the IdeA MCP server declaration written into a CLI's `ConfigFile` MCP /// config (cadrage v3, Décision 3). Coherent **placeholder** for M1: it describes a /// stdio/socket server launched by the `idea` binary — the exact command and the diff --git a/crates/application/src/agent/mod.rs b/crates/application/src/agent/mod.rs index 96f94b4..16973ac 100644 --- a/crates/application/src/agent/mod.rs +++ b/crates/application/src/agent/mod.rs @@ -14,6 +14,7 @@ mod structured; mod usecases; pub(crate) use lifecycle::unique_md_path; +pub(crate) use lifecycle::ReattachDecision; pub use structured::send_blocking; diff --git a/crates/application/src/layout/mod.rs b/crates/application/src/layout/mod.rs index 089ead2..58b0bc9 100644 --- a/crates/application/src/layout/mod.rs +++ b/crates/application/src/layout/mod.rs @@ -21,6 +21,7 @@ //! `TerminalSessions` registry, keyed by that same `SessionId`. mod management; +mod reconcile; mod snapshot; mod store; mod usecases; @@ -30,6 +31,7 @@ pub use management::{ DeleteLayoutOutput, LayoutInfo, ListLayouts, ListLayoutsInput, ListLayoutsOutput, RenameLayout, RenameLayoutInput, SetActiveLayout, SetActiveLayoutInput, }; +pub use reconcile::{ReconcileLayouts, ReconcileLayoutsInput, ReconcileLayoutsOutput}; pub use snapshot::{ SnapshotRunningAgents, SnapshotRunningAgentsInput, SnapshotRunningAgentsOutput, }; diff --git a/crates/application/src/layout/reconcile.rs b/crates/application/src/layout/reconcile.rs new file mode 100644 index 0000000..9b077a5 --- /dev/null +++ b/crates/application/src/layout/reconcile.rs @@ -0,0 +1,101 @@ +//! [`ReconcileLayouts`] — réconcilie, à l'**ouverture** d'un projet, les +//! `layouts.json` qui contiennent des feuilles en **doublon** sur un même agent +//! (lot R0c du cadrage orchestration v5, §3.4 « Trou C »). +//! +//! C'est la **jumelle** de [`super::snapshot::SnapshotRunningAgents`] : là où le +//! snapshot **gèle** `agent_was_running` à la *fermeture*, cette réconciliation +//! **dé-doublonne** à l'*ouverture*. Un `layouts.json` persisté peut déjà porter +//! deux feuilles sur le **même** `agent` id ; à la réouverture il ne faut **pas** +//! relancer la 2ᵉ. On garde **une** feuille « hôte » (potentiellement vivante / +//! reprenable) et on transforme les autres en **vues mortes** : leur +//! `agent_was_running` repasse à `false` et leur `conversation_id` est retiré. +//! C'est précisément ce qui éliminait le symptôme « une cellule reset au retour +//! d'onglet ». +//! +//! Le use case est un mince orchestrateur au-dessus de : +//! - le store des layouts persistés ([`super::store`]), +//! - l'opération pure du domaine +//! [`domain::LayoutTree::reconcile_duplicate_agents`] (qui porte la **règle +//! déterministe de choix de l'hôte** et la garantie d'idempotence). +//! +//! **Idempotence / no-op** : un layout sans doublon (ou déjà réconcilié) ressort +//! **identique** de l'opération pure ; on ne persiste alors **rien** (aucune +//! écriture). Une 2ᵉ ouverture du même projet réconcilié est donc un no-op. +//! +//! **Ordre d'insertion** : à appeler à l'ouverture du projet, **après** le +//! chargement/résolution des layouts et **avant** toute reprise +//! ([`crate::ListResumableAgents`]) — qui relit la version persistée — de sorte +//! qu'on ne propose / ne relance qu'**une** session par agent. + +use std::sync::Arc; + +use domain::ports::{FileSystem, ProjectStore}; +use domain::ProjectId; + +use crate::error::AppError; + +use super::store::{persist_doc, resolve_doc}; + +/// Input de [`ReconcileLayouts::execute`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReconcileLayoutsInput { + /// Le projet dont les layouts doivent être dé-doublonnés. + pub project_id: ProjectId, +} + +/// Output de [`ReconcileLayouts::execute`]. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct ReconcileLayoutsOutput { + /// `true` si au moins un layout a été modifié (et donc le doc persisté). + /// `false` ⇒ aucun doublon : no-op, aucune écriture (idempotence). + pub changed: bool, +} + +/// Réconcilie les feuilles d'agent en doublon de tous les layouts d'un projet, +/// puis persiste le doc **uniquement** s'il a changé. +pub struct ReconcileLayouts { + store: Arc, + fs: Arc, +} + +impl ReconcileLayouts { + /// Construit le use case à partir de ses ports injectés. + #[must_use] + pub fn new(store: Arc, fs: Arc) -> Self { + Self { store, fs } + } + + /// Exécute la réconciliation pour un projet. + /// + /// Pour chaque layout, applique l'opération pure + /// [`domain::LayoutTree::reconcile_duplicate_agents`]. Si **aucun** arbre n'a + /// changé, ne persiste **rien** (no-op idempotent). Sinon, persiste tout le + /// doc une fois. + /// + /// # Errors + /// - [`AppError::NotFound`] si le projet est inconnu, + /// - [`AppError::Store`] sur défaillance du registre, + /// - [`AppError::FileSystem`] sur défaillance de persistance. + pub async fn execute( + &self, + input: ReconcileLayoutsInput, + ) -> Result { + let project = self.store.load_project(input.project_id).await?; + let mut doc = resolve_doc(self.fs.as_ref(), &project).await?; + + let mut changed = false; + for named in &mut doc.layouts { + let reconciled = named.tree.reconcile_duplicate_agents(); + if reconciled != named.tree { + named.tree = reconciled; + changed = true; + } + } + + if changed { + persist_doc(self.fs.as_ref(), &project, &doc).await?; + } + + Ok(ReconcileLayoutsOutput { changed }) + } +} diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 38525d8..24a258f 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -59,9 +59,10 @@ pub use layout::{ CreateLayout, CreateLayoutInput, CreateLayoutOutput, DeleteLayout, DeleteLayoutInput, DeleteLayoutOutput, LayoutInfo, LayoutKind, LayoutOperation, LayoutsDoc, ListLayouts, ListLayoutsInput, ListLayoutsOutput, LoadLayout, LoadLayoutInput, LoadLayoutOutput, - MutateLayout, MutateLayoutInput, MutateLayoutOutput, NamedLayout, RenameLayout, - RenameLayoutInput, SetActiveLayout, SetActiveLayoutInput, SnapshotRunningAgents, - SnapshotRunningAgentsInput, SnapshotRunningAgentsOutput, LAYOUTS_FILE, + MutateLayout, MutateLayoutInput, MutateLayoutOutput, NamedLayout, ReconcileLayouts, + ReconcileLayoutsInput, ReconcileLayoutsOutput, RenameLayout, RenameLayoutInput, + SetActiveLayout, SetActiveLayoutInput, SnapshotRunningAgents, SnapshotRunningAgentsInput, + SnapshotRunningAgentsOutput, LAYOUTS_FILE, }; pub use memory::{ CreateMemory, CreateMemoryInput, CreateMemoryOutput, DeleteMemory, DeleteMemoryInput, diff --git a/crates/application/src/orchestrator/service.rs b/crates/application/src/orchestrator/service.rs index b07a032..f2850e4 100644 --- a/crates/application/src/orchestrator/service.rs +++ b/crates/application/src/orchestrator/service.rs @@ -13,15 +13,18 @@ //! no filesystem watching, no JSON, no process spawning here — those are the //! infrastructure adapter's job. That keeps this fully unit-testable with fakes. -use std::sync::Arc; +use std::collections::HashMap; +use std::sync::{Arc, Mutex as StdMutex}; use std::time::Duration; +use tokio::sync::Mutex as AsyncMutex; + use domain::ports::{EventBus, ProfileStore}; -use domain::{DomainEvent, OrchestratorCommand, OrchestratorVisibility, ProfileId, Project}; +use domain::{AgentId, DomainEvent, OrchestratorCommand, OrchestratorVisibility, ProfileId, Project}; use crate::agent::{ send_blocking, CreateAgentFromScratch, CreateAgentInput, LaunchAgent, LaunchAgentInput, - ListAgents, ListAgentsInput, UpdateAgentContext, UpdateAgentContextInput, + ListAgents, ListAgentsInput, ReattachDecision, UpdateAgentContext, UpdateAgentContextInput, }; use crate::error::AppError; use crate::skill::{CreateSkill, CreateSkillInput}; @@ -43,6 +46,21 @@ const DEFAULT_COLS: u16 = 80; /// without changing the contract). const ASK_AGENT_TIMEOUT: Duration = Duration::from_secs(300); +/// Borne d'attente **en file** pour acquérir le verrou de tour d'un agent (A0, +/// cadrage v5 §4). +/// +/// La sérialisation FIFO par agent (« 1 agent = 1 employé : un tour à la fois ») +/// fait qu'un `ask` concurrent vers la **même** cible patiente derrière le tour en +/// cours. Le timeout de tour ([`ASK_AGENT_TIMEOUT`]) borne **le tour lui-même**, +/// **pas** cette attente : on lui donne donc son propre plafond, généreux mais fini, +/// pour qu'un `ask` ne reste jamais bloqué indéfiniment si la file est longue +/// (inanition). À l'expiration, l'`ask` renvoie un timeout typé (réutilise le +/// **même** type que le timeout de tour, cf. [`AppError::Process`] via +/// [`domain::ports::AgentSessionError::Timeout`]) ; le tour en cours n'est pas +/// affecté. Largeur = un tour complet + sa propre file ⇒ on autorise deux tours +/// pleins d'attente. +const ASK_QUEUE_WAIT_CAP: Duration = Duration::from_secs(600); + /// Dispatches validated orchestrator commands to the agent/terminal use cases. pub struct OrchestratorService { create_agent: Arc, @@ -62,6 +80,25 @@ pub struct OrchestratorService { /// `ask` réussi (§17.4). Injecté via [`Self::with_events`] ; `None` ⇒ pas de /// publication (l'`ask` fonctionne quand même). events: Option>, + /// Verrous de **tour par agent** (A0, cadrage v5 §4) — sérialisation FIFO des + /// `ask` vers une **même** cible : « 1 agent = 1 employé, un tour à la fois ». + /// + /// Clé = `AgentId` de la **cible** ; valeur = un `tokio::Mutex` d'unité dont le + /// garde est tenu **à travers** le `.await` de `send_blocking` (d'où le mutex + /// **async**, pas `std`). La `HashMap` elle-même n'est tenue que le temps du + /// get-or-create (jamais à travers un `.await`), donc protégée par un mutex + /// **synchrone** `std` — pas de garde gardé en travers d'un point de suspension. + /// + /// Le registre vit **ici** (règle applicative d'orchestration, frontière + /// hexagonale, cadrage v5 §5) et **non** dans [`StructuredSessions`] : sérialiser + /// les tours est une responsabilité de l'orchestrateur, pas du stockage de + /// sessions (SRP) ; le couplage reste minimal. Verrou **par agent** ⇒ deux `ask` + /// vers des agents **différents** n'entrent jamais en contention (un map d'`Arc` + /// distincts). + /// + /// Croissance bornée en pratique au nombre d'agents du projet ; une entrée + /// morte ne coûte qu'un `Arc>` vide (pas de session, pas de process). + ask_locks: StdMutex>>>, } /// Outcome of dispatching a command — a short, human-readable success summary the @@ -102,9 +139,24 @@ impl OrchestratorService { sessions, structured: None, events: None, + ask_locks: StdMutex::new(HashMap::new()), } } + /// Returns the per-agent **turn lock**, creating it on first use. + /// + /// Get-or-create under the synchronous map mutex (held only for this lookup, + /// never across an `.await`); the returned `Arc>` is the lock the + /// caller acquires (and holds across `send_blocking`) to serialise turns for + /// `agent_id`. Two different agents get two distinct `Arc`s ⇒ no contention. + fn ask_lock_for(&self, agent_id: &AgentId) -> Arc> { + let mut locks = self + .ask_locks + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + Arc::clone(locks.entry(*agent_id).or_default()) + } + /// Branche le registre des sessions **structurées** (§17.5) pour servir /// `agent.message`/[`OrchestratorCommand::AskAgent`]. Builder additif façon D3 : /// signature de [`Self::new`] **inchangée** (les tests/call sites legacy restent @@ -202,18 +254,43 @@ impl OrchestratorService { }); } OrchestratorVisibility::Visible { node_id } => { - let session = self - .sessions - .rebind_agent_node(&agent_id, node_id) - .ok_or_else(|| { - AppError::NotFound(format!( - "running session {session_id} for agent {name}" - )) - })?; - return Ok(OrchestratorOutcome { - detail: format!("attached agent {name} to cell {}", session.node_id), - reply: None, - }); + // R0a — même règle que le garde de `LaunchAgent` (cadrage v5 §3.2, + // Trou A) : un `spawn` est un **lancement neuf** (pas de + // conversation portée), donc ré-attacher à la **même** cellule + // hôte est légitime (rebind de vue), mais viser un **autre** node + // pour un agent singleton déjà vivant est un second lancement ⇒ + // refus `AgentAlreadyRunning`. + let host_node = self.sessions.node_for_agent(&agent_id); + match ReattachDecision::resolve(Some(node_id), host_node, None) { + ReattachDecision::Rebind { node_id } => { + let session = self + .sessions + .rebind_agent_node(&agent_id, node_id) + .ok_or_else(|| { + AppError::NotFound(format!( + "running session {session_id} for agent {name}" + )) + })?; + return Ok(OrchestratorOutcome { + detail: format!( + "attached agent {name} to cell {}", + session.node_id + ), + reply: None, + }); + } + ReattachDecision::Refuse { node_id } => { + return Err(AppError::AgentAlreadyRunning { agent_id, node_id }); + } + // A `Visible` spawn always carries a node, so the decision is + // never `Idempotent`; treat it as a same-node rebind for safety. + ReattachDecision::Idempotent => { + return Ok(OrchestratorOutcome { + detail: format!("agent {name} already running"), + reply: None, + }); + } + } } } } @@ -287,6 +364,29 @@ impl OrchestratorService { .await? .ok_or_else(|| AppError::NotFound(format!("agent {target}")))?; + // Sérialisation FIFO **par agent** (A0, cadrage v5 §4) : on acquiert le verrou + // de tour de la **cible** avant tout `send_blocking`, et on le tient pour TOUT + // le tour réel (rendez-vous direct *ou* lancement-puis-envoi sur cible morte). + // Le garde `_turn` est RAII : il tombe en fin de portée, y compris sur chaque + // early-return d'erreur ci-dessous ⇒ le tour suivant en file démarre alors. + // + // Verrou par `agent_id` ⇒ un `ask` vers A et un vers B ne se bloquent jamais ; + // un seul verrou tenu (celui de la cible) ⇒ pas d'inter-verrouillage/deadlock. + // Le timeout de TOUR ([`ASK_AGENT_TIMEOUT`]) borne `send_blocking`, **pas** + // l'attente du verrou ; cette attente a son **propre** plafond + // ([`ASK_QUEUE_WAIT_CAP`]) pour éviter l'inanition. + let lock = self.ask_lock_for(&agent_id); + let _turn = match tokio::time::timeout(ASK_QUEUE_WAIT_CAP, lock.lock_owned()).await { + Ok(guard) => guard, + Err(_elapsed) => { + // Réutilise le **même** type de timeout que le tour (cadrage v5 §4) : + // `AgentSessionError::Timeout` ⇒ `AppError::Process`. + return Err(AppError::from( + domain::ports::AgentSessionError::Timeout, + )); + } + }; + // 1. Session structurée déjà vivante ? ⇒ rendez-vous direct. if let Some(session) = structured.session_for_agent(&agent_id) { let content = send_blocking(session.as_ref(), &task, Some(ASK_AGENT_TIMEOUT)).await?; diff --git a/crates/application/tests/agent_lifecycle.rs b/crates/application/tests/agent_lifecycle.rs index efe8331..431bd57 100644 --- a/crates/application/tests/agent_lifecycle.rs +++ b/crates/application/tests/agent_lifecycle.rs @@ -956,11 +956,13 @@ fn nid(n: u128) -> domain::NodeId { domain::NodeId::from_uuid(Uuid::from_u128(n)) } -/// **Singleton invariant (T1)**: launching an agent that already owns a live -/// session in another cell re-attaches the existing session to the requested -/// node. The PTY is never spawned a second time. +/// **Singleton invariant (R0a, décision « 1 agent = 1 employé »)**: a *fresh* +/// launch (no `conversation_id`) targeting **another** cell of an agent already +/// live in cell A is a genuine second launch ⇒ it is **refused** with +/// `AppError::AgentAlreadyRunning { node_id: A, agent_id }`. The session is NOT +/// silently moved, the PTY is never spawned, and the registry is unchanged. #[tokio::test] -async fn launch_reattaches_agent_already_running_elsewhere() { +async fn launch_new_in_other_cell_refuses_when_agent_live_elsewhere() { let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture( ContextInjection::convention_file("CLAUDE.md").unwrap(), Some(ContextInjectionPlan::File { @@ -973,17 +975,32 @@ async fn launch_reattaches_agent_already_running_elsewhere() { seed_live_agent_session(&sessions, agent.id, host, sid(42)); let before = sessions.len(); - // Open the same running agent in a different cell B. + // Fresh launch (conversation_id: None) of the same running agent in a + // *different* cell B ⇒ must be refused, not silently rebound. let mut input = launch_input(agent.id); let target = nid(2); input.node_id = Some(target); - let out = launch.execute(input).await.expect("reattach succeeds"); + let err = launch + .execute(input) + .await + .expect_err("fresh second launch elsewhere is refused"); - assert_eq!(out.session.id, sid(42), "returns the existing session"); - assert_eq!(out.session.node_id, target, "session is rebound to target"); - assert_eq!(sessions.node_for_agent(&agent.id), Some(target)); + match err { + application::AppError::AgentAlreadyRunning { agent_id, node_id } => { + assert_eq!(agent_id, agent.id, "reports the live agent"); + assert_eq!(node_id, host, "reports the live HOST node A, not target B"); + } + other => panic!("expected AgentAlreadyRunning, got {other:?}"), + } + + // No silent move, no respawn, registry untouched. + assert_eq!( + sessions.node_for_agent(&agent.id), + Some(host), + "session stays pinned on its host node" + ); assert_eq!(sessions.len(), before, "registry size is unchanged"); - assert!(pty.spawns().is_empty(), "no PTY spawn on reattach"); + assert!(pty.spawns().is_empty(), "no PTY spawn on a refused launch"); } /// A launch with an unspecified node (`node_id: None`) for an already-live agent @@ -1038,6 +1055,40 @@ async fn launch_same_node_is_idempotent_no_respawn() { assert!(pty.spawns().is_empty(), "no respawn on same-node relaunch"); } +/// **Legitimate explicit reattach (R0a)**: launching an agent that is live in +/// cell A onto a *different* cell B but carrying a `conversation_id` is an +/// explicit view reattach (the cell knows the agent was running) ⇒ rebind, no +/// error, no respawn, same session id. +#[tokio::test] +async fn launch_other_cell_with_conversation_id_rebinds_no_respawn() { + let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture( + ContextInjection::convention_file("CLAUDE.md").unwrap(), + Some(ContextInjectionPlan::File { + target: "CLAUDE.md".to_owned(), + }), + ); + + let host = nid(1); + seed_live_agent_session(&sessions, agent.id, host, sid(42)); + let before = sessions.len(); + + // Different cell B, but an explicit reattach signal (conversation_id present). + let mut input = launch_input(agent.id); + let target = nid(2); + input.node_id = Some(target); + input.conversation_id = Some("conv-live".to_owned()); + let out = launch + .execute(input) + .await + .expect("explicit reattach succeeds"); + + assert_eq!(out.session.id, sid(42), "returns the existing session"); + assert_eq!(out.session.node_id, target, "view rebound to target cell"); + assert_eq!(sessions.node_for_agent(&agent.id), Some(target)); + assert_eq!(sessions.len(), before, "registry size is unchanged"); + assert!(pty.spawns().is_empty(), "no PTY spawn on explicit reattach"); +} + /// After the live session is removed (cell closed / agent exited), a fresh launch /// succeeds — the guard only blocks while the agent is actually live. #[tokio::test] diff --git a/crates/application/tests/orchestrator_service.rs b/crates/application/tests/orchestrator_service.rs index 408dec8..02ff4ac 100644 --- a/crates/application/tests/orchestrator_service.rs +++ b/crates/application/tests/orchestrator_service.rs @@ -781,8 +781,48 @@ async fn agent_run_existing_agent_does_not_require_profile() { assert!(fx.sessions.session(&sid(777)).is_some()); } +/// **Réattache même cellule (R0a, orchestrateur)** : un `spawn`/`agent.run` +/// `Visible` qui re-cible la **même** cellule hôte d'un agent déjà vivant est un +/// rebind de vue ⇒ pas d'erreur, pas de respawn. #[tokio::test] -async fn agent_run_visible_reattaches_existing_session() { +async fn agent_run_visible_same_cell_rebinds_existing_session() { + let agent = scratch_agent(aid(1), "architect", "agents/architect.md"); + let fx = fixture(FakeContexts::with_agent(&agent, "# persona")); + let cell = nid(10); + + fx.service + .dispatch( + &project(), + cmd(&format!( + r#"{{ "action":"spawn_agent", "name":"architect", "profile":"claude", "visibility":"visible", "nodeId":"{cell}" }}"# + )), + ) + .await + .expect("first launch ok"); + + let out = fx + .service + .dispatch( + &project(), + cmd(&format!( + r#"{{ "type":"agent.run", "targetAgent":"architect", "visibility":"visible", "attachToCell":"{cell}" }}"# + )), + ) + .await + .expect("same-cell rebind ok"); + + assert!(out.detail.contains("attached agent architect")); + let session = fx.sessions.session(&sid(777)).expect("live session"); + assert_eq!(session.node_id, cell); + assert_eq!(fx.pty.spawns().len(), 1, "rebind must not respawn"); +} + +/// **Second lancement ailleurs refusé (R0a, orchestrateur)** : un `spawn`/`agent.run` +/// `Visible` qui vise une **autre** cellule d'un agent singleton déjà vivant est un +/// second lancement ⇒ refus `AgentAlreadyRunning` (node hôte rapporté), pas de +/// respawn, session non déplacée. +#[tokio::test] +async fn agent_run_visible_other_cell_refuses_when_live_elsewhere() { let agent = scratch_agent(aid(1), "architect", "agents/architect.md"); let fx = fixture(FakeContexts::with_agent(&agent, "# persona")); let first_cell = nid(10); @@ -798,7 +838,7 @@ async fn agent_run_visible_reattaches_existing_session() { .await .expect("first launch ok"); - let out = fx + let err = fx .service .dispatch( &project(), @@ -807,12 +847,20 @@ async fn agent_run_visible_reattaches_existing_session() { )), ) .await - .expect("reattach ok"); + .expect_err("fresh second launch elsewhere is refused"); - assert!(out.detail.contains("attached agent architect")); + assert_eq!(err.code(), "AGENT_ALREADY_RUNNING", "got {err:?}"); + match err { + application::AppError::AgentAlreadyRunning { node_id, .. } => { + assert_eq!(node_id, first_cell, "reports the live host cell, not target"); + } + other => panic!("expected AgentAlreadyRunning, got {other:?}"), + } + + // Session not moved, no respawn. let session = fx.sessions.session(&sid(777)).expect("live session"); - assert_eq!(session.node_id, next_cell); - assert_eq!(fx.pty.spawns().len(), 1, "reattach must not respawn"); + assert_eq!(session.node_id, first_cell, "session stays on its host cell"); + assert_eq!(fx.pty.spawns().len(), 1, "refused launch must not respawn"); } #[tokio::test] @@ -1276,3 +1324,379 @@ async fn non_regression_agent_run_reply_is_none() { .expect("run ok"); assert_eq!(out.reply, None, "agent.run must not carry a reply"); } + +// --------------------------------------------------------------------------- +// A0 — sérialisation FIFO des tours par agent (cadrage v5 §4) +// +// Le verrou `ask_locks` de l'orchestrateur doit garantir : (1) deux `ask` +// concurrents vers la **même** cible sont sérialisés FIFO (le 2ᵉ `send` ne +// démarre PAS tant que le 1ᵉ tour n'a pas rendu son `Final`) ; (2) deux `ask` +// vers des agents **différents** s'exécutent en parallèle ; (3) le verrou est +// relâché même si le tour se solde par une erreur. +// +// Pour observer le *timing*, on remplace le `FakeSession` scripté one-shot par +// un `GatedSession` **pilotable** : chaque `send` enregistre son démarrage +// (compteur + ordre) puis **bloque** sur un permis de `Semaphore` que le test +// délivre à la demande, avant de retourner son `Final`. Cela matérialise un tour +// dont on contrôle la fin (« retient le Final jusqu'à ce que le test le libère »). +// --------------------------------------------------------------------------- + +use std::sync::atomic::{AtomicUsize, Ordering}; +use tokio::sync::Semaphore; +use tokio::time::{timeout, Duration}; + +/// Borne de sûreté : aucun `await` de test ne doit jamais dépasser ça (garde-fou +/// anti-hang du lot de concurrence). Largement au-dessus du temps réel attendu. +const TEST_GUARD: Duration = Duration::from_secs(10); + +/// Une session **pilotable** : chaque `send` signale son démarrage (incrémente +/// `started`, pousse le prompt dans `order`) puis **attend un permis** du +/// sémaphore `gate` avant de retourner son `Final` (contenu = prompt reçu, pour +/// vérifier l'ordre de complétion). Le test délivre les permis un par un +/// (`release_one`) ⇒ il contrôle exactement quand chaque tour se termine. +/// +/// `mode` choisit l'issue du tour : `Final` (succès) ou `NoFinal` (flux vide ⇒ +/// `send_blocking` renvoie une erreur, pour tester la libération du verrou sur +/// erreur). +struct GatedSession { + id: SessionId, + /// Nombre de `send` **démarrés** (avant déblocage). Observé par le test pour + /// prouver qu'un 2ᵉ tour n'a PAS démarré pendant que le 1ᵉ est retenu. + started: AtomicUsize, + /// Nombre de `send` **terminés** (Final rendu / flux retourné). + finished: AtomicUsize, + /// Ordre des prompts reçus (FIFO attendu). + order: Arc>>, + /// Permis débloquant un tour à la fois. + gate: Semaphore, + /// Nombre de `shutdown` (doit rester 0 : l'orchestrateur ne tue jamais la + /// session de la cible). + shutdowns: AtomicUsize, + mode: GateMode, +} + +#[derive(Clone, Copy)] +enum GateMode { + /// Le tour rend un `Final` dont le contenu reprend le prompt. + FinalEchoesPrompt, + /// Le flux est vide (pas de `Final`) ⇒ tour interrompu, erreur typée. + NoFinal, +} + +impl GatedSession { + fn new(id: SessionId, mode: GateMode) -> Arc { + Arc::new(Self { + id, + started: AtomicUsize::new(0), + finished: AtomicUsize::new(0), + order: Arc::new(Mutex::new(Vec::new())), + gate: Semaphore::new(0), + shutdowns: AtomicUsize::new(0), + mode, + }) + } + fn started(&self) -> usize { + self.started.load(Ordering::SeqCst) + } + fn finished(&self) -> usize { + self.finished.load(Ordering::SeqCst) + } + fn order(&self) -> Vec { + self.order.lock().unwrap().clone() + } + /// Délivre un permis ⇒ débloque exactement UN tour en attente. + fn release_one(&self) { + self.gate.add_permits(1); + } +} + +#[async_trait] +impl AgentSession for GatedSession { + fn id(&self) -> SessionId { + self.id + } + fn conversation_id(&self) -> Option { + None + } + async fn send(&self, prompt: &str) -> Result { + self.started.fetch_add(1, Ordering::SeqCst); + self.order.lock().unwrap().push(prompt.to_owned()); + // Bloque jusqu'à ce que le test délivre un permis (retient le Final). + let permit = self.gate.acquire().await.expect("gate not closed"); + permit.forget(); + self.finished.fetch_add(1, Ordering::SeqCst); + match self.mode { + GateMode::FinalEchoesPrompt => Ok(Box::new(std::iter::once(final_(prompt)))), + GateMode::NoFinal => Ok(Box::new(std::iter::empty())), + } + } + async fn shutdown(&self) -> Result<(), AgentSessionError> { + self.shutdowns.fetch_add(1, Ordering::SeqCst); + Ok(()) + } +} + +/// Attend (borné) qu'une condition observée sur le fake devienne vraie, en cédant +/// la main au runtime entre deux lectures. Évite tout `sleep` fixe fragile. +async fn await_until bool>(cond: F) { + timeout(TEST_GUARD, async { + while !cond() { + tokio::task::yield_now().await; + } + }) + .await + .expect("condition never reached within guard (possible hang/deadlock)"); +} + +const ASK_ARCHITECT: &str = + r#"{ "type":"agent.message", "requestedBy":"Main", "targetAgent":"architect", "task":"T1" }"#; +const ASK_ARCHITECT_2: &str = + r#"{ "type":"agent.message", "requestedBy":"Main", "targetAgent":"architect", "task":"T2" }"#; + +/// **A0 #1 — FIFO même cible (cœur du lot).** Deux `ask` concurrents vers le +/// **même** agent, avec une session pilotable qui retient son `Final`. On prouve +/// que le 2ᵉ tour (`send`) ne **démarre pas** tant que le 1ᵉ n'a pas rendu son +/// `Final`, puis qu'en libérant, les deux complètent **dans l'ordre FIFO** (T1 +/// avant T2), sans entrelacement. +#[tokio::test] +async fn ask_same_target_serialises_turns_fifo() { + let agent = scratch_agent(aid(1), "architect", "agents/architect.md"); + let session = GatedSession::new(sid(500), GateMode::FinalEchoesPrompt); + let throwaway = FakeSession::new(sid(999), SendScript::Stream(vec![final_("unused")])); + let fx = ask_fixture( + FakeContexts::with_agent(&agent, "# persona"), + throwaway, + false, + ); + fx.structured + .insert(Arc::clone(&session) as Arc, aid(1), nid(1)); + + let proj = project(); + let svc = &fx.service; + + timeout(TEST_GUARD, async { + // Tour 1 : on le pilote jusqu'à ce que son `send` soit en vol (verrou + // acquis + bloqué sur le gate) AVANT de lancer le tour 2 ⇒ ordre d'entrée + // en file déterministe (T1 devant T2). `biased` : on tente d'abord r1. + let r1 = svc.dispatch(&proj, cmd(ASK_ARCHITECT)); + tokio::pin!(r1); + loop { + tokio::select! { + biased; + _ = &mut r1 => panic!("turn 1 should stay blocked on its Final"), + () = await_until(|| session.started() >= 1) => break, + } + } + + // À ce point : tour 1 démarré et BLOQUÉ sur le Final. + assert_eq!(session.started(), 1, "exactly the first turn started"); + assert_eq!(session.finished(), 0, "first turn is still blocked on Final"); + + // Lance le tour 2 concurremment. Il doit rester EN FILE (verrou de la même + // cible tenu par le tour 1) : son `send` ne doit PAS démarrer. On poll les + // DEUX futures pendant qu'on laisse au runtime des occasions de mal faire. + let r2 = svc.dispatch(&proj, cmd(ASK_ARCHITECT_2)); + tokio::pin!(r2); + for _ in 0..50 { + tokio::select! { + biased; + _ = &mut r1 => panic!("turn 1 still blocked, must not complete yet"), + _ = &mut r2 => panic!("turn 2 must not run before turn 1 releases the lock"), + () = tokio::task::yield_now() => {} + } + } + assert_eq!( + session.started(), + 1, + "second turn MUST NOT start while the first holds the per-agent lock (no interleaving)" + ); + + // Libère le tour 1 ⇒ il rend son Final, relâche le verrou, le tour 2 démarre. + session.release_one(); + let out1 = (&mut r1).await.expect("turn 1 ok"); + assert_eq!(out1.reply.as_deref(), Some("T1")); + + // Le tour 2 peut maintenant démarrer (verrou libre) ; il bloque sur SON + // gate. On le pilote jusqu'à son démarrage, puis on le libère. + loop { + tokio::select! { + biased; + res = &mut r2 => { + // Démarré ET libéré dans la même boucle : accepte la complétion. + let out2 = res.expect("turn 2 ok"); + assert_eq!(out2.reply.as_deref(), Some("T2")); + break; + } + () = await_until(|| session.started() >= 2) => { + session.release_one(); + } + } + } + }) + .await + .expect("FIFO scenario hung (deadlock?)"); + + // Complétion FIFO stricte : T1 puis T2, sans entrelacement. + assert_eq!(session.order(), vec!["T1".to_owned(), "T2".to_owned()]); + assert_eq!(session.finished(), 2); + assert_eq!( + session.shutdowns.load(Ordering::SeqCst), + 0, + "ask must never shutdown the target session" + ); +} + +/// **A0 #2 — agents différents en parallèle.** Un `ask` vers A (retenu sur son +/// `Final`) ne doit PAS bloquer un `ask` vers B : le tour de B démarre et +/// **complète** pendant que A est encore en cours. Verrou **par agent_id** ⇒ pas +/// de contention croisée. Si A bloquait B, l'attente de complétion de B +/// dépasserait le garde-fou et le test échouerait. +#[tokio::test] +async fn ask_different_targets_run_in_parallel() { + // Deux agents distincts dans le manifeste. + let contexts = FakeContexts::new(); + { + let agent_a = scratch_agent(aid(1), "alpha", "agents/alpha.md"); + let agent_b = scratch_agent(aid(2), "beta", "agents/beta.md"); + let mut inner = contexts.0.lock().unwrap(); + inner + .manifest + .entries + .push(ManifestEntry::from_agent(&agent_a)); + inner + .manifest + .entries + .push(ManifestEntry::from_agent(&agent_b)); + inner + .contents + .insert(agent_a.context_path.clone(), "# a".to_owned()); + inner + .contents + .insert(agent_b.context_path.clone(), "# b".to_owned()); + } + + let throwaway = FakeSession::new(sid(999), SendScript::Stream(vec![final_("unused")])); + let fx = ask_fixture(contexts, throwaway, false); + + // A : retenu indéfiniment (jamais libéré pendant le test). B : libérable. + let sess_a = GatedSession::new(sid(500), GateMode::FinalEchoesPrompt); + let sess_b = GatedSession::new(sid(600), GateMode::FinalEchoesPrompt); + fx.structured + .insert(Arc::clone(&sess_a) as Arc, aid(1), nid(1)); + fx.structured + .insert(Arc::clone(&sess_b) as Arc, aid(2), nid(2)); + + let proj = project(); + let svc = &fx.service; + + let ask_a = r#"{ "type":"agent.message", "targetAgent":"alpha", "task":"A1" }"#; + let ask_b = r#"{ "type":"agent.message", "targetAgent":"beta", "task":"B1" }"#; + + timeout(TEST_GUARD, async { + // Lance A et attends qu'il soit en vol (bloqué sur son Final), sans le libérer. + let a_fut = svc.dispatch(&proj, cmd(ask_a)); + tokio::pin!(a_fut); + // Pilote a_fut jusqu'à ce que A soit en vol. + let drive_a_until_inflight = async { + // a_fut bloque dans send → on le poll une fois via select avec la condition. + loop { + tokio::select! { + biased; + _ = &mut a_fut => panic!("A should stay blocked on its Final"), + () = await_until(|| sess_a.started() >= 1) => break, + } + } + }; + drive_a_until_inflight.await; + assert_eq!(sess_a.started(), 1); + assert_eq!(sess_a.finished(), 0, "A is held on its Final"); + + // Pendant que A est retenu, B doit pouvoir démarrer ET compléter. + let b_out = async { + // B bloque aussi sur son gate : on attend qu'il démarre, on le libère. + let b_fut = svc.dispatch(&proj, cmd(ask_b)); + tokio::pin!(b_fut); + loop { + tokio::select! { + biased; + res = &mut b_fut => break res, + () = await_until(|| sess_b.started() >= 1) => { + sess_b.release_one(); + } + } + } + } + .await + .expect("B completes while A is still in flight"); + + assert_eq!(b_out.reply.as_deref(), Some("B1")); + // A est toujours en vol, non terminé : la parallélisme est prouvée. + assert_eq!(sess_a.finished(), 0, "A must still be in flight (not blocked by B, nor B by A)"); + assert_eq!(sess_b.finished(), 1); + }) + .await + .expect("parallel scenario hung — A likely blocked B (cross-agent contention)"); +} + +/// **A0 #3 — libération du verrou sur erreur.** Un 1ᵉ tour qui se solde par une +/// **erreur** (flux sans `Final` ⇒ `PROCESS`) doit néanmoins **relâcher** le +/// verrou de la cible : un `ask` suivant vers le **même** agent peut alors +/// acquérir le verrou et démarrer (pas de verrou fuité / pas de deadlock). +#[tokio::test] +async fn ask_releases_lock_after_errored_turn() { + let agent = scratch_agent(aid(1), "architect", "agents/architect.md"); + // 1ᵉ tour : flux vide ⇒ send_blocking renvoie une erreur (PROCESS). + let session = GatedSession::new(sid(500), GateMode::NoFinal); + let throwaway = FakeSession::new(sid(999), SendScript::Stream(vec![final_("unused")])); + let fx = ask_fixture( + FakeContexts::with_agent(&agent, "# persona"), + throwaway, + false, + ); + fx.structured + .insert(Arc::clone(&session) as Arc, aid(1), nid(1)); + + let proj = project(); + let svc = &fx.service; + + timeout(TEST_GUARD, async { + // 1ᵉ tour : on le débloque immédiatement, il échoue (pas de Final). + session.release_one(); + let err = svc + .dispatch(&proj, cmd(ASK_ARCHITECT)) + .await + .expect_err("first turn errors (no Final)"); + assert_eq!(err.code(), "PROCESS", "errored turn surfaces typed PROCESS: {err:?}"); + assert_eq!(session.started(), 1); + + // 2ᵉ tour vers la MÊME cible : si le verrou avait fuité sur l'erreur, ce + // `dispatch` resterait bloqué sur `lock_owned()` et le garde-fou sauterait. + // On débloque le 2ᵉ tour et on attend qu'il RENDE LA MAIN — preuve que le + // verrou était libre. (Cette session est `NoFinal` ⇒ le 2ᵉ tour échoue lui + // aussi ; ce qui compte ici est qu'il a pu *acquérir le verrou et démarrer*, + // pas l'issue du tour — l'issue est couverte par le test FIFO.) + session.release_one(); + let err2 = svc + .dispatch(&proj, cmd(ASK_ARCHITECT_2)) + .await + .expect_err("NoFinal session ⇒ second turn also errors typed"); + assert_eq!(err2.code(), "PROCESS", "got {err2:?}"); + assert_eq!( + session.started(), + 2, + "second turn actually started — the lock was freed after the errored first turn" + ); + }) + .await + .expect("lock appears leaked after an errored turn (second ask never acquired it)"); +} + +// A0 #4 — Plafond d'attente en file (`ASK_QUEUE_WAIT_CAP = 600 s`). NON testé en +// unitaire : il n'est ni injectable ni réductible depuis l'extérieur (constante +// privée), et un vrai test exigerait d'attendre 600 s — exclu (durée). La +// sémantique nominale (attente puis acquisition du verrou) est couverte par +// `ask_same_target_serialises_turns_fifo` (le 2ᵉ tour attend en file PUIS +// acquiert). Le chemin d'expiration (timeout d'attente ⇒ `AppError::Process`, +// même type que le timeout de tour) reste non couvert ici — signalé au Dev : +// rendre le cap injectable (ex. champ optionnel surchargeable en test) permettrait +// un test déterministe court. diff --git a/crates/application/tests/reconcile_layouts.rs b/crates/application/tests/reconcile_layouts.rs new file mode 100644 index 0000000..0c8dd5c --- /dev/null +++ b/crates/application/tests/reconcile_layouts.rs @@ -0,0 +1,347 @@ +//! R0c tests for [`ReconcileLayouts`]. +//! +//! At project open, a persisted `layouts.json` may already hold several leaves +//! pinning the **same** agent id. The use case de-duplicates them through the +//! pure domain op [`domain::LayoutTree::reconcile_duplicate_agents`] and +//! persists the doc **only** when something changed (idempotent no-op +//! otherwise). +//! +//! Harness mirrors `tests/snapshot_running_agents.rs` (its twin use case): the +//! same in-memory `FakeFs` / `FakeStore`. The fake FS additionally counts the +//! number of `write` **calls** so test 7 can prove that a no-op performs zero +//! writes (file-count alone cannot, since `layouts.json` already exists). + +use std::collections::{HashMap, HashSet}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use domain::layout::Workspace; +use domain::ports::{DirEntry, FileSystem, FsError, ProjectStore, RemotePath, StoreError}; +use domain::{ + AgentId, Direction, LayoutId, LayoutNode, LayoutTree, LeafCell, NodeId, Project, ProjectId, + ProjectPath, RemoteRef, SplitContainer, WeightedChild, +}; +use uuid::Uuid; + +use application::{ReconcileLayouts, ReconcileLayoutsInput}; + +// --------------------------------------------------------------------------- +// Fakes +// --------------------------------------------------------------------------- + +#[derive(Default)] +struct FakeFsInner { + files: HashMap>, + dirs: HashSet, +} + +#[derive(Default, Clone)] +struct FakeFs { + inner: Arc>, + /// Count of `write` **calls** (not files). Lets a no-op be asserted even + /// though the target file already exists. + write_calls: Arc, +} + +impl FakeFs { + fn read_file(&self, path: &str) -> Option> { + self.inner.lock().unwrap().files.get(path).cloned() + } + fn put(&self, path: &str, data: &[u8]) { + self.inner + .lock() + .unwrap() + .files + .insert(path.to_owned(), data.to_vec()); + } + fn write_calls(&self) -> usize { + self.write_calls.load(Ordering::SeqCst) + } +} + +#[async_trait] +impl FileSystem for FakeFs { + async fn read(&self, path: &RemotePath) -> Result, FsError> { + self.inner + .lock() + .unwrap() + .files + .get(path.as_str()) + .cloned() + .ok_or_else(|| FsError::NotFound(path.as_str().to_owned())) + } + async fn write(&self, path: &RemotePath, data: &[u8]) -> Result<(), FsError> { + self.write_calls.fetch_add(1, Ordering::SeqCst); + self.inner + .lock() + .unwrap() + .files + .insert(path.as_str().to_owned(), data.to_vec()); + Ok(()) + } + async fn exists(&self, path: &RemotePath) -> Result { + let inner = self.inner.lock().unwrap(); + Ok(inner.files.contains_key(path.as_str()) || inner.dirs.contains(path.as_str())) + } + async fn create_dir_all(&self, path: &RemotePath) -> Result<(), FsError> { + self.inner + .lock() + .unwrap() + .dirs + .insert(path.as_str().to_owned()); + Ok(()) + } + async fn list(&self, _path: &RemotePath) -> Result, FsError> { + Ok(Vec::new()) + } + async fn symlink(&self, _src: &RemotePath, _dst: &RemotePath) -> Result<(), FsError> { + Ok(()) + } +} + +#[derive(Default)] +struct FakeStoreInner { + projects: Vec, +} + +#[derive(Default, Clone)] +struct FakeStore(Arc>); + +#[async_trait] +impl ProjectStore for FakeStore { + async fn list_projects(&self) -> Result, StoreError> { + Ok(self.0.lock().unwrap().projects.clone()) + } + async fn load_project(&self, id: ProjectId) -> Result { + self.0 + .lock() + .unwrap() + .projects + .iter() + .find(|p| p.id == id) + .cloned() + .ok_or(StoreError::NotFound) + } + async fn save_project(&self, project: &Project) -> Result<(), StoreError> { + self.0.lock().unwrap().projects.push(project.clone()); + Ok(()) + } + async fn save_workspace(&self, _w: &Workspace) -> Result<(), StoreError> { + Ok(()) + } + async fn load_workspace(&self) -> Result { + Ok(Workspace::default()) + } +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const ROOT: &str = "/home/me/proj"; +const LAYOUTS_PATH: &str = "/home/me/proj/.ideai/layouts.json"; + +fn pid(n: u128) -> ProjectId { + ProjectId::from_uuid(Uuid::from_u128(n)) +} +fn nid(n: u128) -> NodeId { + NodeId::from_uuid(Uuid::from_u128(n)) +} +fn aid(n: u128) -> AgentId { + AgentId::from_uuid(Uuid::from_u128(n)) +} +fn lid(n: u128) -> LayoutId { + LayoutId::from_uuid(Uuid::from_u128(n)) +} + +fn agent_leaf(node: NodeId, agent: Option, conv: Option<&str>, running: bool) -> LeafCell { + LeafCell { + id: node, + session: None, + agent, + conversation_id: conv.map(str::to_string), + agent_was_running: running, + } +} + +/// Two leaves of the same agent, both flagged running, under a Row split. +fn duplicate_tree(agent: AgentId, a: NodeId, b: NodeId) -> LayoutTree { + LayoutTree::new(LayoutNode::Split(SplitContainer { + id: nid(1), + direction: Direction::Row, + children: vec![ + WeightedChild { + node: LayoutNode::Leaf(agent_leaf(a, Some(agent), Some("c-a"), true)), + weight: 1.0, + }, + WeightedChild { + node: LayoutNode::Leaf(agent_leaf(b, Some(agent), Some("c-b"), true)), + weight: 1.0, + }, + ], + })) +} + +async fn register_project(store: &FakeStore, id: ProjectId) { + let project = Project::new( + id, + "Demo", + ProjectPath::new(ROOT).unwrap(), + RemoteRef::Local, + 0, + ) + .unwrap(); + store.save_project(&project).await.unwrap(); +} + +fn seed_layouts(fs: &FakeFs, id: LayoutId, tree: &LayoutTree) { + let doc = serde_json::json!({ + "version": 1, + "activeId": id.to_string(), + "layouts": [ { "id": id.to_string(), "name": "Default", "tree": tree } ], + }); + fs.put(LAYOUTS_PATH, &serde_json::to_vec(&doc).unwrap()); +} + +fn doc_json(fs: &FakeFs) -> serde_json::Value { + serde_json::from_slice(&fs.read_file(LAYOUTS_PATH).expect("layouts.json present")).unwrap() +} + +/// `agent_was_running` for the leaf `node` in the active persisted layout. +fn was_running(fs: &FakeFs, node: NodeId) -> Option { + let doc = doc_json(fs); + let tree: LayoutTree = + serde_json::from_value(doc["layouts"][0]["tree"].clone()).expect("tree parseable"); + fn find(node: &LayoutNode, target: NodeId) -> Option { + match node { + LayoutNode::Leaf(l) if l.id == target => Some(l.agent_was_running), + LayoutNode::Leaf(_) => None, + LayoutNode::Split(s) => s.children.iter().find_map(|c| find(&c.node, target)), + LayoutNode::Grid(g) => g.cells.iter().find_map(|c| find(&c.node, target)), + } + } + find(&tree.root, node) +} + +fn make_use_case(store: &FakeStore, fs: &FakeFs) -> ReconcileLayouts { + ReconcileLayouts::new( + Arc::new(store.clone()) as Arc, + Arc::new(fs.clone()) as Arc, + ) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +/// 6. A persisted layout with a duplicate agent ⇒ `changed == true` and the +/// persisted version keeps a single "was running" leaf for the agent. +#[tokio::test] +async fn reconcile_persists_and_reports_changed_on_duplicate() { + let store = FakeStore::default(); + let fs = FakeFs::default(); + register_project(&store, pid(1)).await; + + let a = nid(10); + let b = nid(11); + let agent = aid(100); + seed_layouts(&fs, lid(1), &duplicate_tree(agent, a, b)); + + let uc = make_use_case(&store, &fs); + let out = uc + .execute(ReconcileLayoutsInput { project_id: pid(1) }) + .await + .unwrap(); + + assert_eq!(out.changed, true, "duplicate must be reconciled"); + + // Exactly one of the two leaves is still flagged running in the persisted doc. + let ra = was_running(&fs, a).expect("leaf a"); + let rb = was_running(&fs, b).expect("leaf b"); + assert_eq!( + [ra, rb].iter().filter(|x| **x).count(), + 1, + "only one host stays running after reconciliation" + ); + // First in pre-order is the host (both carried a signal). + assert_eq!(ra, true); + assert_eq!(rb, false); +} + +/// 7. No-op: a second `execute` on an already-reconciled project ⇒ +/// `changed == false` and **zero** writes occur. +#[tokio::test] +async fn reconcile_second_pass_is_noop_no_write() { + let store = FakeStore::default(); + let fs = FakeFs::default(); + register_project(&store, pid(1)).await; + + let a = nid(10); + let b = nid(11); + let agent = aid(100); + seed_layouts(&fs, lid(1), &duplicate_tree(agent, a, b)); + + let uc = make_use_case(&store, &fs); + + // First pass reconciles + writes once. + let first = uc + .execute(ReconcileLayoutsInput { project_id: pid(1) }) + .await + .unwrap(); + assert_eq!(first.changed, true); + assert_eq!(fs.write_calls(), 1, "first pass persists once"); + + // Second pass must be a strict no-op. + let writes_before = fs.write_calls(); + let second = uc + .execute(ReconcileLayoutsInput { project_id: pid(1) }) + .await + .unwrap(); + assert_eq!(second.changed, false, "already reconciled → no change"); + assert_eq!( + fs.write_calls(), + writes_before, + "no-op must not write anything" + ); +} + +/// 8. A layout WITHOUT duplicates ⇒ `changed == false`, unchanged, no write. +#[tokio::test] +async fn reconcile_no_duplicate_is_noop() { + let store = FakeStore::default(); + let fs = FakeFs::default(); + register_project(&store, pid(1)).await; + + let a = nid(10); + let b = nid(11); + // Distinct agents — no duplicate. + let tree = LayoutTree::new(LayoutNode::Split(SplitContainer { + id: nid(1), + direction: Direction::Row, + children: vec![ + WeightedChild { + node: LayoutNode::Leaf(agent_leaf(a, Some(aid(100)), Some("c-a"), true)), + weight: 1.0, + }, + WeightedChild { + node: LayoutNode::Leaf(agent_leaf(b, Some(aid(101)), None, false)), + weight: 1.0, + }, + ], + })); + seed_layouts(&fs, lid(1), &tree); + let before = doc_json(&fs); + let writes_before = fs.write_calls(); + + let uc = make_use_case(&store, &fs); + let out = uc + .execute(ReconcileLayoutsInput { project_id: pid(1) }) + .await + .unwrap(); + + assert_eq!(out.changed, false, "no duplicate → no change"); + assert_eq!(fs.write_calls(), writes_before, "no write on no-op"); + assert_eq!(doc_json(&fs), before, "persisted doc unchanged"); + assert_eq!(was_running(&fs, a), Some(true)); +} diff --git a/crates/application/tests/structured_launch_d3.rs b/crates/application/tests/structured_launch_d3.rs index 22c356b..b874338 100644 --- a/crates/application/tests/structured_launch_d3.rs +++ b/crates/application/tests/structured_launch_d3.rs @@ -770,37 +770,119 @@ async fn non_structured_profile_takes_pty_path_unchanged() { assert_eq!(out.session.id, sid(777)); } -/// Invariant « 1 session vivante/agent » côté structuré : relancer un agent -/// structuré déjà vivant ⇒ rebind/idempotent (pas de 2e `factory.start`). +/// Invariant « 1 agent = 1 employé » côté structuré (R0a) : un **lancement neuf** +/// (sans `conversation_id`) sur une **autre** cellule B d'un agent structuré déjà +/// vivant en cellule A est un second lancement ⇒ **refus** +/// `AppError::AgentAlreadyRunning { node_id: A }`, **pas** de 2e `factory.start`, +/// une seule session structurée vivante. #[tokio::test] -async fn structured_relaunch_is_idempotent_no_second_start() { +async fn structured_launch_new_in_other_cell_refuses_when_live_elsewhere() { let factory = FakeFactory::new(500, Some("engine-conv")); let f = launch_fixture(structured_profile(pid(9)), factory); // 1er lancement sur la cellule A. + let host = nid(1); let mut first = launch_input(f.agent.id); - first.node_id = Some(nid(1)); + first.node_id = Some(host); f.launch.execute(first).await.expect("first launch"); assert_eq!(f.factory.start_count(), 1); assert_eq!(f.structured.len(), 1); - // Relance dans une cellule B : rebind de la vue, PAS de 2e start. + // Lancement NEUF (conversation_id: None) dans une cellule B ⇒ refus. let mut second = launch_input(f.agent.id); - second.node_id = Some(nid(2)); - let out = f.launch.execute(second).await.expect("relaunch"); + let target = nid(2); + second.node_id = Some(target); + let err = second_launch_err(&f, second).await; + + match err { + application::AppError::AgentAlreadyRunning { agent_id, node_id } => { + assert_eq!(agent_id, f.agent.id, "reports the live agent"); + assert_eq!(node_id, host, "reports the live HOST node A, not target B"); + } + other => panic!("expected AgentAlreadyRunning, got {other:?}"), + } assert_eq!( f.factory.start_count(), 1, - "no second factory.start on relaunch of a live structured agent" + "no second factory.start on a refused launch" ); assert_eq!(f.pty.spawn_count(), 0, "still no pty spawn"); assert_eq!(f.structured.len(), 1, "still a single live structured session"); - // La vue est rebindée sur la cellule B. - assert_eq!(f.structured.node_for_agent(&f.agent.id), Some(nid(2))); + assert_eq!( + f.structured.node_for_agent(&f.agent.id), + Some(host), + "session stays pinned on its host node A" + ); +} + +/// **Réattache légitime même node (R0a, structuré)** : relancer sur la **même** +/// cellule hôte ⇒ rebind, pas d'erreur, pas de 2e `factory.start`, même session. +#[tokio::test] +async fn structured_relaunch_same_node_rebinds_no_second_start() { + let factory = FakeFactory::new(500, Some("engine-conv")); + let f = launch_fixture(structured_profile(pid(9)), factory); + + let host = nid(1); + let mut first = launch_input(f.agent.id); + first.node_id = Some(host); + f.launch.execute(first).await.expect("first launch"); + assert_eq!(f.factory.start_count(), 1); + + // Re-ouverture de la MÊME cellule. + let mut again = launch_input(f.agent.id); + again.node_id = Some(host); + let out = f.launch.execute(again).await.expect("same-node rebind"); + + assert_eq!(f.factory.start_count(), 1, "no second factory.start"); + assert_eq!(f.pty.spawn_count(), 0, "still no pty spawn"); + assert_eq!(f.structured.len(), 1, "single live structured session"); + assert_eq!(f.structured.node_for_agent(&f.agent.id), Some(host)); let desc = out.structured.expect("descriptor on rebind"); assert_eq!(desc.session_id, sid(500), "same live session id"); - assert_eq!(desc.node_id, nid(2)); + assert_eq!(desc.node_id, host); +} + +/// **Réattache explicite par conversation (R0a, structuré)** : relancer sur une +/// **autre** cellule B mais avec un `conversation_id` ⇒ rebind légitime de la vue, +/// pas d'erreur, pas de 2e `factory.start`, même session, vue déplacée sur B. +#[tokio::test] +async fn structured_relaunch_other_cell_with_conversation_id_rebinds() { + let factory = FakeFactory::new(500, Some("engine-conv")); + let f = launch_fixture(structured_profile(pid(9)), factory); + + let host = nid(1); + let mut first = launch_input(f.agent.id); + first.node_id = Some(host); + f.launch.execute(first).await.expect("first launch"); + assert_eq!(f.factory.start_count(), 1); + + // Cellule B + signal de réattache explicite. + let mut second = launch_input(f.agent.id); + let target = nid(2); + second.node_id = Some(target); + second.conversation_id = Some("conv-live".to_owned()); + let out = f.launch.execute(second).await.expect("explicit reattach"); + + assert_eq!( + f.factory.start_count(), + 1, + "no second factory.start on explicit reattach" + ); + assert_eq!(f.pty.spawn_count(), 0, "still no pty spawn"); + assert_eq!(f.structured.len(), 1, "still a single live structured session"); + assert_eq!(f.structured.node_for_agent(&f.agent.id), Some(target)); + let desc = out.structured.expect("descriptor on rebind"); + assert_eq!(desc.session_id, sid(500), "same live session id"); + assert_eq!(desc.node_id, target); +} + +/// Helper: executes a launch that is expected to be refused, returning the error. +async fn second_launch_err(f: &LaunchFixture, input: LaunchAgentInput) -> application::AppError { + f.launch + .execute(input) + .await + .expect_err("fresh second launch elsewhere is refused") } // =========================================================================== diff --git a/crates/domain/src/layout.rs b/crates/domain/src/layout.rs index d2f76ef..f2fc325 100644 --- a/crates/domain/src/layout.rs +++ b/crates/domain/src/layout.rs @@ -620,6 +620,87 @@ impl LayoutTree { out } + /// Réconcilie les feuilles en doublon sur un même agent : à la réouverture + /// d'un projet, un `layouts.json` persisté peut contenir **plusieurs** + /// feuilles portant le **même** `agent` id (constaté). L'invariant produit + /// est **« 1 session vivante par agent »** : une seule feuille doit rester + /// « hôte » (potentiellement vivante / reprenable), les autres deviennent des + /// **vues mortes** — elles gardent leur agent (la cellule reste), mais ne sont + /// plus considérées « était en cours » : leur `agent_was_running` repasse à + /// `false` et leur `conversation_id` est retiré, de sorte qu'aucune reprise ni + /// relance ne les vise. + /// + /// ## Règle déterministe de choix de l'hôte + /// + /// Parmi les N feuilles d'un même agent, **dans l'ordre de parcours + /// déterministe de l'arbre** (le même pré-ordre que [`Self::agent_leaves`]), + /// l'hôte est la **première feuille porteuse d'un signal de reprise** + /// (`conversation_id.is_some()` **ou** `agent_was_running`) ; à défaut de tout + /// signal, l'hôte est simplement la **première** feuille rencontrée. On garde + /// ainsi sur l'unique hôte l'état de reprise le plus pertinent, et le choix est + /// totalement déterministe (l'ordre de parcours est stable). + /// + /// ## Idempotence + /// + /// Un arbre **sans doublon** est renvoyé **inchangé** (`self.clone()` à + /// l'identique). Un arbre **déjà réconcilié** (≤ 1 feuille par agent porte un + /// signal de reprise) l'est aussi : la 2ᵉ passe ne dé-flagge rien. La fonction + /// est donc un point fixe — utile pour garantir le no-op à la 2ᵉ ouverture. + /// + /// Pure : renvoie un nouvel arbre (non revalidé — la réconciliation ne touche + /// ni la structure ni les sessions, seuls des champs scalaires de feuille). + #[must_use] + pub fn reconcile_duplicate_agents(&self) -> Self { + use std::collections::{HashMap, HashSet}; + + // Première passe (lecture seule) : pour chaque agent, déterminer le node + // hôte selon la règle déterministe ci-dessus. + let mut host_of: HashMap = HashMap::new(); + let mut has_signal: HashSet = HashSet::new(); + for (node_id, agent_id) in self.agent_leaves() { + // `leaf` ne peut pas être `None` ici : le node vient de l'arbre. + let signal = self + .leaf(node_id) + .is_some_and(|l| l.conversation_id.is_some() || l.agent_was_running); + match host_of.get(&agent_id) { + // Pas encore d'hôte : cette feuille le devient (provisoirement). + None => { + host_of.insert(agent_id, node_id); + if signal { + has_signal.insert(agent_id); + } + } + // Un hôte sans signal est supplanté par la première feuille à + // signal rencontrée ensuite. + Some(_) if signal && !has_signal.contains(&agent_id) => { + host_of.insert(agent_id, node_id); + has_signal.insert(agent_id); + } + Some(_) => {} + } + } + + // Seconde passe : dé-flagger toute feuille d'agent qui n'est pas son hôte. + let root = map_node(&self.root, &mut |node| { + if let LayoutNode::Leaf(leaf) = node { + if let Some(agent) = leaf.agent { + let is_host = host_of.get(&agent) == Some(&leaf.id); + if !is_host && (leaf.agent_was_running || leaf.conversation_id.is_some()) { + return LayoutNode::Leaf(LeafCell { + id: leaf.id, + session: leaf.session, + agent: leaf.agent, + conversation_id: None, + agent_was_running: false, + }); + } + } + } + node.clone() + }); + Self { root } + } + /// Retrouve la [`LeafCell`] portant l'identifiant `node`. /// /// Renvoie `None` si aucun node ne porte cet id, **ou** si le node existe mais diff --git a/crates/domain/tests/layout.rs b/crates/domain/tests/layout.rs index 528ba02..155cc7a 100644 --- a/crates/domain/tests/layout.rs +++ b/crates/domain/tests/layout.rs @@ -826,6 +826,209 @@ fn move_session_preserves_resume_fields_on_both_leaves() { ); } +// --------------------------------------------------------------------------- +// reconcile_duplicate_agents (R0c: dé-doublonnage à l'ouverture) +// --------------------------------------------------------------------------- + +/// Builds an agent-bearing leaf with explicit resume signals. +fn agent_leaf_full( + id: u128, + agent: u128, + conv: Option<&str>, + running: bool, +) -> LeafCell { + LeafCell { + id: node(id), + session: None, + agent: Some(agent_id(agent)), + conversation_id: conv.map(str::to_string), + agent_was_running: running, + } +} + +/// Wraps an ordered list of leaves into a single Row split (pre-order = the +/// children order, same traversal as `agent_leaves`). +fn split_of(leaves: Vec) -> LayoutTree { + LayoutTree::new(LayoutNode::Split(SplitContainer { + id: node(900), + direction: Direction::Row, + children: leaves + .into_iter() + .map(|l| WeightedChild { + node: LayoutNode::Leaf(l), + weight: 1.0, + }) + .collect(), + })) +} + +/// 1. Two leaves of the SAME agent, both `agent_was_running = true`: after +/// reconciliation exactly ONE stays running; the other is neutralised +/// (running=false, conversation_id=None) but the leaf/agent SURVIVES. +#[test] +fn reconcile_basic_duplicate_keeps_single_host() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, None, true), + agent_leaf_full(2, 7, None, true), + ]); + let out = tree.reconcile_duplicate_agents(); + + // First in pre-order is the host (no resume signal differentiates them). + let host = leaf_for(&out, node(1)).expect("leaf 1"); + let other = leaf_for(&out, node(2)).expect("leaf 2"); + + assert_eq!(host.agent_was_running, true, "host stays running"); + assert_eq!(other.agent_was_running, false, "duplicate neutralised"); + assert_eq!(other.conversation_id, None); + + // The leaf and its agent binding still exist (cell not removed). + assert_eq!(host.agent, Some(agent_id(7))); + assert_eq!(other.agent, Some(agent_id(7))); + + // Exactly one running leaf for the agent. + let running = [&host, &other] + .iter() + .filter(|l| l.agent_was_running) + .count(); + assert_eq!(running, 1); +} + +/// 2a. Host chosen by resume signal: the FIRST leaf carries no signal, the +/// SECOND carries one (conversation_id). The second must become the host; the +/// first (no signal) is left untouched (already neutral). +#[test] +fn reconcile_host_is_first_leaf_carrying_resume_signal() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, None, false), // no signal, first in order + agent_leaf_full(2, 7, Some("conv-2"), false), // signal → becomes host + ]); + let out = tree.reconcile_duplicate_agents(); + + let first = leaf_for(&out, node(1)).expect("leaf 1"); + let second = leaf_for(&out, node(2)).expect("leaf 2"); + + assert_eq!( + second.conversation_id, + Some("conv-2".to_string()), + "the signal-bearing leaf is kept as host" + ); + // The first leaf has no signal to strip; stays neutral. + assert_eq!(first.conversation_id, None); + assert_eq!(first.agent_was_running, false); +} + +/// 2b. When NO leaf carries a signal, the host is simply the first encountered. +/// Here both are inert; reconciliation must leave them unchanged (nothing to +/// strip), and not invent a running flag. +#[test] +fn reconcile_no_signal_host_is_first_and_tree_unchanged() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, None, false), + agent_leaf_full(2, 7, None, false), + ]); + let out = tree.reconcile_duplicate_agents(); + assert_eq!(out, tree, "no resume signal anywhere → nothing to strip"); +} + +/// 2c. The signal-bearing second leaf wins even against a first leaf that has a +/// signal too? No — the FIRST signal-bearer wins. Verify the first running leaf +/// stays host and the second (also running) is neutralised. +#[test] +fn reconcile_first_signal_bearer_wins_over_later_signal() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, Some("conv-1"), true), // first signal → host + agent_leaf_full(2, 7, Some("conv-2"), true), + ]); + let out = tree.reconcile_duplicate_agents(); + + let host = leaf_for(&out, node(1)).expect("leaf 1"); + let other = leaf_for(&out, node(2)).expect("leaf 2"); + + assert_eq!(host.conversation_id, Some("conv-1".to_string())); + assert_eq!(host.agent_was_running, true); + assert_eq!(other.conversation_id, None, "later duplicate stripped"); + assert_eq!(other.agent_was_running, false); +} + +/// 3. Triplet (N=3) of the same agent: exactly one host, two neutralised. +#[test] +fn reconcile_triplet_one_host_two_neutralised() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, None, true), + agent_leaf_full(2, 7, None, true), + agent_leaf_full(3, 7, Some("c3"), true), + ]); + let out = tree.reconcile_duplicate_agents(); + + let l1 = leaf_for(&out, node(1)).unwrap(); + let l2 = leaf_for(&out, node(2)).unwrap(); + let l3 = leaf_for(&out, node(3)).unwrap(); + + // First signal-bearer in pre-order is leaf 1 (running=true is a signal). + let running_count = [&l1, &l2, &l3] + .iter() + .filter(|l| l.agent_was_running) + .count(); + assert_eq!(running_count, 1, "exactly one host remains running"); + assert_eq!(l1.agent_was_running, true, "leaf 1 is the host"); + assert_eq!(l2.agent_was_running, false); + assert_eq!(l3.agent_was_running, false); + assert_eq!(l3.conversation_id, None, "duplicate conv stripped"); + // All three cells survive. + for l in [&l1, &l2, &l3] { + assert_eq!(l.agent, Some(agent_id(7))); + } +} + +/// 4. Distinct agents are never affected: two leaves with DIFFERENT agents, +/// each running, must both stay running. +#[test] +fn reconcile_distinct_agents_unaffected() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, Some("c1"), true), + agent_leaf_full(2, 8, Some("c2"), true), + ]); + let out = tree.reconcile_duplicate_agents(); + assert_eq!(out, tree, "different agents → no duplicates → unchanged"); +} + +/// 5a. Idempotence (fixed point): reconcile(reconcile(t)) == reconcile(t). +#[test] +fn reconcile_is_idempotent_fixed_point() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, Some("c1"), true), + agent_leaf_full(2, 7, Some("c2"), true), + agent_leaf_full(3, 7, None, true), + ]); + let once = tree.reconcile_duplicate_agents(); + let twice = once.reconcile_duplicate_agents(); + assert_eq!(twice, once, "second pass is a no-op (fixed point)"); +} + +/// 5b. A tree WITHOUT duplicates is returned unchanged (== t). +#[test] +fn reconcile_no_duplicate_returns_identical() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, Some("c1"), true), + agent_leaf_full(2, 8, None, false), + leaf(3, Some(100)), // plain non-agent leaf + ]); + let out = tree.reconcile_duplicate_agents(); + assert_eq!(out, tree, "no duplicate agent → identical tree"); +} + +/// Source immutability: reconciliation must not mutate the input tree. +#[test] +fn reconcile_is_immutable_source_unchanged() { + let tree = split_of(vec![ + agent_leaf_full(1, 7, None, true), + agent_leaf_full(2, 7, None, true), + ]); + let before = tree.clone(); + let _ = tree.reconcile_duplicate_agents(); + assert_eq!(tree, before, "source tree must not be mutated"); +} + // --------------------------------------------------------------------------- // serde: compat ascendante & combinaisons des nouveaux champs // --------------------------------------------------------------------------- diff --git a/frontend/src/features/layout/LayoutGrid.tsx b/frontend/src/features/layout/LayoutGrid.tsx index e6796ec..f573e1b 100644 --- a/frontend/src/features/layout/LayoutGrid.tsx +++ b/frontend/src/features/layout/LayoutGrid.tsx @@ -168,6 +168,37 @@ interface PendingResume { reject: (e: unknown) => void; } +/** + * A transient in-cell notice. `goToNodeId`, when present, identifies the layout + * leaf that already hosts the conflicting live agent; the notice then renders an + * "aller à la cellule" button that focuses that cell. An agent is a singleton + * ("1 agent = 1 employé"): we surface where it already lives instead of cloning. + */ +interface CellNotice { + message: string; + goToNodeId?: string; +} + +/** + * Focuses the layout leaf with the given node id: scrolls it into view and + * flashes a brief outline so the user sees where the agent already lives. Works + * off the `data-node-id` attribute each {@link LeafView} renders. Best-effort — + * a no-op when the node is not currently mounted (e.g. on another tab). + */ +function goToCell(nodeId: string): void { + if (typeof document === "undefined") return; + const el = document.querySelector(`[data-node-id="${nodeId}"]`); + if (!el) return; + // `scrollIntoView` is absent in some headless DOMs (jsdom); the highlight is + // the essential affordance, so guard the scroll and still flash the outline. + el.scrollIntoView?.({ block: "nearest", inline: "nearest" }); + const previous = el.style.outline; + el.style.outline = "2px solid var(--color-primary, #5b9bd5)"; + window.setTimeout(() => { + el.style.outline = previous; + }, 1200); +} + function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm, parentSplit, projectId, visibleNodeIds }: LeafViewProps) { // A cell can be closed only when it lives inside a (binary) split: closing it // collapses the parent split, keeping the *sibling*. Splits are always binary @@ -241,6 +272,32 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm const liveFor = (candidate: string): LiveAgent | undefined => liveAgents.find((la) => la.agentId === candidate); + /** + * Maps an error into a {@link CellNotice}. For the singleton-invariant refusal + * (`AGENT_ALREADY_RUNNING`) it resolves the host cell from the live-agents set + * (the source of truth exposed by R0b — PTY *and* chat) so the notice can + * offer "aller à la cellule" rather than relaunch a clone. The lookup is done + * against a *fresh* `listLiveAgents` call (the refusal often races the launch + * on mount, before the cached `liveAgents` state has settled), falling back to + * the cached set. The error message embeds the host node id too, but the + * structured live-agents lookup is preferred. + */ + const noticeFromError = async ( + err: unknown, + candidate: string, + ): Promise => { + const message = describeNotice(err); + if (!isAlreadyRunning(err)) return { message }; + const fresh = agentGateway?.listLiveAgents + ? await agentGateway.listLiveAgents(projectId).catch(() => liveAgents) + : liveAgents; + const host = fresh.find((la) => la.agentId === candidate); + return { + message: "Cet agent est déjà actif dans une autre cellule.", + goToNodeId: host && host.nodeId !== id ? host.nodeId : undefined, + }; + }; + /** True when the live session is already displayed by another visible cell. */ const visibleElsewhere = (candidate: string): LiveAgent | undefined => { const live = liveFor(candidate); @@ -259,7 +316,10 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm // A transient notice shown when an action is blocked by the singleton // invariant (selecting / launching an agent already live in another cell). - const [busyNotice, setBusyNotice] = useState(null); + // `goToNodeId`, when set, is the host cell of the conflicting live agent: the + // notice then offers an "aller à la cellule" action that focuses that cell — + // we never silently relaunch a clone (product rule "1 agent = 1 employé"). + const [busyNotice, setBusyNotice] = useState(null); // ── Resume popup state (T7) ─────────────────────────────────────────────── // When an agent cell carries a persisted conversation id and its PTY session @@ -316,12 +376,23 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm return result.handle; } - const handle = await agentGateway!.launchAgent( - projectId, - agentId!, - { ...opts, conversationId: convId, nodeId: id }, - onData, - ); + const handle = await agentGateway! + .launchAgent( + projectId, + agentId!, + { ...opts, conversationId: convId, nodeId: id }, + onData, + ) + .catch(async (err: unknown) => { + // A neat backend refusal (`AGENT_ALREADY_RUNNING`) means the agent is a + // singleton already alive in another cell: surface a clear notice with + // an "aller à la cellule" action (host resolved from the live set), then + // re-throw so the view layer does not treat the launch as succeeded. + if (isAlreadyRunning(err)) { + setBusyNotice(await noticeFromError(err, agentId!)); + } + throw err; + }); // First launch on a fresh cell mints a conversation id: persist it on the // leaf so the next open resumes (T4b — closes the persistence loop). if (handle.assignedConversationId) { @@ -448,14 +519,19 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm const isVisible = live && live.nodeId !== id && visibleNodeIds.has(live.nodeId); if (isVisible) { - setBusyNotice("Cet agent est déjà visible dans une autre cellule."); + setBusyNotice({ + message: "Cet agent est déjà actif dans une autre cellule.", + goToNodeId: live!.nodeId, + }); return; } const isBackground = live && live.nodeId !== id && !visibleNodeIds.has(live.nodeId); if (isBackground) { if (!agentGateway?.attachLiveAgent || !live.sessionId) { - setBusyNotice("Session active introuvable pour cet agent."); + setBusyNotice({ + message: "Session active introuvable pour cet agent.", + }); return; } setBusyNotice(null); @@ -466,7 +542,9 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm } setBusyNotice(null); await vm.setCellAgent(id, val); - })().catch((err: unknown) => setBusyNotice(describeNotice(err))); + })().catch(async (err: unknown) => + setBusyNotice(await noticeFromError(err, val)), + ); }} style={{ fontSize: 11, @@ -548,7 +626,7 @@ function LeafView({ id, session, agent, conversationId, agentWasRunning, cwd, vm /> )} {busyNotice && ( -

- {busyNotice} -

+ {busyNotice.message} + {busyNotice.goToNodeId && ( + + )} + )} {pendingResume && ( ({ + Terminal: class { + loadAddon() {} + open() {} + onData() { + return { dispose() {} }; + } + onResize() { + return { dispose() {} }; + } + write() {} + dispose() {} + get cols() { + return 80; + } + get rows() { + return 24; + } + }, +})); +vi.mock("@xterm/addon-fit", () => ({ + FitAddon: class { + fit() {} + }, +})); +vi.mock("@xterm/xterm/css/xterm.css", () => ({})); + +if (typeof globalThis.ResizeObserver === "undefined") { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +} + +import type { Gateways } from "@/ports"; +import { + MockAgentGateway, + MockLayoutGateway, + MockSystemGateway, + MockTerminalGateway, +} from "@/adapters/mock"; +import { DIProvider } from "@/app/di"; +import { leaves } from "./layout"; +import { LayoutGrid } from "./LayoutGrid"; + +function renderGrid(gateways: Gateways) { + return render( + + + , + ); +} + +describe("R0d — launch refused with AGENT_ALREADY_RUNNING", () => { + it("maps the refusal to a clear notice + 'aller à la cellule' on the host", async () => { + const layout = new MockLayoutGateway(); + const agentGateway = new MockAgentGateway(); + const terminal = new MockTerminalGateway(); + const system = new MockSystemGateway(); + + const agent = await agentGateway.createAgent("p1", { + name: "Solo", + profileId: "claude", + }); + + // Two visible leaves; pin the agent on cell A. + const initial = await layout.loadLayout("p1"); + const aId = leaves(initial)[0].id; + await layout.mutateLayout("p1", { + type: "split", + target: aId, + direction: "row", + newLeaf: "b", + container: "c", + }); + const after = leaves(await layout.loadLayout("p1")); + const bId = after.find((l) => l.id !== aId)!.id; + await layout.mutateLayout("p1", { + type: "setCellAgent", + target: aId, + agent: agent.id, + }); + + // The live set reports the agent already hosted in cell B (the host). + vi.spyOn(agentGateway, "listLiveAgents").mockResolvedValue([ + { agentId: agent.id, nodeId: bId, sessionId: "s-1" }, + ]); + // The launch from cell A is refused by the backend (singleton invariant). + vi.spyOn(agentGateway, "launchAgent").mockRejectedValue({ + code: "AGENT_ALREADY_RUNNING", + message: `agent ${agent.id} is already running in cell ${bId}`, + }); + + const gateways = { + layout, + agent: agentGateway, + terminal, + system, + } as unknown as Gateways; + renderGrid(gateways); + + // The leaf's terminal opener runs the launch → refused → mapped to a notice. + const status = await screen.findByRole("status"); + expect(status.textContent).toMatch(/déjà actif dans une autre cellule/); + // And NOT a raw error dump. + expect(status.textContent).not.toMatch(/AGENT_ALREADY_RUNNING/); + + // The go-to action focuses the live host cell (B): it gets an outline flash. + const goTo = await screen.findByRole("button", { + name: "aller à la cellule", + }); + fireEvent.click(goTo); + await waitFor(() => { + const host = document.querySelector( + `[data-node-id="${bId}"]`, + )!; + expect(host.style.outline).not.toBe(""); + }); + }); +}); diff --git a/frontend/src/features/layout/singletonAgent.test.tsx b/frontend/src/features/layout/singletonAgent.test.tsx index 94eb064..51c3929 100644 --- a/frontend/src/features/layout/singletonAgent.test.tsx +++ b/frontend/src/features/layout/singletonAgent.test.tsx @@ -200,3 +200,94 @@ describe("singleton agent — dropdown guard (T6)", () => { expect((option as HTMLOptionElement).disabled).toBe(false); }); }); + +describe("R0d — dropdown disables agents live elsewhere + go-to-cell", () => { + function renderGrid(layout: MockLayoutGateway, agent: MockAgentGateway) { + const system = { + onDomainEvent: vi.fn().mockResolvedValue(() => {}), + }; + const gateways = { layout, agent, system } as unknown as Gateways; + return render( + + + , + ); + } + + /** Splits the project's single leaf into two visible leaves; returns both ids. */ + async function splitInTwo( + layout: MockLayoutGateway, + ): Promise<{ a: string; b: string }> { + const initial = await layout.loadLayout("p1"); + const a = leaves(initial)[0].id; + await layout.mutateLayout("p1", { + type: "split", + target: a, + direction: "row", + newLeaf: "b", + container: "c", + }); + const after = leaves(await layout.loadLayout("p1")); + return { a, b: after.find((l) => l.id !== a)!.id }; + } + + it("an agent live in another VISIBLE cell ⇒ option disabled + 'aller à la cellule'", async () => { + const layout = new MockLayoutGateway(); + const agent = new MockAgentGateway(); + const ag = await agent.createAgent("p1", { name: "Busy", profileId: "p" }); + const { a, b } = await splitInTwo(layout); + + // The agent is live in cell B (a currently-visible leaf). + vi.spyOn(agent, "listLiveAgents").mockResolvedValue([ + { agentId: ag.id, nodeId: b, sessionId: "s-1" }, + ]); + + renderGrid(layout, agent); + + // In cell A's dropdown the option is DISABLED (cannot run in two cells). + const selectA = (await screen.findByLabelText( + `agent selector ${a}`, + )) as HTMLSelectElement; + await waitFor(() => { + const opt = Array.from(selectA.options).find((o) => o.value === ag.id)!; + expect(opt.disabled).toBe(true); + expect(opt.textContent).toMatch(/visible ailleurs/); + }); + + // Selecting it surfaces a clear notice + an "aller à la cellule" action. + fireEvent.change(selectA, { target: { value: ag.id } }); + expect((await screen.findByRole("status")).textContent).toMatch( + /déjà actif dans une autre cellule/, + ); + expect( + screen.getByRole("button", { name: "aller à la cellule" }), + ).toBeTruthy(); + }); + + it("an agent NOT live anywhere stays selectable (no regression)", async () => { + const layout = new MockLayoutGateway(); + const agent = new MockAgentGateway(); + const ag = await agent.createAgent("p1", { name: "Free", profileId: "p" }); + + // No live agents at all. + vi.spyOn(agent, "listLiveAgents").mockResolvedValue([]); + + renderGrid(layout, agent); + + const option = await screen.findByRole("option", { name: "Free" }); + expect((option as HTMLOptionElement).disabled).toBe(false); + + // And selecting it pins the agent on the cell (no notice, no block). + const leafId = leaves(await layout.loadLayout("p1"))[0].id; + const select = screen.getByRole("combobox") as HTMLSelectElement; + fireEvent.change(select, { target: { value: ag.id } }); + await waitFor(async () => { + const leaf = leaves(await layout.loadLayout("p1")).find( + (l) => l.id === leafId, + )!; + expect(leaf.agent).toBe(ag.id); + }); + expect(screen.queryByRole("status")).toBeNull(); + }); + +});