From 3fe39c01e665a27f36abc307fabef68d32b4f3f3 Mon Sep 17 00:00:00 2001 From: Carsten Koch Date: Sun, 4 Oct 2026 18:37:00 +0200 Subject: [PATCH 1/4] One persona definition for every repository: a user-level declaration folder, merged per name (nxf 6j6v.7k58) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beside a workspace's own `.nxs-personas/`, every workspace reads the user-level folder `/personas` — `~/.nexusflow/personas` for the installed suite and an embedding app, `~/.nexusflow-/personas` for a development build. A persona or channel the repository declares hides the user-level one of the same name; everything else is added. - The merge lives in `Definitions::resolve` alone, so the CLI, every `Engine` verb, `nxs prime` and a spawned persona's own `nxc` see one team. Duplicates are judged per folder (naming it); handle form and flow cycles on the merged team. - `DeclarationSource` carries `user_path`, `from_user` and `shadowed`, all off the wire when there is no user-level declaration, so such a machine reads exactly as before (point 8). - `nxc list` / `Engine::directory` stamp `origin: user` per entry and print the account whenever the user-level folder takes part; `nxs prime` says it under "Declarations". - A user-level persona's session start says where its declaration lives: a relative path in a prompt means the repository the session runs in, never the declaration's folder. - The freeze projects the merged catalogue, so it holds for user-level hurdles and prompts; proved by an edit mid-operation. - The readers that loaded the folder directly (thread/search channel policy, list, prime) go through the merged catalogue now. Proved through the real binary: two repositories without a pm file both run the user-level pm, the commission scenario runs on it, a repository file hides it out loud; the live test with real sessions now uses one user-level pm for both repositories (22 s, green). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018Sipejn8YQhKgngxeHpMhT --- changes/7k58-one-definition-many-repos.md | 37 +++ crates/chat/docs/guide/de/personas.md | 51 +++- crates/chat/docs/guide/en/personas.md | 49 +++- crates/chat/src/cli.rs | 57 ++-- crates/chat/src/definitions.rs | 261 +++++++++++++++++- crates/chat/src/engine.rs | 22 +- crates/chat/src/facade.rs | 54 +++- crates/chat/src/persona.rs | 60 +++- crates/chat/src/workspace.rs | 5 +- ...vel_declaration_is_frozen_per_operation.rs | 146 ++++++++++ .../one_definition_at_the_engine_seam.rs | 95 +++++++ .../tests/one_definition_many_repositories.rs | 237 ++++++++++++++++ .../tests/two_workspaces_on_one_machine.rs | 120 ++++++-- crates/service/src/home.rs | 15 + 14 files changed, 1130 insertions(+), 79 deletions(-) create mode 100644 changes/7k58-one-definition-many-repos.md create mode 100644 crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs create mode 100644 crates/chat/tests/one_definition_at_the_engine_seam.rs create mode 100644 crates/chat/tests/one_definition_many_repositories.rs diff --git a/changes/7k58-one-definition-many-repos.md b/changes/7k58-one-definition-many-repos.md new file mode 100644 index 0000000..50c5b67 --- /dev/null +++ b/changes/7k58-one-definition-many-repos.md @@ -0,0 +1,37 @@ +--- +type: added +facade: breaking +--- +[en] +One persona definition for every repository on a machine. Beside a workspace's own +`.nxs-personas/`, every workspace now reads the user-level folder `~/.nexusflow/personas/` — +personas and a `channels.yaml`, laid out the same way — merged per name: a persona or channel the +repository declares itself hides the user-level one of the same name, everything else is added. One +`pm` written once runs in every repository as that repository's own participant, with its board, +memory and working copy. `nxc list` marks each entry that comes from the user-level folder and says +which ones the repository hides, and so does `nxs prime`; a persona from it is told at session start +where its declaration lives, because a path in its instructions relative to "this repository" means +the repository it runs in. The per-operation freeze holds for it as for the repository's own folder. +A development build reads `~/.nexusflow-/personas/` of its own service instance. A machine +without the folder behaves exactly as before. For embedding apps: `Engine::definitions`, +`Engine::directory` and `Engine::prime_as` read the same folder (an app cannot choose another); +new `user_path`, `from_user` and `shadowed` on `DeclarationSource`, `origin` on `PersonaEntry` and +`ChannelEntry`, `declared_in` on `PersonaBrief`, and `Definitions::resolve_with_user_dir` / +`Definitions::from_source` — a struct literal of those four types needs the new fields. +[de] +Eine Persona-Definition für jedes Repository auf einer Maschine. Neben dem eigenen `.nxs-personas/` +liest jeder Arbeitsbereich jetzt den Ordner auf Benutzerebene `~/.nexusflow/personas/` — Personas +und eine `channels.yaml`, genauso aufgebaut — und führt beide je Name zusammen: Eine Persona oder ein +Kanal, den das Repository selbst deklariert, verdeckt den gleichnamigen Eintrag auf Benutzerebene, +alles andere kommt hinzu. Ein einmal geschriebener `pm` läuft in jedem Repository als dessen eigener +Teilnehmer, mit dessen Board, Gedächtnis und Arbeitskopie. `nxc list` markiert jeden Eintrag aus dem +Ordner auf Benutzerebene und sagt, welche das Repository verdeckt, `nxs prime` ebenso; eine Persona +von dort erfährt beim Sitzungsstart, wo ihre Deklaration liegt, denn ein Pfad in ihren Anweisungen +relativ zu „diesem Repository" meint das Repository, in dem sie läuft. Das Einfrieren je Vorgang gilt +dort wie für den eigenen Ordner. Ein Entwicklungs-Build liest `~/.nexusflow-/personas/` seiner +eigenen Dienst-Instanz. Eine Maschine ohne den Ordner verhält sich genau wie bisher. Für einbettende +Apps: `Engine::definitions`, `Engine::directory` und `Engine::prime_as` lesen denselben Ordner (eine +App kann keinen anderen wählen); neu sind `user_path`, `from_user` und `shadowed` an +`DeclarationSource`, `origin` an `PersonaEntry` und `ChannelEntry`, `declared_in` an `PersonaBrief` +sowie `Definitions::resolve_with_user_dir` / `Definitions::from_source` — ein Struct-Literal dieser +vier Typen braucht die neuen Felder. diff --git a/crates/chat/docs/guide/de/personas.md b/crates/chat/docs/guide/de/personas.md index 6678f18..f662fbf 100644 --- a/crates/chat/docs/guide/de/personas.md +++ b/crates/chat/docs/guide/de/personas.md @@ -286,7 +286,10 @@ startet seine Persona in SEINER Arbeitskopie, mit SEINEM Board und Gedächtnis, beauftragt, um zu antworten, bleibt in ihrem Arbeitsbereich. Eine Rückfrage (`--escalate`) geht an den Auftraggeber zurück — nie an ihm vorbei zum Besitzer des anderen Arbeitsbereichs —, und die Antwort des Auftraggebers auf demselben Faden setzt die Persona fort. Die Tiefengrenze gilt für die -ganze Kette über beide Arbeitsbereiche. `nxc status` zeigt den Faden in beiden Arbeitsbereichen mit +ganze Kette über beide Arbeitsbereiche — auf das Wort des Aufrufers: Die Tiefe reist im signierten +Stempel des Auftrags, und der Empfänger kann sie nicht gegen eine Kette prüfen, die er nie sieht. Einem +Arbeitsbereich zu vertrauen heißt, seinem Koordinator zu vertrauen, und ein feindseliger könnte +Tiefe 0 stempeln; vertrauen Sie nur Arbeitsbereichen, deren Koordinator Sie selbst laufen ließen. `nxc status` zeigt den Faden in beiden Arbeitsbereichen mit der Gegenseite und einem von vier Zuständen: `submitted`, `working`, `input-required`, `completed` (oder `rejected` / `canceled`). Zwei Stunden ohne Lebenszeichen — keine Nachricht auf dem Faden, nichts im Transkript der empfangenden Persona — brechen ihn ab und wecken den Auftraggeber mit dem @@ -499,6 +502,52 @@ gemeldet. Der Rat oben heißt deshalb nicht „das Werkzeug passt schon auf": es Versionsverwaltung, die aufpasst, und dies ist ein zweites Paar Augen auf der Hälfte des Ordners, die es sehen kann. +## Eine Definition für jedes Repo + +Eine Persona, die Sie in jedem Repo haben wollen — denselben `pm` überall —, müssen Sie nicht in jedes +kopieren. Neben dem eigenen `.nxs-personas/` eines Workspace gibt es einen **Ordner auf +Benutzerebene**, `~/.nexusflow/personas/`, genauso aufgebaut: eine `.yaml` je Persona und eine +`channels.yaml`. Jeder Workspace auf der Maschine liest ihn. + +- **Zusammengeführt je Name.** Eine Persona oder ein Kanal, den das Repo selbst deklariert, + **verdeckt** den gleichnamigen Eintrag auf Benutzerebene; alles andere kommt hinzu. Ein Kanal kommt + mit den Personas, die er voraussetzt, und seine Mitglieder lösen gegen das zusammengeführte Team + auf — ein Kanal `planning` auf Benutzerebene, der `coder` nennt, erreicht den eigenen `coder` jedes + Repos. +- **Die Identität bleibt je Repo.** Aus einer Definition `pm` wird in jedem Repo, in dem sie läuft, + ein eigener Teilnehmer, mit Board, Gedächtnis und Arbeitskopie dieses Repos. +- **Woher etwas kommt, wird gezeigt.** `nxc list` markiert jeden Eintrag aus dem Ordner auf + Benutzerebene und sagt, welche Einträge das Repo verdeckt; `nxc list --json` trägt an einem solchen + Eintrag `"origin": "user"` und unter `declarations` die Felder `user_path`, `from_user` und + `shadowed`. `nxs prime` sagt dasselbe unter „Declarations". Eine Verdeckung ist nie still: ein Repo, + das noch eine alte Kopie hält, führt die alte Kopie aus, und das sollten Sie wissen. +- **Wie der Ordner gefüllt wird, entscheiden Sie** — ein Git-Klon, eine symbolische Verknüpfung. Der + Ordner ist der Vertrag, nicht seine Herkunft. + +**Der Preis, ausgesprochen:** Eine Deklaration außerhalb des Repos ist nicht mit ihm versioniert, und +ein Klon auf einer Maschine ohne den Ordner hat keinen `pm`. Alles, was oben über Versionsverwaltung +steht, gilt für diesen Ordner genauso — halten Sie ihn in einem eigenen Repo. + +Drei Dinge gelten genau wie für den eigenen Ordner des Repos: + +- **Er wird je Vorgang eingefroren.** Ein Vorgang läuft unter dem zusammengeführten Katalog, wie er + beim Öffnen des Vorgangs stand — Hürden (`preconditions:`) aus dem Ordner auf Benutzerebene + eingeschlossen. Eine Änderung dort erreicht den nächsten Vorgang, nie einen laufenden. +- **Ein relativer Pfad meint das Repo, in dem die Persona läuft.** Nichts löst einen Pfad auf, der in + einer Deklaration steht: Ein System-Prompt, der sagt „lies `knowledge/x.md`, relativ zur Wurzel + dieses Arbeitsbereichs", wird von einer Sitzung gelesen, deren Arbeitsverzeichnis das Repo ist, und + eine Hürde läuft ebenfalls dort. Dateien, die *neben* einer Deklaration auf Benutzerebene liegen, + werden so also nicht gefunden. Eine Persona aus dem Ordner auf Benutzerebene erfährt beim + Sitzungsstart, wo ihre Deklaration liegt („Declared in"), damit ihre Anweisungen dorthin zeigen + können. +- **Eine einbettende App liest denselben Ordner.** Sie wählt keinen anderen und kann es nicht: Die + Personas, die sie startet, laufen mit `nxc`, das diesen Ordner liest, und zwei Antworten auf „wer + existiert" würden eine App von ihren eigenen Sitzungen trennen. + +Ein Entwicklungs-Build liest den Ordner auf Benutzerebene seiner eigenen Dienst-Instanz — +`~/.nexusflow-/personas/`, wenn `NXS_SERVICE_INSTANCE` sie benennt —, sodass ein Build im Test nie +die Personas aufgreift, mit denen Ihre installierte Suite läuft. + ## Wenn eine Deklaration falsch ist Eine fehlerhafte Datei ist ein lauter `validation`-Fehler, der den Pfad nennt — nie eine still diff --git a/crates/chat/docs/guide/en/personas.md b/crates/chat/docs/guide/en/personas.md index a86832f..2ff921d 100644 --- a/crates/chat/docs/guide/en/personas.md +++ b/crates/chat/docs/guide/en/personas.md @@ -265,7 +265,10 @@ thread in its own log, and only that thread crosses: the receiver starts its per copy, with ITS board and memory, and whatever that persona commissions to answer stays in its own workspace. A question (`--escalate`) goes back to the commissioner — never past it to the other workspace's owner — and the commissioner's answer on the same thread resumes the persona. The depth -cap holds for the whole chain across both workspaces. `nxc status` shows the thread in both +cap holds for the whole chain across both workspaces — on the caller's word: the depth +travels in the commission's signed stamp, and the receiver cannot check it against a chain it never +sees. Trusting a workspace means trusting its coordinator, and a hostile one could stamp depth 0; +trust only workspaces whose coordinator you would run yourself. `nxc status` shows the thread in both workspaces with the other side and one of four states: `submitted`, `working`, `input-required`, `completed` (or `rejected` / `canceled`). Two hours without a sign of life — no message on the thread, nothing in the receiving persona's transcript — cancel it and wake the commissioner with @@ -466,6 +469,50 @@ that was rolled back the same way — different members, a different `working_tr is not "the tooling has your back": it is version control that has your back, and this is a second pair of eyes on the half of the folder it can see. +## One definition for every repository + +A persona you want in every repository — the same `pm` in each — does not have to be copied into +each one. Beside a workspace's own `.nxs-personas/` there is a **user-level folder**, +`~/.nexusflow/personas/`, laid out the same way: one `.yaml` per persona, and a +`channels.yaml`. Every workspace on the machine reads it. + +- **Merged per name.** A persona or channel the repository declares itself **hides** the user-level + one of the same name; everything else is added. A channel comes along with the personas it + presupposes, and its members resolve against the merged team — a user-level `planning` channel + naming `coder` reaches each repository's own `coder`. +- **Identity stays per repository.** One definition `pm` is a participant of its own in every + repository it runs in, with that repository's board, memory and working copy. +- **Where it came from is shown.** `nxc list` marks each entry from the user-level folder and says + which entries the repository hides; `nxc list --json` carries `"origin": "user"` on such an entry + and `user_path`, `from_user` and `shadowed` under `declarations`. `nxs prime` says the same under + "Declarations". A hiding is never silent: a repository that still keeps an old copy runs the old + copy, and you should know. +- **How the folder is filled is yours to decide** — a git clone, a symbolic link. The folder is the + contract, not its history. + +**The price, said out loud:** a declaration outside the repository is not versioned with it, and a +clone on a machine without the folder has no `pm`. Everything said above about version control holds +for this folder too — keep it in a repository of its own. + +Three things hold exactly as they do for the repository's own folder: + +- **It is frozen per operation.** What an operation runs under is the merged catalogue as it stood + when the operation opened — hurdles (`preconditions:`) from the user-level folder included. An + edit there reaches the next operation, never a running one. +- **A relative path means the repository the persona runs in.** Nothing resolves a path written in a + declaration: a system prompt that says "read `knowledge/x.md`, relative to the root of this + workspace" is read by a session whose working directory is the repository, and a hurdle runs + there too. Files kept *beside* a user-level declaration are therefore not found that way. A + persona from the user-level folder is told at session start where its declaration lives + ("Declared in"), so its instructions can point there instead. +- **An embedding app reads the same folder.** It does not choose another one, and cannot: the + personas it starts run `nxc`, which reads this folder, and two answers to "who exists" would split + an app from its own sessions. + +A development build reads the user-level folder of its own service instance — +`~/.nexusflow-/personas/` when `NXS_SERVICE_INSTANCE` names it — so a build under test never +picks up the personas your installed suite runs with. + ## When a declaration is wrong A malformed file is a loud `validation` error naming the path — never a silently skipped persona. A diff --git a/crates/chat/src/cli.rs b/crates/chat/src/cli.rs index f73275f..1e58de0 100644 --- a/crates/chat/src/cli.rs +++ b/crates/chat/src/cli.rs @@ -1061,7 +1061,7 @@ fn origin_in(ws: &Workspace) -> String { } /// [`origin_in`] for a call site that has not resolved its workspace yet — it resolves one from -/// `--db`/cwd exactly as [`open`], [`workspace_root`] and [`declaration_dir`] next door do. +/// `--db`/cwd exactly as [`open`], [`workspace_root`] and [`declared_channels`] next door do. /// /// **Why this takes `db` at all** (nxf 6j6v.07me): the origin stopped being a value the process /// environment alone can answer, so the function that answers it needs a workspace, and every @@ -1198,11 +1198,15 @@ fn workspace_root(db: Option<&str>) -> Result { .to_path_buf()) } -/// The declaration directory this invocation READS — `.nxs-personas/`, or a legacy `roles/` folder -/// when that is the one carrying declarations (nxf 6j6v.dvyq step 4). One resolution, owned by -/// [`crate::definitions::DeclarationSource::locate`], so the CLI and `Engine` cannot disagree. -fn declaration_dir(db: Option<&str>) -> Result { - Workspace::resolve(db, &cwd()?)?.declaration_dir() +/// The declared channels this invocation resolves through — the MERGED catalogue's (nxf 6j6v.7k58), +/// so a channel that arrives from the user-level folder gates `threads show` and `search` exactly +/// like one the workspace declares. +fn declared_channels(db: Option<&str>) -> Result> { + Ok( + crate::definitions::Definitions::resolve(&workspace_root(db)?)? + .channels() + .to_vec(), + ) } /// Everything [`crate::orchestration::Ctx`] borrows, owned — the CLI adapter's job in one place. @@ -1881,13 +1885,13 @@ fn prime( // host that started the session itself, else the session this invocation runs under, else a // human. See `crate::persona` for why it is deliberately not the process id, and for what the // answer may and may not be used for. - let declarations = crate::definitions::DeclarationSource::resolve(&workspace_root(db)?)?; - let identity = crate::persona::resolve_identity( - &store, - &crate::role::load_all_roles(&declarations.read_dir)?, - persona, - session().as_deref(), - )?; + let defs = crate::definitions::Definitions::resolve(&workspace_root(db)?)?; + let declarations = defs + .source() + .cloned() + .expect("Definitions::resolve always records its source"); + let identity = + crate::persona::resolve_identity(&store, defs.roles(), persona, session().as_deref())?; let report = facade::prime_for( &store, &consumer, @@ -1915,13 +1919,18 @@ fn prime( /// human sees the complete declared team, which is the directory an app renders whole. fn list(json: bool, db: Option<&str>, persona: Option<&str>) -> Result<()> { let store = open(db)?; - let declarations = crate::definitions::DeclarationSource::resolve(&workspace_root(db)?)?; - let roles = crate::role::load_all_roles(&declarations.read_dir)?; - let channels = crate::channel::load_all_channels(&declarations.read_dir)?; - let identity = crate::persona::resolve_identity(&store, &roles, persona, session().as_deref())?; + // The MERGED catalogue (nxf 6j6v.7k58): the workspace's own declarations and the user-level + // ones it does not hide, with the account of which is which on its source. + let defs = crate::definitions::Definitions::resolve(&workspace_root(db)?)?; + let declarations = defs + .source() + .cloned() + .expect("Definitions::resolve always records its source"); + let (roles, channels) = (defs.roles(), defs.channels()); + let identity = crate::persona::resolve_identity(&store, roles, persona, session().as_deref())?; let directory = match identity.persona() { - Some(handle) => crate::persona::Directory::for_persona(&roles, &channels, handle), - None => crate::persona::Directory::full(&roles, &channels), + Some(handle) => crate::persona::Directory::for_persona(roles, channels, handle), + None => crate::persona::Directory::full(roles, channels), }; // The directory AND where it came from, in one value: an app renders its empty list with the // explanation and the path beside it rather than re-deriving either (nxf 6j6v.dvyq). Attached @@ -1949,8 +1958,10 @@ fn list(json: bool, db: Option<&str>, persona: Option<&str>) -> Result<()> { } else { println!("{}", directory.render_markdown()); // A LEGACY folder is worth saying out loud every time it is the one being read — otherwise - // the move to `.nxs-personas/` is invisible to the person who has to make it. - if declarations.kind == crate::definitions::DeclarationSourceKind::LegacyRoles { + // the move to `.nxs-personas/` is invisible to the person who has to make it. So is the + // user-level folder (nxf 6j6v.7k58): what came from it is not versioned with this + // repository, and a repository file hiding one of its entries runs the repository's copy. + if declarations.worth_saying() { println!("\n{}", declarations.explain()); } } @@ -3911,7 +3922,7 @@ fn threads_show( let channel_id = store .thread_channel(thread_id) .or_else(|| store.thread_channel_via_message(thread_id)); - let channels = crate::channel::load_all_channels(&declaration_dir(db)?)?; + let channels = declared_channels(db)?; let policy = crate::channel::declared_policy(&channels, channel_id.as_deref()); let board = crate::facade::thread_board(&store, thread_id, &resolve_now()?, &consumer, policy)?; if json { @@ -4002,7 +4013,7 @@ fn search(json: bool, db: Option<&str>, query: &str, consumer: Option<&str>) -> // Since nxf 6j6v.cs03 the catalogue decides the caller's MEMBERSHIP there as well — which is why // the qualified handle `resolve_consumer` falls back to now finds a declared channel's boards at // all, where it used to match no materialised member row and answer "no matches". - let channels = crate::channel::load_all_channels(&declaration_dir(db)?)?; + let channels = declared_channels(db)?; let hits = crate::facade::search(&store, &consumer, query, &channels)?; if json { println!("{}", serde_json::to_string(&hits).expect("hits serialize")); diff --git a/crates/chat/src/definitions.rs b/crates/chat/src/definitions.rs index abdcc33..f5cf3c8 100644 --- a/crates/chat/src/definitions.rs +++ b/crates/chat/src/definitions.rs @@ -23,6 +23,25 @@ //! //! Since 6j6v.dvyq the folder is `/.nxs-personas/`, and WHICH folder a catalogue //! came from is itself part of the answer — see [`DeclarationSource`]. +//! +//! **Since 6j6v.7k58 there is a second folder, and the two are MERGED PER NAME.** Beside the +//! workspace's own folder every workspace reads the USER-LEVEL folder +//! ([`user_declarations_dir`], `~/.nexusflow/personas/` for the installed suite): a persona or a +//! channel the workspace declares itself hides the user-level one of the same name, everything else +//! is added. One definition `pm` therefore runs in every repository as that repository's own +//! participant — its board, its memory, its working copy — while it is written down once. The price +//! is said where it is paid: a declaration outside the repository is not versioned with it, and a +//! clone on a machine without the folder has no `pm`. That is why the catalogue says, per entry, +//! where it came from ([`DeclarationSource::from_user`]) and which user-level entries the workspace +//! hides ([`DeclarationSource::shadowed`]). +//! +//! The merge happens HERE, inside [`Definitions::resolve`], and nowhere else — so every seam that +//! resolves a catalogue (the CLI per invocation, every `Engine` verb, `nxs prime`, a spawned +//! persona's own `nxc`) sees the same team, and everything built on the catalogue inherits the +//! user-level entries without knowing they exist: the per-operation freeze +//! ([`crate::declaration_freeze`]) projects the MERGED catalogue, so a hurdle (`preconditions:`) that +//! arrives from the user-level folder is frozen at an operation's start exactly like one from the +//! workspace, and an edit to the user-level folder reaches the next operation, not a running one. use std::collections::HashSet; use std::path::{Path, PathBuf}; @@ -67,8 +86,82 @@ pub struct DeclarationSource { /// is declared at all — every loader reads a missing folder as an empty catalogue, so the /// resolution stays total. pub read_dir: PathBuf, - /// How many declarations the catalogue carries: personas + channels + flows. + /// How many declarations the catalogue carries: personas + channels — the workspace's own and + /// the user-level ones it does not hide, together. pub count: usize, + /// The USER-LEVEL folder (nxf 6j6v.7k58, see [`user_declarations_dir`]) — present only when it + /// carries at least one declaration, so a workspace on a machine without one keeps exactly the + /// shape (and the wording) it had before the folder existed. + #[serde(skip_serializing_if = "Option::is_none")] + pub user_path: Option, + /// The entries of the catalogue that came from [`user_path`](DeclarationSource::user_path) + /// rather than from the workspace's own folder — the per-entry origin `nxc list` and + /// [`crate::engine::Engine::directory`] stamp onto the directory. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub from_user: Vec, + /// User-level entries the workspace HIDES with its own declaration of the same name. Reported + /// rather than resolved silently: a repository that keeps an old copy of a persona the owner + /// now maintains centrally runs the old copy, and nobody would know. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub shadowed: Vec, +} + +/// One declaration by kind and name — how [`DeclarationSource`] names an entry's origin and a +/// shadowing (nxf 6j6v.7k58). +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +pub struct DeclaredName { + pub kind: DeclarationKind, + pub name: String, +} + +/// The two kinds of declaration that merge per name. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DeclarationKind { + Persona, + Channel, +} + +impl DeclarationKind { + pub fn as_str(&self) -> &'static str { + match self { + DeclarationKind::Persona => "persona", + DeclarationKind::Channel => "channel", + } + } +} + +/// Where ONE entry of a catalogue came from (nxf 6j6v.7k58) — carried on the directory's entries, +/// and absent from their wire form for the common case, so a workspace without a user-level folder +/// renders exactly as before. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DeclarationOrigin { + /// The workspace's own folder. + #[default] + Workspace, + /// The user-level folder. + User, +} + +impl DeclarationOrigin { + pub fn is_workspace(&self) -> bool { + *self == DeclarationOrigin::Workspace + } +} + +/// The USER-LEVEL declaration folder this process reads beside every workspace's own (nxf +/// 6j6v.7k58): `/personas` — `~/.nexusflow/personas` for the installed suite and for +/// an embedding app, `~/.nexusflow-/personas` for a development build named so by +/// `NXS_SERVICE_INSTANCE` ([`nxs_service::ServiceHome::personas`]). +/// +/// **One rule, the service's, and no knob.** An embedding app does not choose this location, and +/// must not be able to: the personas it starts run the `nxc` CLI, which resolves the folder by this +/// same rule, and an app reading a folder of its own would disagree with the very sessions it +/// spawned about who exists. Following the service home is also what keeps a build under test off +/// the personas the machine's installed suite runs with. +pub fn user_declarations_dir() -> Result { + Ok(nxs_service::ServiceHome::resolve()?.personas()) } /// The three answers to "where did this catalogue come from". @@ -147,6 +240,9 @@ impl DeclarationSource { legacy_path, read_dir, count: 0, + user_path: None, + from_user: Vec::new(), + shadowed: Vec::new(), } } @@ -166,12 +262,29 @@ impl DeclarationSource { /// own is a lie by omission; a failure would be wrong too, since having declared nothing yet is /// a legitimate state. pub fn explain(&self) -> String { + let repo = self.explain_workspace_half(); + match &self.user_path { + None => repo, + Some(user) => format!("{repo} {}", self.explain_user_half(user)), + } + } + + /// The workspace's own folder, said as it was before the user-level folder existed — so a + /// machine without one reads exactly as it did (acceptance point 8 of 6j6v.70dy). + fn explain_workspace_half(&self) -> String { let where_it_belongs = format!( "declare one as `{}/.yaml` (a persona) or `{}/channels.yaml` (a channel)", self.path.display(), self.path.display() ); + // The workspace's OWN count: `count` is the merged catalogue's. + let count = self.count.saturating_sub(self.from_user.len()); match (self.kind, &self.legacy_path) { + // With user-level entries in the catalogue, "nobody is declared here" would be false: + // somebody is, just not by this repository. + (DeclarationSourceKind::None, _) if self.user_path.is_some() => { + format!("this repository declares nobody of its own — {where_it_belongs}.") + } (DeclarationSourceKind::None, Some(legacy)) => format!( "nobody is declared here yet — {where_it_belongs}. A legacy folder exists at {} \ but declares nothing.", @@ -184,27 +297,72 @@ impl DeclarationSource { "read from the LEGACY folder {} ({} declared). Declarations belong in {} now; move \ the folder when it suits you — nothing here rewrites your project.", self.read_dir.display(), - self.count, + count, self.path.display() ), (DeclarationSourceKind::Personas, Some(legacy)) => format!( "read from {} ({} declared). A legacy folder is still present at {} and is NOT \ read while {} carries declarations.", self.path.display(), - self.count, + count, legacy.display(), PERSONAS_DIR ), (DeclarationSourceKind::Personas, None) => { - format!( - "read from {} ({} declared).", - self.path.display(), - self.count - ) + format!("read from {} ({} declared).", self.path.display(), count) } } } + /// The user-level folder: what came from it, and what this workspace hides of it. + fn explain_user_half(&self, user: &Path) -> String { + let names = |list: &[DeclaredName]| { + list.iter() + .map(|d| format!("{} `{}`", d.kind.as_str(), d.name)) + .collect::>() + .join(", ") + }; + let mut out = match self.from_user.is_empty() { + true => format!( + "The user-level folder {} adds nothing here.", + user.display() + ), + false => format!( + "From the user-level folder {}: {} — not versioned with this repository.", + user.display(), + names(&self.from_user) + ), + }; + if !self.shadowed.is_empty() { + out.push_str(&format!( + " This repository's own declaration HIDES the user-level {}.", + names(&self.shadowed) + )); + } + out + } + + /// Whether a surface that shows a non-empty directory should print [`explain`] beside it: a + /// LEGACY folder is being read, or the user-level folder contributes or is hidden. The plain + /// case — the workspace's own `.nxs-personas/` and nothing else — has nothing to say. + /// + /// [`explain`]: DeclarationSource::explain + pub fn worth_saying(&self) -> bool { + self.kind == DeclarationSourceKind::LegacyRoles || self.user_path.is_some() + } + + /// Where the entry `name` of `kind` in this catalogue came from. + pub fn origin_of(&self, kind: DeclarationKind, name: &str) -> DeclarationOrigin { + match self + .from_user + .iter() + .any(|d| d.kind == kind && d.name == name) + { + true => DeclarationOrigin::User, + false => DeclarationOrigin::Workspace, + } + } + /// The `--json` projection — the same fact the human sentence carries, as data. /// /// Delegates to the derived `Serialize` rather than assembling a second object by hand: two @@ -394,8 +552,85 @@ impl Definitions { /// 2026-08-14; pinned on the app side by `engine-bridge/src/chat.rs` and /// `engine-server/tests/chat_wire.rs`). pub fn resolve(root: &Path) -> Result { + Definitions::resolve_with_user_dir(root, Some(&user_declarations_dir()?)) + } + + /// [`resolve`](Definitions::resolve) against a NAMED user-level folder, or none — the form for + /// a test that must not read the machine's real one, and for a caller that has resolved the + /// folder already. Production reads the one [`user_declarations_dir`] names; see there for why + /// no seam lets an app pick another. + pub fn resolve_with_user_dir(root: &Path, user_dir: Option<&Path>) -> Result { let mut source = DeclarationSource::locate(root); - let mut defs = Definitions::from_dir(&source.read_dir)?; + source.user_path = user_dir + .filter(|dir| carries_declarations(dir)) + .map(Path::to_path_buf); + Definitions::load(source) + } + + /// Re-read the catalogue a [`DeclarationSource`] describes — both folders, merged by the same + /// rule. For a caller that was handed the resolution rather than the root (`prime`). + pub fn from_source(source: &DeclarationSource) -> Result { + let mut source = source.clone(); + source.from_user.clear(); + source.shadowed.clear(); + Definitions::load(source) + } + + /// Load the workspace's folder and, when there is one, the user-level folder, and MERGE them + /// per name (nxf 6j6v.7k58): a workspace entry hides the user-level entry of the same name and + /// kind, everything else is added. Recorded on `source` as it happens, so the catalogue and the + /// account of where it came from cannot disagree. + /// + /// **A duplicate is judged per folder, then the merged whole is validated once.** Two files in + /// one folder declaring the same name are still the ambiguity [`Definitions::new`] refuses, and + /// the error names the folder. Across the two folders the same name is not a duplicate — it is + /// the shadowing the merge exists for. Everything else (handle form, reserved prefix, a flow + /// reaching itself) is checked on the MERGED catalogue, because that is the team that runs: a + /// user-level flow can only form a cycle through the workspace's channels, or be relieved of one + /// by a workspace channel that hides a step. + /// + /// A malformed user-level file fails every workspace's catalogue, with that file's path in the + /// error — the same fail-closed rule a malformed workspace file has, and for the same reason: a + /// team half-read is worse than a team refused with the name of the file to fix. + fn load(mut source: DeclarationSource) -> Result { + let mut roles = crate::role::load_all_roles(&source.read_dir)?; + let mut channels = crate::channel::load_all_channels(&source.read_dir)?; + reject_duplicates_in(&source.read_dir, &roles, &channels)?; + if let Some(user_dir) = source.user_path.clone() { + let user_roles = crate::role::load_all_roles(&user_dir)?; + let user_channels = crate::channel::load_all_channels(&user_dir)?; + reject_duplicates_in(&user_dir, &user_roles, &user_channels)?; + for role in user_roles { + let name = DeclaredName { + kind: DeclarationKind::Persona, + name: role.handle.clone(), + }; + match roles.iter().any(|r| r.handle == role.handle) { + true => source.shadowed.push(name), + false => { + source.from_user.push(name); + roles.push(role); + } + } + } + for channel in user_channels { + let name = DeclaredName { + kind: DeclarationKind::Channel, + name: channel.name.clone(), + }; + match channels.iter().any(|c| c.name == channel.name) { + true => source.shadowed.push(name), + false => { + source.from_user.push(name); + channels.push(channel); + } + } + } + // The loader's own contract: roles handle-sorted. Channels keep declaration order, + // the workspace's first. + roles.sort_by(|a, b| a.handle.cmp(&b.handle)); + } + let mut defs = Definitions::new(roles, channels)?; source.count = defs.len(); defs.source = Some(source); Ok(defs) @@ -564,6 +799,14 @@ fn validate_role_handle(handle: &str) -> Result<()> { Ok(()) } +/// [`reject_duplicates`] for one folder's personas and channels, naming the folder — so a duplicate +/// in the user-level folder is not mistaken for one in the workspace (nxf 6j6v.7k58). +fn reject_duplicates_in(dir: &Path, roles: &[RoleDecl], channels: &[ChannelDecl]) -> Result<()> { + reject_duplicates("role handle", roles.iter().map(|r| r.handle.as_str())) + .and_then(|()| reject_duplicates("channel name", channels.iter().map(|c| c.name.as_str()))) + .map_err(|e| NxfError::validation(format!("{}: {}", dir.display(), e.msg))) +} + /// Reject a repeated name, saying which kind and which value — the ambiguity a caller-supplied /// `Vec` can express and a one-file-per-name directory could not. fn reject_duplicates<'a>(what: &str, names: impl Iterator) -> Result<()> { diff --git a/crates/chat/src/engine.rs b/crates/chat/src/engine.rs index d3e3bf5..7e55a81 100644 --- a/crates/chat/src/engine.rs +++ b/crates/chat/src/engine.rs @@ -85,8 +85,9 @@ use std::time::Duration; /// untouched and orchestration is a deliberate opt-in through [`Engine::open_with`]. /// /// **Declarations are NOT on this config, and that is the point** (nxf 6j6v.dvyq step 6). They come -/// from the workspace's own `.nxs-personas/` folder, always, and are re-read on every verb that -/// needs them — exactly as a fresh `nxc` process re-reads the folder per invocation. There is no +/// from the workspace's own `.nxs-personas/` folder, merged per name with the user-level folder +/// (nxf 6j6v.7k58, [`crate::definitions::user_declarations_dir`] — not configurable here either, +/// for the reason given there), and are re-read on every verb that needs them — exactly as a fresh `nxc` process re-reads the folder per invocation. There is no /// injection path any more: `DefinitionSource::Supplied` and `Engine::set_definitions` are gone, /// because personas and channels live in the folder even for an app, and a later in-app role editor /// writes into that same folder rather than into an app-owned database (owner decision, 2026-08-14, @@ -300,15 +301,14 @@ impl Worker for DisabledWorker { /// in. fn declared_policy_of( store: &ChatStore, - decl_dir: &Path, + channels: &[crate::channel::ChannelDecl], thread_id: &str, ) -> Result { let channel_id = store .thread_channel(thread_id) .or_else(|| store.thread_channel_via_message(thread_id)); - let channels = crate::channel::load_all_channels(decl_dir)?; Ok(crate::channel::declared_policy( - &channels, + channels, channel_id.as_deref(), )) } @@ -425,9 +425,10 @@ impl Engine { /// the filter is the one that went. `AllMembers` remains the honest fallback for a thread whose /// channel no declaration names. pub fn thread(&self, thread_id: &str, as_handle: &str) -> Result { - let decl_dir = self.workspace().declaration_dir()?; + // The MERGED catalogue's channels (nxf 6j6v.7k58), read before the store lock is taken. + let defs = self.definitions()?; self.handle.try_with_state(|s| { - let policy = declared_policy_of(&s.store, &decl_dir, thread_id)?; + let policy = declared_policy_of(&s.store, defs.channels(), thread_id)?; facade::thread(&s.store, thread_id, as_handle, policy) }) } @@ -475,9 +476,9 @@ impl Engine { /// rule and what it costs; `AllMembers` remains the honest fallback for a channel no /// declaration names, so an undeclared workspace reads exactly as it always did. pub fn search(&self, handle: &str, query: &str) -> Result> { - let channels = crate::channel::load_all_channels(&self.workspace().declaration_dir()?)?; + let defs = self.definitions()?; self.handle - .try_with_state(|s| facade::search(&s.store, handle, query, &channels)) + .try_with_state(|s| facade::search(&s.store, handle, query, defs.channels())) } /// **Where does this operation stand** (nxf 6j6v.a71h) — the thread TREE an operation is, across @@ -786,7 +787,8 @@ impl Engine { // ---- declarations ------------------------------------------------------ /// The declarations this handle resolves through, read fresh from the workspace's - /// `.nxs-personas/` folder (see [`crate::definitions::DeclarationSource`] for the resolution + /// `.nxs-personas/` folder merged with the user-level one (nxf 6j6v.7k58), each entry's origin + /// on the source (see [`crate::definitions::DeclarationSource`] for the resolution /// and for the legacy `roles/` compatibility). Carries its own source resolution, so an app can /// render "nothing is declared, and here is where it goes" without re-deriving the path. /// diff --git a/crates/chat/src/facade.rs b/crates/chat/src/facade.rs index 21c19da..b0e1735 100644 --- a/crates/chat/src/facade.rs +++ b/crates/chat/src/facade.rs @@ -2285,7 +2285,18 @@ pub struct PrimeRoster { pub fn validate_declared_team(dir: &Path) -> Result { let roles = crate::role::load_all_roles(dir)?; let channels = crate::channel::load_all_channels(dir)?; - let channel_errors = crate::channel::validate_channels(&roles, &channels); + Ok(validate_declared(&roles, &channels)) +} + +/// [`validate_declared_team`] over a catalogue already in hand — what `prime` partitions since the +/// catalogue is two folders merged (nxf 6j6v.7k58). "Whole-file" below then means the merged +/// channels as one: a channel error from either `channels.yaml` excludes every channel, which is +/// coarser than per folder and is the reading under which the partition still says one thing. +pub(crate) fn validate_declared( + roles: &[crate::role::RoleDecl], + channels: &[ChannelDecl], +) -> PrimeRoster { + let channel_errors = crate::channel::validate_channels(roles, channels); // Whole-file scoping: any CHANNEL error excludes every channel this file declared — and only a // channel error does. The persona pass below is appended after this is decided, deliberately: // a persona's dangling reference says nothing about whether `channels.yaml` can be trusted, and @@ -2300,19 +2311,19 @@ pub fn validate_declared_team(dir: &Path) -> Result { // stay in the roster whatever it finds — see [`PrimeRoster`], which has always said a role file // fails at LOAD time or not at all. let mut errors = channel_errors; - errors.extend(crate::role::validate_roles(&roles, &channels)); + errors.extend(crate::role::validate_roles(roles, channels)); // Over the SAME two slices, and deliberately not bounded by the partition above: a quality // warning is about how a declaration is written, so a referential error elsewhere in // `channels.yaml` neither suppresses it nor is suppressed by it. - let warnings = crate::declaration_quality::warn_declarations(&roles, &channels); + let warnings = crate::declaration_quality::warn_declarations(roles, channels); - Ok(PrimeRoster { - roles: roles.into_iter().map(|r| r.handle).collect(), + PrimeRoster { + roles: roles.iter().map(|r| r.handle.clone()).collect(), channels: clean_channels, errors, warnings, - }) + } } // ---- prime: the session bootstrap (nxf 6j6v.r5a2) -------------------------- @@ -2955,7 +2966,11 @@ pub fn render_declared_team(roster: &PrimeRoster) -> String { /// pointing at a section that no longer exists. pub fn render_declaration_source(source: &DeclarationSource) -> String { match (source.kind, &source.legacy_path) { - (crate::definitions::DeclarationSourceKind::Personas, None) => String::new(), + // Silent no longer when the user-level folder takes part (nxf 6j6v.7k58): what came from + // it is not versioned with this repository, and a hiding is news to whoever reads this. + (crate::definitions::DeclarationSourceKind::Personas, None) if !source.worth_saying() => { + String::new() + } _ => format!("## Declarations\n\n{}", source.explain()), } } @@ -3090,15 +3105,26 @@ pub fn prime_for( // The declarations are read a second time here (`validate_declared_team` loads its own copy to // partition them): a handful of small YAML files, once per prime, in exchange for leaving that // function's roster contract — and every caller of it — untouched. - let decl_dir = declarations.read_dir.as_path(); - let roles = crate::role::load_all_roles(decl_dir)?; - let channels = crate::channel::load_all_channels(decl_dir)?; + // + // Both folders, merged by the one rule (nxf 6j6v.7k58): `from_source` re-reads the workspace's + // folder and the user-level one the resolution names. + let defs = crate::definitions::Definitions::from_source(declarations)?; + let (roles, channels) = (defs.roles(), defs.channels()); let brief = persona .and_then(|handle| roles.iter().find(|r| r.handle == handle)) - .map(|decl| PersonaBrief::from_decl(decl, &channels)); + .map(|decl| { + PersonaBrief::from_decl(decl, channels).declared_in( + match declarations + .origin_of(crate::definitions::DeclarationKind::Persona, &decl.handle) + { + crate::definitions::DeclarationOrigin::User => declarations.user_path.clone(), + crate::definitions::DeclarationOrigin::Workspace => None, + }, + ) + }); let directory = match persona { - Some(handle) => Directory::for_persona(&roles, &channels, handle), - None => Directory::full(&roles, &channels), + Some(handle) => Directory::for_persona(roles, channels, handle), + None => Directory::full(roles, channels), }; // **The caller's own previous session end** (nxf 6j6v.2hx9) — the watermark the notice below is // a window over. @@ -3149,7 +3175,7 @@ pub fn prime_for( // rather than a gap: a human at a terminal has no session whose end could be a watermark, // and `nxc status` answers the same question on demand. wake: opener_wake(store, consumer, now, watermark.as_deref())?, - roster: validate_declared_team(decl_dir)?, + roster: validate_declared(roles, channels), persona: brief, directory, declarations: declarations.clone(), diff --git a/crates/chat/src/persona.rs b/crates/chat/src/persona.rs index cf909a4..ac91e2c 100644 --- a/crates/chat/src/persona.rs +++ b/crates/chat/src/persona.rs @@ -41,6 +41,7 @@ use serde::Serialize; use crate::channel::ChannelDecl; +use crate::definitions::{DeclarationKind, DeclarationOrigin}; use crate::error::Result; use crate::role::{Addressable, RoleDecl, Stage}; use crate::store::ChatStore; @@ -131,6 +132,11 @@ pub struct PersonaEntry { /// line comes from the caller's own declaration rather than from the target's. #[serde(skip_serializing_if = "Option::is_none")] pub why: Option, + /// Where this persona's declaration came from (nxf 6j6v.7k58) — stamped by + /// [`Directory::with_declarations`], and absent from the wire form for the workspace's own + /// folder, so a workspace without a user-level folder reads exactly as it did. + #[serde(skip_serializing_if = "DeclarationOrigin::is_workspace")] + pub origin: DeclarationOrigin, } impl PersonaEntry { @@ -154,6 +160,7 @@ impl PersonaEntry { false => route_in(decl, channels), }, why, + origin: DeclarationOrigin::Workspace, } } } @@ -184,6 +191,9 @@ pub struct ChannelEntry { /// Why THIS caller would address it — address-book projection only, as on [`PersonaEntry`]. #[serde(skip_serializing_if = "Option::is_none")] pub why: Option, + /// Where this channel's declaration came from — as on [`PersonaEntry::origin`]. + #[serde(skip_serializing_if = "DeclarationOrigin::is_workspace")] + pub origin: DeclarationOrigin, } impl ChannelEntry { @@ -197,6 +207,7 @@ impl ChannelEntry { members: crate::channel::cast(decl), description: decl.description.clone(), why, + origin: DeclarationOrigin::Workspace, } } } @@ -308,11 +319,20 @@ impl Directory { } /// Attach the resolution the catalogue behind this directory came from — see the - /// [`declarations`](Directory::declarations) field. + /// [`declarations`](Directory::declarations) field — and stamp each entry with where it came + /// from (nxf 6j6v.7k58), which only the resolution knows. pub fn with_declarations( mut self, source: Option<&crate::definitions::DeclarationSource>, ) -> Directory { + if let Some(source) = source { + for p in &mut self.personas { + p.origin = source.origin_of(DeclarationKind::Persona, &p.handle); + } + for c in &mut self.channels { + c.origin = source.origin_of(DeclarationKind::Channel, &c.name); + } + } self.declarations = source.cloned(); self } @@ -493,6 +513,11 @@ impl Directory { } } +/// What an entry from the user-level folder carries in its parenthesis (nxf 6j6v.7k58). Only `nxc +/// list` stamps origins ([`Directory::with_declarations`]), so a persona's own address book in +/// `prime` never shows it — where a peer's definition is kept is not what a persona addresses it by. +const USER_LEVEL_QUALIFIER: &str = "from the user-level folder"; + /// One persona's entry: who they are, how to name them, and what they are for. /// /// The bold name is the human one (`job_title`), because that is what a reader recognises; the @@ -505,6 +530,9 @@ fn render_persona_entry(p: &PersonaEntry) -> String { if let Some(stage) = p.stage { qualifiers.push(format!("level: {}", stage_label(stage))); } + if !p.origin.is_workspace() { + qualifiers.push(USER_LEVEL_QUALIFIER.to_string()); + } let mut line = format!( "**{}** ({})", display_name(p.job_title.as_deref(), &p.handle), @@ -534,6 +562,9 @@ fn render_channel_entry(c: &ChannelEntry) -> String { if !c.members.is_empty() { qualifiers.push(format!("members: {}", c.members.join(", "))); } + if !c.origin.is_workspace() { + qualifiers.push(USER_LEVEL_QUALIFIER.to_string()); + } let mut line = format!( "**{}** ({})", display_name(None, &c.name), @@ -610,6 +641,17 @@ pub struct PersonaBrief { pub channels: Vec, /// How this persona uses `nxc` — one line per rule, in display order. pub instructions: &'static [&'static str], + /// The user-level folder this persona's declaration came from, when it did (nxf 6j6v.7k58) — + /// absent for a declaration in the workspace's own folder. + /// + /// **It answers a question the session cannot answer for itself:** where the files a + /// declaration keeps BESIDE itself are. A system prompt that says "read `knowledge/x.md`, + /// relative to the root of this workspace" means the repository the session runs in — its + /// working directory, and the working directory a hurdle runs in — never the folder the + /// declaration was read from; nothing in the engine resolves a path inside a declaration. A + /// user-level persona whose knowledge lives beside its definition is told where that is. + #[serde(skip_serializing_if = "Option::is_none")] + pub declared_in: Option, } /// The `nxc` rules every persona is handed. Rendered in order; the answering rule comes first @@ -688,9 +730,16 @@ impl PersonaBrief { addressable: decl.addressable.clone(), channels: route_in(decl, channels), instructions: PERSONA_INSTRUCTIONS, + declared_in: None, } } + /// Say where the declaration came from — see [`declared_in`](PersonaBrief::declared_in). + pub fn declared_in(mut self, folder: Option) -> PersonaBrief { + self.declared_in = folder; + self + } + /// The "you are …" block as Markdown — identity first, then the `nxc` rules. pub fn render_markdown(&self) -> String { let mut lines = vec![format!("## You are `{}`", self.handle)]; @@ -778,6 +827,15 @@ impl PersonaBrief { if let Some(reachable) = reachable { lines.push(format!("- **Reachable:** {reachable}")); } + if let Some(folder) = &self.declared_in { + lines.push(format!( + "- **Declared in:** the user-level folder `{}`, not in this repository. A path your \ + instructions give relative to this repository means THIS repository; files kept \ + beside your declaration are under `{}`.", + folder.display(), + folder.display() + )); + } lines.push(String::new()); lines.push("### How you use `nxc`".to_string()); for rule in self.instructions { diff --git a/crates/chat/src/workspace.rs b/crates/chat/src/workspace.rs index 485df59..8b35adf 100644 --- a/crates/chat/src/workspace.rs +++ b/crates/chat/src/workspace.rs @@ -85,7 +85,10 @@ pub trait ChatWorkspaceExt { /// declarations lived at before 6j6v.dvyq. Resolved, not necessarily existing. fn legacy_roles_dir(&self) -> Result; - /// The folder declarations are actually READ from, resolved over both locations — see + /// The workspace's OWN folder declarations are read from — only the workspace's half since nxf + /// 6j6v.7k58: the catalogue also merges the user-level folder, so a reader that wants the team + /// resolves [`crate::definitions::Definitions::resolve`] rather than loading this folder alone. + /// Resolved over both workspace locations — see /// [`crate::definitions::DeclarationSource::locate`], which owns the rule. Shared by every /// declared-persona/channel/flow loader, so the CLI and [`crate::engine::Engine`] cannot /// resolve it differently (independent review, Integrity #3/Code Quality #1, High). diff --git a/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs new file mode 100644 index 0000000..ec19f29 --- /dev/null +++ b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs @@ -0,0 +1,146 @@ +//! **The per-operation freeze holds for the user-level folder** (nxf 6j6v.7k58, nxf 6j6v.n92p). +//! +//! A declaration arriving from `~/.nexusflow/personas/` is executable text from a second place — +//! a persona's prompt, a channel's hurdles. The freeze projects the MERGED catalogue, so an edit to +//! the user-level folder made while an operation runs reaches the next operation and never the +//! running one, exactly as an edit to the workspace's own folder does (`tests/declaration_freeze.rs`). +//! +//! `$HOME` is pinned, and that is process-wide: ONE test in a binary of its own. + +use std::sync::{Arc, Mutex}; + +use nexus_chat::engine::{Engine, EngineConfig}; +use nexus_chat::orchestration::Caller; +use nexus_chat::surface::{ReplyThreadRequest, SendToRefs, SendToRequest}; +use nexus_chat::timer::TimerConfig; +use nexus_chat::worker::{TriggerOutcome, TriggerRequest, TriggerResult, Worker, WorkerConfig}; +use nexus_chat::workspace::{chat_config, setup}; +use tempfile::TempDir; + +const NOW: &str = "2026-10-04T10:00:00Z"; + +#[derive(Default)] +struct RecordingWorker(Mutex>); + +impl Worker for RecordingWorker { + fn trigger(&self, req: TriggerRequest) -> TriggerResult { + self.0.lock().unwrap().push(req); + Ok(TriggerOutcome::Accepted) + } +} + +impl RecordingWorker { + fn prompts_of(&self, handle: &str) -> Vec { + self.0 + .lock() + .unwrap() + .iter() + .filter(|r| r.role.handle == handle) + .map(|r| r.role.system_prompt.clone()) + .collect() + } +} + +fn declare_version(user: &std::path::Path, version: &str) { + for handle in ["coder", "finisher"] { + std::fs::write( + user.join(format!("{handle}.yaml")), + format!("handle: {handle}\nsystem_prompt: {version}\n"), + ) + .unwrap(); + } +} + +fn open(engine: &Engine) -> String { + engine + .send_to( + Caller { + session: None, + actor: Some("pm"), + now: Some(NOW), + }, + SendToRequest { + machine: None, + to: "coding", + body: "build T5", + refs: SendToRefs::ExplicitlyNone, + }, + ) + .expect("the user-level channel opens") + .thread_id +} + +#[test] +fn an_edit_to_the_user_level_folder_reaches_the_next_operation_and_not_the_running_one() { + let home = TempDir::new().unwrap(); + // SAFETY of the mutation: this binary has exactly one test, and nothing else runs in it. + std::env::set_var("HOME", home.path()); + std::env::remove_var("XDG_CONFIG_HOME"); + std::env::remove_var("XDG_DATA_HOME"); + let user = nexus_chat::definitions::user_declarations_dir().unwrap(); + std::fs::create_dir_all(&user).unwrap(); + declare_version(&user, "VERSION-ONE"); + std::fs::write( + user.join("channels.yaml"), + "- name: coding\n members: [coder, finisher]\n flow: sequential\n", + ) + .unwrap(); + + // A repository that declares nothing of its own. + let repo = TempDir::new().unwrap(); + setup(repo.path(), &chat_config()).unwrap(); + let worker = Arc::new(RecordingWorker::default()); + let engine = Engine::open_with( + None, + repo.path(), + EngineConfig { + worker: WorkerConfig::Custom(worker.clone()), + timer: TimerConfig::Dry, + ..EngineConfig::default() + }, + ) + .unwrap(); + + open(&engine); + let coder = worker + .0 + .lock() + .unwrap() + .iter() + .find(|r| r.role.handle == "coder") + .cloned() + .expect("the first step started"); + assert!(coder.role.system_prompt.contains("VERSION-ONE")); + + // The owner edits the user-level folder mid-operation. + declare_version(&user, "VERSION-TWO"); + engine + .reply_thread( + Caller { + session: Some(&coder.internal_session), + actor: None, + now: Some(NOW), + }, + ReplyThreadRequest { + machine: None, + thread: coder.reply_thread.as_deref().unwrap(), + body: "done", + escalate: false, + needs_rework: false, + accept: false, + }, + ) + .unwrap(); + let finisher = worker.prompts_of("finisher"); + assert_eq!(finisher.len(), 1, "the flow advanced"); + assert!( + finisher[0].contains("VERSION-ONE"), + "the running operation keeps the version it opened with: {finisher:?}" + ); + + // The next operation takes the edit. + open(&engine); + let coders = worker.prompts_of("coder"); + assert_eq!(coders.len(), 2); + assert!(coders[1].contains("VERSION-TWO"), "{coders:?}"); +} diff --git a/crates/chat/tests/one_definition_at_the_engine_seam.rs b/crates/chat/tests/one_definition_at_the_engine_seam.rs new file mode 100644 index 0000000..4ee8c52 --- /dev/null +++ b/crates/chat/tests/one_definition_at_the_engine_seam.rs @@ -0,0 +1,95 @@ +//! **The user-level declaration folder at the seam an embedding app reads** (nxf 6j6v.7k58). +//! +//! An app resolves its catalogue through `Engine`, and the folder it reads must be the one the +//! CLI reads — the personas an app starts run `nxc`, and two answers to "who exists" would split +//! the app from its own sessions. So the engine takes no location: it follows the service home, from +//! `$HOME`. Pinning `$HOME` is process-wide, which is why this is ONE test in a binary of its own. + +use nexus_chat::definitions::{DeclarationKind, DeclarationOrigin}; +use tempfile::TempDir; + +const NOW: &str = "2026-10-04T10:00:00Z"; + +#[test] +fn the_engine_reads_the_user_level_folder_the_cli_reads_and_says_where_each_entry_came_from() { + let home = TempDir::new().unwrap(); + // SAFETY of the mutation: this binary has exactly one test, and nothing else runs in it. + std::env::set_var("HOME", home.path()); + std::env::remove_var("XDG_CONFIG_HOME"); + std::env::remove_var("XDG_DATA_HOME"); + // Whichever instance the ambient environment names, the folder is that instance's — resolved + // the way production resolves it, not spelled out here. + let user = nexus_chat::definitions::user_declarations_dir().unwrap(); + assert!(user.starts_with(home.path()), "{}", user.display()); + std::fs::create_dir_all(&user).unwrap(); + std::fs::write( + user.join("pm.yaml"), + "handle: pm\njob_title: Product manager\nsystem_prompt: The central PM.\n", + ) + .unwrap(); + std::fs::write( + user.join("channels.yaml"), + "- name: planning\n members: [pm]\n", + ) + .unwrap(); + + let repo = TempDir::new().unwrap(); + nexus_chat::workspace::setup(repo.path(), &nexus_chat::workspace::chat_config()).unwrap(); + let engine = nexus_chat::engine::Engine::open(None, repo.path()).unwrap(); + + // `definitions`: the merged catalogue, with its account. + let defs = engine.definitions().unwrap(); + assert_eq!(defs.role("pm").unwrap().system_prompt, "The central PM."); + let source = defs.source().unwrap(); + assert_eq!(source.user_path.as_deref(), Some(user.as_path())); + assert_eq!( + source.origin_of(DeclarationKind::Channel, "planning"), + DeclarationOrigin::User + ); + + // `directory`: every entry stamped with its origin, the same record `nxc list --json` prints. + let directory = engine.directory(None).unwrap(); + let pm = directory + .personas + .iter() + .find(|p| p.handle == "pm") + .unwrap(); + assert_eq!(pm.origin, DeclarationOrigin::User); + let planning = directory + .channels + .iter() + .find(|c| c.name == "planning") + .unwrap(); + assert_eq!(planning.origin, DeclarationOrigin::User); + let wire = serde_json::to_value(&directory).unwrap(); + assert_eq!(wire["personas"][0]["origin"], "user", "{wire}"); + + // `prime_as` for the persona: it is told where its declaration lives, because a path in its + // instructions relative to "this repository" means the repository it runs in. + let report = engine.prime_as("local/pm", Some("pm"), NOW).unwrap(); + let brief = report.persona.as_ref().expect("a persona brief"); + assert_eq!(brief.declared_in.as_deref(), Some(user.as_path())); + let rendered = brief.render_markdown(); + assert!( + rendered.contains("**Declared in:** the user-level folder"), + "{rendered}" + ); + // The composed session start a spawned persona is actually handed carries it. + let start = engine.persona_prime("pm", NOW).unwrap(); + assert!(start.contains("**Declared in:**"), "{start}"); + // ... and the workspace's own declaration of the same name takes over, reported. + std::fs::create_dir_all(repo.path().join(".nxs-personas")).unwrap(); + std::fs::write( + repo.path().join(".nxs-personas/pm.yaml"), + "handle: pm\nsystem_prompt: The repository's own PM.\n", + ) + .unwrap(); + let defs = engine.definitions().unwrap(); + assert_eq!( + defs.role("pm").unwrap().system_prompt, + "The repository's own PM." + ); + let report = engine.prime_as("local/pm", Some("pm"), NOW).unwrap(); + assert_eq!(report.persona.as_ref().unwrap().declared_in, None); + assert_eq!(report.declarations.shadowed.len(), 1); +} diff --git a/crates/chat/tests/one_definition_many_repositories.rs b/crates/chat/tests/one_definition_many_repositories.rs new file mode 100644 index 0000000..8848368 --- /dev/null +++ b/crates/chat/tests/one_definition_many_repositories.rs @@ -0,0 +1,237 @@ +//! **One definition, many repositories** (nxf 6j6v.7k58) — the user-level declaration folder beside +//! a workspace's own, merged per name. +//! +//! Everything here resolves against a NAMED user-level folder +//! ([`Definitions::resolve_with_user_dir`]), so no test reads the machine's real one. The seam an +//! embedding app reads — `Engine::definitions`, `Engine::directory`, `Engine::prime_as` — resolves +//! the folder from `$HOME` and is proved in its own test binary, +//! `tests/one_definition_at_the_engine_seam.rs`, because pinning `$HOME` is process-wide. + +use nexus_chat::definitions::{ + DeclarationKind, DeclarationOrigin, DeclarationSourceKind, DeclaredName, Definitions, +}; +use std::path::Path; +use tempfile::TempDir; + +const CENTRAL_PM: &str = "handle: pm\njob_title: Product manager\nsystem_prompt: The central PM.\n"; +const REPO_PM: &str = "handle: pm\nsystem_prompt: The repository's own PM.\n"; +const CODER: &str = "handle: coder\nsystem_prompt: You are the coder.\n"; +const PLANNING: &str = "- name: planning\n members: [pm, coder]\n"; + +fn declare(dir: &Path, file: &str, body: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(dir.join(file), body).unwrap(); +} + +struct Machine { + repo: TempDir, + user: TempDir, +} + +impl Machine { + fn new() -> Machine { + Machine { + repo: TempDir::new().unwrap(), + user: TempDir::new().unwrap(), + } + } + fn repo_personas(&self) -> std::path::PathBuf { + self.repo.path().join(".nxs-personas") + } + fn resolve(&self) -> nexus_chat::error::Result { + Definitions::resolve_with_user_dir(self.repo.path(), Some(self.user.path())) + } +} + +fn persona(name: &str) -> DeclaredName { + DeclaredName { + kind: DeclarationKind::Persona, + name: name.to_string(), + } +} + +fn channel(name: &str) -> DeclaredName { + DeclaredName { + kind: DeclarationKind::Channel, + name: name.to_string(), + } +} + +#[test] +fn a_repository_without_a_file_of_its_own_runs_the_user_level_persona_and_its_channel() { + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + declare(m.user.path(), "channels.yaml", PLANNING); + declare(&m.repo_personas(), "coder.yaml", CODER); + + let defs = m.resolve().unwrap(); + assert_eq!(defs.role("pm").unwrap().system_prompt, "The central PM."); + assert_eq!( + defs.role("coder").unwrap().system_prompt, + "You are the coder." + ); + // The channel arrives with the persona it presupposes, and its members resolve against the + // MERGED team — the repository's own coder included. + let planning = defs + .declared_channel("planning") + .unwrap() + .expect("declared"); + assert_eq!(planning.members, vec!["pm", "coder"]); + // Roles stay handle-sorted, the loader's own contract. + let handles: Vec<_> = defs.roles().iter().map(|r| r.handle.as_str()).collect(); + assert_eq!(handles, ["coder", "pm"]); + + let source = defs.source().unwrap(); + assert_eq!(source.kind, DeclarationSourceKind::Personas); + assert_eq!(source.count, 3); + assert_eq!(source.user_path.as_deref(), Some(m.user.path())); + assert_eq!(source.from_user, vec![persona("pm"), channel("planning")]); + assert!(source.shadowed.is_empty()); + assert_eq!( + source.origin_of(DeclarationKind::Persona, "pm"), + DeclarationOrigin::User + ); + assert_eq!( + source.origin_of(DeclarationKind::Persona, "coder"), + DeclarationOrigin::Workspace + ); + let said = source.explain(); + assert!( + said.contains("(1 declared)"), + "the workspace's OWN count: {said}" + ); + assert!( + said.contains("persona `pm`, channel `planning`") && said.contains("not versioned"), + "{said}" + ); + assert!(source.worth_saying()); +} + +#[test] +fn a_repository_that_declares_nothing_of_its_own_is_not_told_that_nobody_is_declared() { + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + let defs = m.resolve().unwrap(); + let source = defs.source().unwrap(); + assert_eq!(source.kind, DeclarationSourceKind::None); + assert_eq!(source.count, 1); + let said = source.explain(); + assert!(!said.contains("nobody is declared here yet"), "{said}"); + assert!(said.contains("declares nobody of its own"), "{said}"); + assert!(said.contains("persona `pm`"), "{said}"); +} + +#[test] +fn a_repository_file_hides_the_user_level_entry_of_the_same_name_and_says_so() { + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + declare(m.user.path(), "channels.yaml", PLANNING); + declare(&m.repo_personas(), "pm.yaml", REPO_PM); + declare(&m.repo_personas(), "coder.yaml", CODER); + declare( + &m.repo_personas(), + "channels.yaml", + "- name: planning\n members: [pm]\n", + ); + + let defs = m.resolve().unwrap(); + assert_eq!( + defs.role("pm").unwrap().system_prompt, + "The repository's own PM." + ); + assert_eq!(defs.channel("planning").unwrap().members, vec!["pm"]); + assert_eq!(defs.roles().len(), 2, "hidden, not added twice"); + let source = defs.source().unwrap(); + assert!(source.from_user.is_empty()); + assert_eq!(source.shadowed, vec![persona("pm"), channel("planning")]); + let said = source.explain(); + assert!( + said.contains("HIDES the user-level persona `pm`, channel `planning`"), + "{said}" + ); + assert!(said.contains("adds nothing here"), "{said}"); +} + +/// Acceptance point 8: no user-level folder — or one that declares nothing — and a repository reads +/// exactly as it did before the folder existed, down to the sentence. +#[test] +fn without_a_user_level_declaration_the_resolution_is_the_one_from_before() { + let m = Machine::new(); + declare(&m.repo_personas(), "pm.yaml", REPO_PM); + let before = Definitions::resolve_with_user_dir(m.repo.path(), None).unwrap(); + // A folder that exists but carries nothing (only a README) is the same as no folder. + declare(m.user.path(), "README.md", "# mine\n"); + let after = m.resolve().unwrap(); + assert_eq!(before, after); + let source = after.source().unwrap(); + assert_eq!(source.user_path, None); + assert!(!source.worth_saying()); + assert_eq!( + source.explain(), + format!("read from {} (1 declared).", m.repo_personas().display()) + ); + let wire = source.to_value(); + for key in ["user_path", "from_user", "shadowed"] { + assert!(wire.get(key).is_none(), "{key} stays off the wire: {wire}"); + } +} + +#[test] +fn two_user_level_files_declaring_one_handle_are_refused_naming_that_folder() { + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + declare(m.user.path(), "pm-copy.yaml", CENTRAL_PM); + let err = m.resolve().unwrap_err(); + assert!( + err.msg.contains(&m.user.path().display().to_string()) + && err.msg.contains("duplicate role handle"), + "{}", + err.msg + ); +} + +#[test] +fn a_malformed_user_level_file_fails_the_catalogue_with_its_path() { + let m = Machine::new(); + declare(&m.repo_personas(), "coder.yaml", CODER); + declare(m.user.path(), "pm.yaml", "handle: [not, a, handle]\n"); + let err = m.resolve().unwrap_err(); + assert!( + err.msg + .contains(&m.user.path().join("pm.yaml").display().to_string()), + "{}", + err.msg + ); +} + +/// A cycle is judged on the team that RUNS: a repository channel hiding a user-level step relieves +/// the user-level flow of the cycle it would otherwise form. +#[test] +fn a_flow_is_judged_on_the_merged_team() { + let m = Machine::new(); + declare(&m.repo_personas(), "coder.yaml", CODER); + declare( + m.user.path(), + "channels.yaml", + "- name: a\n members: [b]\n- name: b\n members: [a]\n", + ); + let err = m.resolve().unwrap_err(); + assert!(err.msg.contains("reaches itself"), "{}", err.msg); + declare( + &m.repo_personas(), + "channels.yaml", + "- name: b\n members: [coder]\n", + ); + let defs = m.resolve().expect("the repository's b breaks the cycle"); + assert_eq!(defs.source().unwrap().shadowed, vec![channel("b")]); +} + +#[test] +fn from_source_reads_the_same_merged_catalogue_again() { + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + declare(&m.repo_personas(), "coder.yaml", CODER); + let defs = m.resolve().unwrap(); + let again = Definitions::from_source(defs.source().unwrap()).unwrap(); + assert_eq!(defs, again); +} diff --git a/crates/nxs/tests/two_workspaces_on_one_machine.rs b/crates/nxs/tests/two_workspaces_on_one_machine.rs index efe7ddd..ad682d3 100644 --- a/crates/nxs/tests/two_workspaces_on_one_machine.rs +++ b/crates/nxs/tests/two_workspaces_on_one_machine.rs @@ -22,6 +22,11 @@ struct Machine { } impl Machine { + /// The user-level declaration folder of the pinned home's (production) service instance. + fn user_personas(&self) -> PathBuf { + self.home.path().join(".nexusflow/personas") + } + fn dry_log(&self) -> PathBuf { self.home.path().join("starts.log") } @@ -98,16 +103,13 @@ fn machine() -> Machine { let (ok, _, err) = m.run(dir, &["init", "--json", "--module", "chat", "--no-service"]); assert!(ok, "init: {err}"); } - std::fs::create_dir_all(m.alpha.join(".nxs-personas")).unwrap(); + // ONE pm for both repositories, from the user-level folder (nxf 6j6v.7k58): neither repository + // declares it. The pinned home is the production instance's, so the folder is + // `/.nexusflow/personas`. + std::fs::create_dir_all(m.user_personas()).unwrap(); std::fs::write( - m.alpha.join(".nxs-personas/pm.yaml"), - "handle: pm\nsystem_prompt: You are the PM of test/alpha.\n", - ) - .unwrap(); - std::fs::create_dir_all(m.beta.join(".nxs-personas")).unwrap(); - std::fs::write( - m.beta.join(".nxs-personas/pm.yaml"), - "handle: pm\nsystem_prompt: You are the PM of test/beta.\naddressable:\n humans: true\n external:\n - \"*/pm\"\n", + m.user_personas().join("pm.yaml"), + "handle: pm\nsystem_prompt: You are the PM of this repository.\naddressable:\n humans: true\n external:\n - \"*/pm\"\n", ) .unwrap(); m @@ -252,11 +254,83 @@ fn a_commission_crosses_to_the_other_workspace_and_its_coordinator_starts_the_pe assert!(m.beta.join(".nxs/border-open").exists()); } +/// **One definition, every repository** (nxf 6j6v.7k58, acceptance point 7 of the slice): both +/// repositories run the pm out of the user-level folder without a file of their own, `nxc list` +/// says where it came from, and a repository file of the same name hides it — reported, not +/// silent. The scenario test above runs on exactly this pm. +#[test] +fn both_repositories_run_the_user_level_pm_and_a_repository_file_hides_it_out_loud() { + let m = machine(); + for dir in [&m.alpha, &m.beta] { + assert!(!dir.join(".nxs-personas/pm.yaml").exists()); + let list = m.json(dir, &["chat", "--json", "list"]); + let pm = list["personas"] + .as_array() + .unwrap() + .iter() + .find(|p| p["handle"] == "pm") + .unwrap_or_else(|| panic!("pm in {}: {list}", dir.display())); + assert_eq!(pm["origin"], "user", "{list}"); + let declarations = &list["declarations"]; + assert_eq!( + declarations["user_path"], + m.user_personas().display().to_string(), + "{list}" + ); + assert_eq!( + declarations["from_user"], + serde_json::json!([{"kind": "persona", "name": "pm"}]), + "{list}" + ); + let (ok, human, err) = m.run(dir, &["chat", "list"]); + assert!(ok, "{err}"); + assert!( + human.contains("from the user-level folder") && human.contains("persona `pm`"), + "the human list names the source: {human}" + ); + } + + // Beta keeps a copy of its own: it wins, and the hiding is said. + std::fs::write( + m.beta.join(".nxs-personas/pm.yaml"), + "handle: pm\nsystem_prompt: The repository's own pm.\n", + ) + .unwrap(); + let list = m.json(&m.beta, &["chat", "--json", "list"]); + let pm = list["personas"] + .as_array() + .unwrap() + .iter() + .find(|p| p["handle"] == "pm") + .unwrap(); + assert!(pm.get("origin").is_none(), "the repository's own: {list}"); + assert_eq!( + list["declarations"]["shadowed"], + serde_json::json!([{"kind": "persona", "name": "pm"}]), + "{list}" + ); + let (_, human, _) = m.run(&m.beta, &["chat", "list"]); + assert!( + human.contains("HIDES the user-level persona `pm`"), + "the hiding is reported: {human}" + ); + let (ok, prime, err) = m.run(&m.beta, &["prime"]); + assert!(ok, "{err}"); + assert!( + prime.contains("HIDES the user-level persona `pm`"), + "prime reports it too: {prime}" + ); + // Alpha is untouched by beta's copy. + let list = m.json(&m.alpha, &["chat", "--json", "list"]); + assert!(list["declarations"].get("shadowed").is_none(), "{list}"); +} + /// **The scenario with real sessions** (nxf 6j6v.70dy, section 1 of the slice's specification) — /// the PM of one repository asks the PM of another and passes the answer on, through the real /// binary, the real sidecar and a real model. What the routing tests above cannot show: that a /// persona actually commissions across the border, the other persona actually answers, and the -/// answer actually wakes the commissioner. +/// answer actually wakes the commissioner — and, since nxf 6j6v.7k58, that both PMs are ONE +/// definition from the user-level folder, each knowing only its own repository. /// /// `#[ignore]`d for the reasons every live smoke here is: real subscription auth, real seconds, /// real cost. It does NOT pin `HOME` — the SDK's credentials live there — so the registry it @@ -300,6 +374,7 @@ fn live_the_pm_of_one_repository_asks_the_pm_of_another_and_tells_its_owner() { let _ = std::fs::remove_dir_all(&self.0); } } + let service_home_root = service_home.clone(); let _cleanup = Cleanup(service_home); let dirs = TempDir::new().unwrap(); @@ -334,24 +409,31 @@ fn live_the_pm_of_one_repository_asks_the_pm_of_another_and_tells_its_owner() { .unwrap() .success(); assert!(ok); - std::fs::create_dir_all(dir.join(".nxs-personas")).unwrap(); } + // ONE pm for both repositories, from the instance's user-level folder (nxf 6j6v.7k58) — + // neither repository declares it. What each one knows is in ITS repository: the prompt names + // `BOARD.md` relative to the repository, which is the repository the session runs in. + let user_personas = service_home_root.join("personas"); + std::fs::create_dir_all(&user_personas).unwrap(); std::fs::write( - alpha.join(".nxs-personas/pm.yaml"), - "handle: pm\nsystem_prompt: |\n You are the PM of test/alpha. Keep every message short.\n \ + user_personas.join("pm.yaml"), + "handle: pm\naddressable:\n humans: true\n external:\n - \"*/pm\"\nsystem_prompt: |\n \ + You are the PM of the repository you run in. Keep every message short.\n \ When the person asks you to find something out from the PM of another repository, run\n \ exactly `nxc send --to //pm --no-ref \"\"` and end your turn without\n \ - replying. When you are woken with the answer, pass it on to the person with `nxc reply`.\n", + replying. When you are woken with the answer, pass it on to the person with `nxc reply`.\n \ + When the PM of another repository asks YOU something, read `BOARD.md` at the root of this\n \ + repository and answer with `nxc reply`, in one sentence.\n", ) .unwrap(); std::fs::write( - beta.join(".nxs-personas/pm.yaml"), - "handle: pm\naddressable:\n humans: true\n external:\n - \"*/pm\"\nsystem_prompt: |\n \ - You are the PM of test/beta. Keep every message short. Before answering, look at your\n \ - team with `nxc list`. Your board says: the access rule on channels ships with version\n \ - 0.300. Answer what you are asked with `nxc reply`, in one sentence.\n", + beta.join("BOARD.md"), + "The access rule on channels ships with version 0.300.\n", ) .unwrap(); + for dir in [&alpha, &beta] { + assert!(!dir.join(".nxs-personas/pm.yaml").exists()); + } for (dir, other) in [(&alpha, "test/beta"), (&beta, "test/alpha")] { let ok = nxs(dir) .args(["sync", "trust", "add", "--workspace", other]) diff --git a/crates/service/src/home.rs b/crates/service/src/home.rs index ccdd36c..8b93835 100644 --- a/crates/service/src/home.rs +++ b/crates/service/src/home.rs @@ -40,6 +40,9 @@ const HEARTBEAT_FILE: &str = "sync-daemon.json"; const LOG_DIR: &str = "logs"; /// Where the alias lives — see [`ServiceHome::program`]. const BIN_DIR: &str = "bin"; +/// The user-level declarations every workspace on this machine reads beside its own (nxf +/// 6j6v.7k58) — see [`ServiceHome::personas`]. +const PERSONAS_DIR: &str = "personas"; /// The directory a nexus-flow background service keeps its state in, and the files within it. /// @@ -128,6 +131,18 @@ impl ServiceHome { self.root.join(LOG_DIR) } + /// `/personas` — the USER-LEVEL declaration folder (nxf 6j6v.7k58): personas and a + /// `channels.yaml` that every workspace on this machine reads beside its own `.nxs-personas/`, + /// merged per name, the workspace's own winning. + /// + /// It lives in the service home on purpose rather than at a fixed `~/.nexusflow/personas`: a + /// development build reads `~/.nexusflow-/personas`, so a build under test can never + /// pick up the personas the machine's installed suite runs with — the same rule, and the same + /// reason, as the registry beside it. + pub fn personas(&self) -> PathBuf { + self.root.join(PERSONAS_DIR) + } + /// `/bin` — the directory holding the alias. pub fn bin(&self) -> PathBuf { self.root.join(BIN_DIR) From cf10bc0bf55e8ac7435a5caeec57b1e317ab2db3 Mon Sep 17 00:00:00 2001 From: Carsten Koch Date: Sun, 4 Oct 2026 18:37:00 +0200 Subject: [PATCH 2/4] Border follow-ups from the review of PR #14: revoked trust is named, a stuck handover is stopped, the cooperative depth is written down (nxf 6j6v.dcpd) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. When the caller's workspace stops trusting the receiver while a border thread is open, the receiver's newest message is unvouched and steers nothing. The caller's coordinator now cancels at once with "the answer arrived from , a workspace no longer trusted here" and wakes the commissioner, instead of "no sign of life" two hours later. 2. The depth across the border rests on the trusted coordinator's stamp — within the trust model (spec section 4); said where `external` and the depth guard are documented. 3. A peer handover past HANDOVER_WAIT is killed and reaped, so the long-lived service no longer accumulates stuck children. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018Sipejn8YQhKgngxeHpMhT --- changes/dcpd-border-follow-ups.md | 19 ++++ .../chat/docs/guide/de/limits-and-safety.md | 9 ++ .../chat/docs/guide/en/limits-and-safety.md | 7 ++ crates/chat/src/border.rs | 30 ++++++ ...a_persona_commissions_across_the_border.rs | 46 +++++++++ crates/nxs/src/peers.rs | 93 ++++++++++++++----- 6 files changed, 183 insertions(+), 21 deletions(-) create mode 100644 changes/dcpd-border-follow-ups.md diff --git a/changes/dcpd-border-follow-ups.md b/changes/dcpd-border-follow-ups.md new file mode 100644 index 0000000..b7138a1 --- /dev/null +++ b/changes/dcpd-border-follow-ups.md @@ -0,0 +1,19 @@ +--- +type: fixed +--- +[en] +Across a workspace border: when the caller's workspace stops trusting the receiver while a +commission is open, the answer no longer sits unread until the two-hour deadline — the commission is +canceled at once and the commissioner is woken with "the answer arrived from , a +workspace no longer trusted here". A peer's handover that runs past its 60-second limit is now +stopped and reaped instead of being left running, so the background service no longer collects +stuck processes. The guide now says plainly that the depth cap across the border rests on the +trusted workspace's word. +[de] +Über die Grenze eines Arbeitsbereichs: Entzieht der Arbeitsbereich des Aufrufers dem Empfänger das +Vertrauen, während ein Auftrag offen ist, liegt die Antwort nicht mehr ungelesen bis zur +Zwei-Stunden-Frist — der Auftrag wird sofort abgebrochen und der Auftraggeber mit „the answer arrived +from , a workspace no longer trusted here" geweckt. Eine Übergabe beim Nachbarn, die ihre +60-Sekunden-Grenze überschreitet, wird jetzt gestoppt und eingesammelt, statt weiterzulaufen; der +Hintergrunddienst sammelt so keine hängenden Prozesse mehr an. Der Guide sagt jetzt deutlich, dass +die Tiefengrenze über die Grenze auf dem Wort des vertrauten Arbeitsbereichs beruht. diff --git a/crates/chat/docs/guide/de/limits-and-safety.md b/crates/chat/docs/guide/de/limits-and-safety.md index 8fa4972..2ce092d 100644 --- a/crates/chat/docs/guide/de/limits-and-safety.md +++ b/crates/chat/docs/guide/de/limits-and-safety.md @@ -63,6 +63,15 @@ seinen Posts, keine Wiederaufnahme, keine Sitzung, der sich irgendetwas zurechne Lücke zu schließen braucht ein fälschungssicheres Merkmal, das dem gestarteten Prozess übergeben wird, statt einer Umgebungsvariablen — das ist ein Zugriffsentwurf, nicht diese Bremse. +**Über die Grenze eines Arbeitsbereichs gilt das Wort des Aufrufers.** Ein Auftrag aus einem anderen +Arbeitsbereich auf dieser Maschine trägt seine Tiefe im signierten Stempel, den dessen Koordinator +geschrieben hat, und der Empfänger rechnet von dieser Zahl weiter: Er kann sie nicht gegen eine Kette +prüfen, die im Log des anderen Arbeitsbereichs liegt. Ein vertrauter Arbeitsbereich, der jeden Auftrag +mit Tiefe 0 stempelt, hebelt die Grenze zwischen beiden aus. Das liegt innerhalb dessen, was Vertrauen +in einen Arbeitsbereich heißt — seinem Koordinator wird geglaubt, wer fragt, in welcher Rolle und wie +tief —, also ist `nxs sync trust add --workspace` für Arbeitsbereiche, deren Koordinator Sie selbst +laufen ließen. + ## Das menschliche Tor **Kein Mensch steht mitten im Ablauf.** Ein Mensch eröffnet einen Vorgang und ein Mensch liest sein diff --git a/crates/chat/docs/guide/en/limits-and-safety.md b/crates/chat/docs/guide/en/limits-and-safety.md index 9aff66c..56eea37 100644 --- a/crates/chat/docs/guide/en/limits-and-safety.md +++ b/crates/chat/docs/guide/en/limits-and-safety.md @@ -59,6 +59,13 @@ anything to be attributed to. Closing that last gap needs an unforgeable credent spawned process rather than an environment variable, which is an access-control design, not this guard. +**Across a workspace border it is the caller\'s word.** A commission from another workspace on this +machine carries its depth in the signed stamp its coordinator wrote, and the receiver continues from +that number: it cannot check it against a chain that lives in the other workspace\'s log. A trusted +workspace that stamps depth 0 on every commission defeats the cap between the two. That is inside +what trusting a workspace means — its coordinator is trusted to say who asks, in which role, and how +deep — so `nxs sync trust add --workspace` is for workspaces whose coordinator you would run yourself. + ## The human gate **No human stands inside the flow.** A person opens an operation and a person reads its result; diff --git a/crates/chat/src/border.rs b/crates/chat/src/border.rs index 4788144..33d8464 100644 --- a/crates/chat/src/border.rs +++ b/crates/chat/src/border.rs @@ -1239,6 +1239,18 @@ fn act_outbound( return Ok(()); }; let acting = store.acting_messages_in_thread(&row.thread_id)?; + // **An answer from a workspace this one no longer trusts** (nxf 6j6v.dcpd, item 1): trust was + // revoked after the commission, so the receiver's newest message is in the log but steers + // nothing. Left alone the thread would end two hours later as "no sign of life" — bounded, but + // the wrong reason, told to the first person who uses the route. Say the right one, now. + if row.state.is_in_progress() && answered_unvouched(store, &row.thread_id, &worker, &acting)? { + let why = format!( + "the answer arrived from {}, a workspace no longer trusted here", + row.peer + ); + cancel(ctx, store, row, &why)?; + return wake_with_cancellation(ctx, store, row, &why); + } if matches!(row.state, BorderState::Working | BorderState::Submitted) && overdue(ctx, row, &acting) { @@ -1297,6 +1309,24 @@ fn act_outbound( Ok(()) } +/// Whether the receiver's NEWEST message on a border thread is one this workspace cannot vouch for +/// — present in the log, absent from what may act. On a border thread the signature was verified +/// when the message was copied in, so the only way to fall out of `acting` is a signer this +/// workspace does not trust (any more). +fn answered_unvouched( + store: &ChatStore, + thread: &str, + worker: &str, + acting: &[MessageRow], +) -> Result { + let newest = store + .messages_in_thread(thread)? + .into_iter() + .rev() + .find(|m| m.sender == worker); + Ok(newest.is_some_and(|m| !acting.iter().any(|a| a.message_id == m.message_id))) +} + /// **Whether a working border thread has been quiet past its deadline** — [`DEADLINE_HOURS`] since /// the last sign of life (6j6v.nj20, owner 2026-09-17: two hours of INACTIVITY, not two hours from /// the start). A sign of life is a message on the thread, either side's, or an entry in the diff --git a/crates/chat/tests/a_persona_commissions_across_the_border.rs b/crates/chat/tests/a_persona_commissions_across_the_border.rs index 4be38ea..7e5a060 100644 --- a/crates/chat/tests/a_persona_commissions_across_the_border.rs +++ b/crates/chat/tests/a_persona_commissions_across_the_border.rs @@ -858,6 +858,52 @@ fn withdrawing_the_operation_takes_the_border_thread_back_in_both_workspaces() { assert!(w.worker.stopped.lock().unwrap().contains(&beta_pm)); } +/// Trust revoked AFTER the commission (nxf 6j6v.dcpd, item 1): the answer arrives signed by a key +/// this workspace no longer vouches for, so it steers nothing — and the commissioner must hear that +/// reason, now, rather than "no sign of life" two hours later. +#[test] +fn an_answer_from_a_workspace_no_longer_trusted_cancels_it_with_that_reason() { + let tmp = TempDir::new().unwrap(); + let (a, b, w) = two(&tmp, &[ALPHA_PM], &[BETA_PM], true, true); + let (alpha_pm, _) = alpha_session(&a, &w, "pm"); + let receipt = a.as_session(&alpha_pm, "test/beta/pm", "When?").unwrap(); + let beta_pm = w.worker.of(&b, "pm")[0].internal_session.clone(); + + // The owner of alpha stops trusting beta while the thread is open. + { + let ws = nxs_foundation::workspace::discover(&a.root).unwrap(); + let mut store = ws.open_chat_store().unwrap(); + assert!(store.distrust_key(&key_of(&b.root)).unwrap()); + } + b.reply(&beta_pm, &receipt.thread_id, "With version 0.300.", false); + + let out = a.border_rows(); + assert_eq!(out[0].state, BorderState::Canceled, "{out:?}"); + assert!( + out[0] + .reason + .as_deref() + .unwrap() + .contains("test/beta, a workspace no longer trusted here"), + "{out:?}" + ); + let wake = w.worker.of(&a, "pm").last().unwrap().clone(); + assert_eq!(wake.internal_session, alpha_pm); + assert!( + wake.message.contains("no longer trusted"), + "{}", + wake.message + ); + assert!( + !wake.message.contains("0.300"), + "the unvouched answer itself is not handed on: {}", + wake.message + ); + // The commissioner is no longer waiting on it. + let row = status_row(&a, &receipt.thread_id); + assert!(row.outstanding.is_empty(), "{row:?}"); +} + // ---- an answer for a commissioner still in its turn ------------------------------------------ #[test] diff --git a/crates/nxs/src/peers.rs b/crates/nxs/src/peers.rs index 2b95654..0b5f65c 100644 --- a/crates/nxs/src/peers.rs +++ b/crates/nxs/src/peers.rs @@ -99,28 +99,40 @@ impl Peers for ServicePeers { let mut child = cmd .spawn() .map_err(|e| format!("could not start the handover in {}: {e}", peer.name))?; - let deadline = Instant::now() + HANDOVER_WAIT; - loop { - match child.try_wait() { - Ok(Some(status)) if status.success() => return Ok(()), - Ok(Some(status)) => { - return Err(format!( - "the handover in {} exited with {status}", - peer.name - )) - } - Ok(None) if Instant::now() < deadline => { - std::thread::sleep(Duration::from_millis(50)) - } - Ok(None) => { - return Err(format!( - "the handover in {} is still running after {}s; it goes on by itself", - peer.name, - HANDOVER_WAIT.as_secs() - )) - } - Err(e) => return Err(format!("waiting for the handover in {}: {e}", peer.name)), + wait_or_stop(&mut child, HANDOVER_WAIT, &peer.name) + } +} + +/// Wait for a peer's handover, and STOP it when it outlives `limit` (nxf 6j6v.dcpd, item 3). +/// +/// It used to be left running with "it goes on by itself", and its child was then reaped by nobody +/// until the calling process exited. Harmless for a short-lived CLI; the background service is +/// long-lived, and it runs a border pass every few seconds, so one wedged handover per pass would +/// pile up processes for as long as the service runs. A handover only copies a thread and starts a +/// persona session it does not wait for, so one past the limit is stuck rather than busy: killing it +/// rolls back at most one SQLite transaction, and the next pass hands over whatever it had not. +/// Killed AND waited for, so no zombie is left behind either. +fn wait_or_stop( + child: &mut std::process::Child, + limit: Duration, + name: &str, +) -> Result<(), String> { + let deadline = Instant::now() + limit; + loop { + match child.try_wait() { + Ok(Some(status)) if status.success() => return Ok(()), + Ok(Some(status)) => return Err(format!("the handover in {name} exited with {status}")), + Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(50)), + Ok(None) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!( + "the handover in {name} was still running after {}s and was stopped; the \ + service's next pass hands over what it had not", + limit.as_secs() + )); } + Err(e) => return Err(format!("waiting for the handover in {name}: {e}")), } } } @@ -142,4 +154,43 @@ mod tests { .expect_err("nothing to start in"); assert!(err.contains("test/gone"), "{err}"); } + + #[cfg(unix)] + #[test] + fn a_handover_past_its_limit_is_stopped_and_reaped_not_left_running() { + // nxf 6j6v.dcpd, item 3: the long-lived service must not collect wedged children. + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .unwrap(); + let pid = child.id(); + let started = Instant::now(); + let err = wait_or_stop(&mut child, Duration::from_millis(200), "test/stuck") + .expect_err("past the limit"); + assert!( + err.contains("test/stuck") && err.contains("stopped"), + "{err}" + ); + assert!( + started.elapsed() < Duration::from_secs(10), + "it did not wait for sleep" + ); + // Reaped: the pid is gone, not a zombie still answering `kill -0`. + let alive = std::process::Command::new("kill") + .args(["-0", &pid.to_string()]) + .stderr(std::process::Stdio::null()) + .status() + .unwrap() + .success(); + assert!(!alive, "the child {pid} was left behind"); + } + + #[test] + fn a_handover_that_finishes_in_time_is_ok() { + let mut child = std::process::Command::new("true").spawn().unwrap(); + assert_eq!( + wait_or_stop(&mut child, Duration::from_secs(10), "test/quick"), + Ok(()) + ); + } } From 4a7a7a5b097a466268777979464d5c3fcca95a46 Mon Sep 17 00:00:00 2001 From: Carsten Koch Date: Sun, 4 Oct 2026 19:02:21 +0200 Subject: [PATCH 3/4] Pin the in-process home through pinned_home_env, and mark the dcpd fragment's facade impact The two single-test binaries set HOME by hand, which the source gate every_pinned_home_pins_the_instance refuses: a pinned home without the instance resolves another directory under this repo's .envrc. They now apply nxs_test_support::pinned_home_env() to their own process. The dcpd fragment says `facade: changed` (a behaviour change behind unchanged signatures: an unvouched answer now cancels the border thread). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018Sipejn8YQhKgngxeHpMhT --- changes/dcpd-border-follow-ups.md | 1 + ...er_level_declaration_is_frozen_per_operation.rs | 11 ++++++----- .../tests/one_definition_at_the_engine_seam.rs | 14 ++++++++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/changes/dcpd-border-follow-ups.md b/changes/dcpd-border-follow-ups.md index b7138a1..49a92b5 100644 --- a/changes/dcpd-border-follow-ups.md +++ b/changes/dcpd-border-follow-ups.md @@ -1,5 +1,6 @@ --- type: fixed +facade: changed --- [en] Across a workspace border: when the caller's workspace stops trusting the receiver while a diff --git a/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs index ec19f29..903b786 100644 --- a/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs +++ b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs @@ -72,11 +72,12 @@ fn open(engine: &Engine) -> String { #[test] fn an_edit_to_the_user_level_folder_reaches_the_next_operation_and_not_the_running_one() { - let home = TempDir::new().unwrap(); - // SAFETY of the mutation: this binary has exactly one test, and nothing else runs in it. - std::env::set_var("HOME", home.path()); - std::env::remove_var("XDG_CONFIG_HOME"); - std::env::remove_var("XDG_DATA_HOME"); + // The pinning `nxs_test_support::PinHome` gives a subprocess, applied to THIS process: the + // home, the XDG directories and the instance together (nxf 6j6v.9bjv). Process-wide, which is + // safe because this binary has exactly one test. + for (key, value) in nxs_test_support::pinned_home_env() { + std::env::set_var(key, value); + } let user = nexus_chat::definitions::user_declarations_dir().unwrap(); std::fs::create_dir_all(&user).unwrap(); declare_version(&user, "VERSION-ONE"); diff --git a/crates/chat/tests/one_definition_at_the_engine_seam.rs b/crates/chat/tests/one_definition_at_the_engine_seam.rs index 4ee8c52..e00a6a0 100644 --- a/crates/chat/tests/one_definition_at_the_engine_seam.rs +++ b/crates/chat/tests/one_definition_at_the_engine_seam.rs @@ -12,15 +12,17 @@ const NOW: &str = "2026-10-04T10:00:00Z"; #[test] fn the_engine_reads_the_user_level_folder_the_cli_reads_and_says_where_each_entry_came_from() { - let home = TempDir::new().unwrap(); - // SAFETY of the mutation: this binary has exactly one test, and nothing else runs in it. - std::env::set_var("HOME", home.path()); - std::env::remove_var("XDG_CONFIG_HOME"); - std::env::remove_var("XDG_DATA_HOME"); + // The pinning `nxs_test_support::PinHome` gives a subprocess, applied to THIS process: the + // home, the XDG directories and the instance together (nxf 6j6v.9bjv). Process-wide, which is + // safe because this binary has exactly one test. + for (key, value) in nxs_test_support::pinned_home_env() { + std::env::set_var(key, value); + } + let home = nxs_test_support::pinned_home(); // Whichever instance the ambient environment names, the folder is that instance's — resolved // the way production resolves it, not spelled out here. let user = nexus_chat::definitions::user_declarations_dir().unwrap(); - assert!(user.starts_with(home.path()), "{}", user.display()); + assert!(user.starts_with(home), "{}", user.display()); std::fs::create_dir_all(&user).unwrap(); std::fs::write( user.join("pm.yaml"), From 2ec45afd6446cb8d0b8035e132a74a295788e904 Mon Sep 17 00:00:00 2001 From: Carsten Koch Date: Sun, 4 Oct 2026 22:14:03 +0200 Subject: [PATCH 4/4] Mitigate the review of PR #17: hermetic user-level folder, channel-only policy reads, named revoked-trust cause, group stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A build reads a user-level folder only under a NAMED development instance, never the production one, and an unresolvable home or instance means no folder rather than an error (Code #2, #3; Integrity #3, #4). Guard test: a build under the production instance does not see personas planted in the production folder. - An existing but unreadable user-level folder is an error, not a silence (Integrity #5). - `threads show`, `search`, `Engine::thread` and `Engine::search` read the merged channels only (`merged_channels`), so a malformed persona file cannot make a thread unreadable (Code #1). - `nxc prime`, `Engine::prime_as` and `nxs prime --persona` resolve the catalogue once (`prime_with`) (Code #5). - The revoked-trust cancel fires only on an answer not yet served, so a question delivered before the revocation does not cancel; two negative controls, mutation-checked (Test #1, Integrity #7). - A stuck peer handover runs in its own process group, which is killed as a whole and then reaped by polling; the sidecar it started has a group of its own and survives; start-idempotence rests on the admission claim (Code #7, Integrity #6). Test drops the wall-clock assertion and gates `true` on unix (Test #3, #4). - The freeze test now edits a user-level hurdle too (Code #6); loader failing cases for channels, a missing and an unreadable folder, a dangling member (Test #2). - A user-level `external` admits only from workspaces the running repository trusts — tested through the binary and written in the guide (Integrity #2); the guide no longer calls a hiding "never silent" but says where it is shown (Integrity #1, the lock question filed as 6j6v.3mgy). - Stale prose and wraps (Code #4, #9). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018Sipejn8YQhKgngxeHpMhT --- changes/7k58-one-definition-many-repos.md | 21 ++-- .../chat/docs/guide/de/limits-and-safety.md | 13 +- crates/chat/docs/guide/de/personas.md | 82 +++++++------ .../chat/docs/guide/en/limits-and-safety.md | 8 +- crates/chat/docs/guide/en/personas.md | 37 +++--- crates/chat/src/border.rs | 19 ++- crates/chat/src/cli.rs | 21 ++-- crates/chat/src/definitions.rs | 85 +++++++++++-- crates/chat/src/engine.rs | 26 ++-- crates/chat/src/facade.rs | 37 +++++- ...a_persona_commissions_across_the_border.rs | 61 ++++++++- ...vel_declaration_is_frozen_per_operation.rs | 49 ++++++-- .../one_definition_at_the_engine_seam.rs | 38 ++++-- .../tests/one_definition_many_repositories.rs | 96 +++++++++++++++ crates/nxs/src/peers.rs | 116 ++++++++++++++---- crates/nxs/src/prime.rs | 6 +- .../tests/two_workspaces_on_one_machine.rs | 64 +++++++++- 17 files changed, 616 insertions(+), 163 deletions(-) diff --git a/changes/7k58-one-definition-many-repos.md b/changes/7k58-one-definition-many-repos.md index 50c5b67..291d220 100644 --- a/changes/7k58-one-definition-many-repos.md +++ b/changes/7k58-one-definition-many-repos.md @@ -12,12 +12,15 @@ memory and working copy. `nxc list` marks each entry that comes from the user-le which ones the repository hides, and so does `nxs prime`; a persona from it is told at session start where its declaration lives, because a path in its instructions relative to "this repository" means the repository it runs in. The per-operation freeze holds for it as for the repository's own folder. -A development build reads `~/.nexusflow-/personas/` of its own service instance. A machine -without the folder behaves exactly as before. For embedding apps: `Engine::definitions`, +A user-level `external` admits callers only from workspaces the running repository itself trusts +by name. A development build reads a user-level folder only under a named service instance +(`~/.nexusflow-/personas/`), never the production one. A machine without the folder behaves +as before. For embedding apps: `Engine::definitions`, `Engine::directory` and `Engine::prime_as` read the same folder (an app cannot choose another); new `user_path`, `from_user` and `shadowed` on `DeclarationSource`, `origin` on `PersonaEntry` and -`ChannelEntry`, `declared_in` on `PersonaBrief`, and `Definitions::resolve_with_user_dir` / -`Definitions::from_source` — a struct literal of those four types needs the new fields. +`ChannelEntry`, `declared_in` on `PersonaBrief`, `user_declarations_dir`, `merged_channels`, and +`Definitions::resolve_with_user_dir` / `Definitions::from_source` — a struct literal of those four +types needs the new fields. [de] Eine Persona-Definition für jedes Repository auf einer Maschine. Neben dem eigenen `.nxs-personas/` liest jeder Arbeitsbereich jetzt den Ordner auf Benutzerebene `~/.nexusflow/personas/` — Personas @@ -28,10 +31,12 @@ Teilnehmer, mit dessen Board, Gedächtnis und Arbeitskopie. `nxc list` markiert Ordner auf Benutzerebene und sagt, welche das Repository verdeckt, `nxs prime` ebenso; eine Persona von dort erfährt beim Sitzungsstart, wo ihre Deklaration liegt, denn ein Pfad in ihren Anweisungen relativ zu „diesem Repository" meint das Repository, in dem sie läuft. Das Einfrieren je Vorgang gilt -dort wie für den eigenen Ordner. Ein Entwicklungs-Build liest `~/.nexusflow-/personas/` seiner -eigenen Dienst-Instanz. Eine Maschine ohne den Ordner verhält sich genau wie bisher. Für einbettende +dort wie für den eigenen Ordner. Ein `external` auf Benutzerebene lässt Aufrufer nur aus Arbeitsbereichen +zu, denen das laufende Repository selbst beim Namen vertraut. Ein Entwicklungs-Build liest einen +Ordner auf Benutzerebene nur unter einer benannten Dienst-Instanz (`~/.nexusflow-/personas/`), +nie den produktiven. Eine Maschine ohne den Ordner verhält sich wie bisher. Für einbettende Apps: `Engine::definitions`, `Engine::directory` und `Engine::prime_as` lesen denselben Ordner (eine App kann keinen anderen wählen); neu sind `user_path`, `from_user` und `shadowed` an -`DeclarationSource`, `origin` an `PersonaEntry` und `ChannelEntry`, `declared_in` an `PersonaBrief` -sowie `Definitions::resolve_with_user_dir` / `Definitions::from_source` — ein Struct-Literal dieser +`DeclarationSource`, `origin` an `PersonaEntry` und `ChannelEntry`, `declared_in` an `PersonaBrief`, +`user_declarations_dir`, `merged_channels` sowie `Definitions::resolve_with_user_dir` / `Definitions::from_source` — ein Struct-Literal dieser vier Typen braucht die neuen Felder. diff --git a/crates/chat/docs/guide/de/limits-and-safety.md b/crates/chat/docs/guide/de/limits-and-safety.md index 2ce092d..dc19e06 100644 --- a/crates/chat/docs/guide/de/limits-and-safety.md +++ b/crates/chat/docs/guide/de/limits-and-safety.md @@ -65,13 +65,12 @@ wird, statt einer Umgebungsvariablen — das ist ein Zugriffsentwurf, nicht dies **Über die Grenze eines Arbeitsbereichs gilt das Wort des Aufrufers.** Ein Auftrag aus einem anderen Arbeitsbereich auf dieser Maschine trägt seine Tiefe im signierten Stempel, den dessen Koordinator -geschrieben hat, und der Empfänger rechnet von dieser Zahl weiter: Er kann sie nicht gegen eine Kette -prüfen, die im Log des anderen Arbeitsbereichs liegt. Ein vertrauter Arbeitsbereich, der jeden Auftrag -mit Tiefe 0 stempelt, hebelt die Grenze zwischen beiden aus. Das liegt innerhalb dessen, was Vertrauen -in einen Arbeitsbereich heißt — seinem Koordinator wird geglaubt, wer fragt, in welcher Rolle und wie -tief —, also ist `nxs sync trust add --workspace` für Arbeitsbereiche, deren Koordinator Sie selbst -laufen ließen. - +geschrieben hat, und der Empfänger rechnet von dieser Zahl weiter: Er kann sie nicht gegen eine +Kette prüfen, die im Log des anderen Arbeitsbereichs liegt. Ein vertrauter Arbeitsbereich, der jeden +Auftrag mit Tiefe 0 stempelt, hebelt die Grenze zwischen beiden aus. Das liegt innerhalb dessen, was +Vertrauen in einen Arbeitsbereich heißt — seinem Koordinator wird geglaubt, wer fragt, in welcher +Rolle und wie tief —, also ist `nxs sync trust add --workspace` für Arbeitsbereiche, deren +Koordinator Sie selbst laufen ließen. ## Das menschliche Tor **Kein Mensch steht mitten im Ablauf.** Ein Mensch eröffnet einen Vorgang und ein Mensch liest sein diff --git a/crates/chat/docs/guide/de/personas.md b/crates/chat/docs/guide/de/personas.md index f662fbf..ca9ae97 100644 --- a/crates/chat/docs/guide/de/personas.md +++ b/crates/chat/docs/guide/de/personas.md @@ -280,20 +280,21 @@ addressable: on this machine*, *not trusted*, *role not admitted*, *no such persona* oder *depth limit reached*. -Was dann geschieht, ist eine Konsultation mit einer Übergabe in der Mitte. Der Auftrag des -Aufrufers ist ein Faden in dessen eigenem Log, und nur dieser Faden geht hinüber: Der Empfänger -startet seine Persona in SEINER Arbeitskopie, mit SEINEM Board und Gedächtnis, und was diese Persona -beauftragt, um zu antworten, bleibt in ihrem Arbeitsbereich. Eine Rückfrage (`--escalate`) geht an -den Auftraggeber zurück — nie an ihm vorbei zum Besitzer des anderen Arbeitsbereichs —, und die -Antwort des Auftraggebers auf demselben Faden setzt die Persona fort. Die Tiefengrenze gilt für die -ganze Kette über beide Arbeitsbereiche — auf das Wort des Aufrufers: Die Tiefe reist im signierten -Stempel des Auftrags, und der Empfänger kann sie nicht gegen eine Kette prüfen, die er nie sieht. Einem +Was dann geschieht, ist eine Konsultation mit einer Übergabe in der Mitte. Der Auftrag des Aufrufers +ist ein Faden in dessen eigenem Log, und nur dieser Faden geht hinüber: Der Empfänger startet seine +Persona in SEINER Arbeitskopie, mit SEINEM Board und Gedächtnis, und was diese Persona beauftragt, +um zu antworten, bleibt in ihrem Arbeitsbereich. Eine Rückfrage (`--escalate`) geht an den +Auftraggeber zurück — nie an ihm vorbei zum Besitzer des anderen Arbeitsbereichs —, und die Antwort +des Auftraggebers auf demselben Faden setzt die Persona fort. Die Tiefengrenze gilt für die ganze +Kette über beide Arbeitsbereiche — auf das Wort des Aufrufers: Die Tiefe reist im signierten Stempel +des Auftrags, und der Empfänger kann sie nicht gegen eine Kette prüfen, die er nie sieht. Einem Arbeitsbereich zu vertrauen heißt, seinem Koordinator zu vertrauen, und ein feindseliger könnte -Tiefe 0 stempeln; vertrauen Sie nur Arbeitsbereichen, deren Koordinator Sie selbst laufen ließen. `nxc status` zeigt den Faden in beiden Arbeitsbereichen mit -der Gegenseite und einem von vier Zuständen: `submitted`, `working`, `input-required`, `completed` -(oder `rejected` / `canceled`). Zwei Stunden ohne Lebenszeichen — keine Nachricht auf dem Faden, -nichts im Transkript der empfangenden Persona — brechen ihn ab und wecken den Auftraggeber mit dem -Grund; `nxc withdraw` auf der Operation zieht ihn auch im anderen Arbeitsbereich zurück. +Tiefe 0 stempeln; vertrauen Sie nur Arbeitsbereichen, deren Koordinator Sie selbst laufen ließen. +`nxc status` zeigt den Faden in beiden Arbeitsbereichen mit der Gegenseite und einem von vier +Zuständen: `submitted`, `working`, `input-required`, `completed` (oder `rejected` / `canceled`). +Zwei Stunden ohne Lebenszeichen — keine Nachricht auf dem Faden, nichts im Transkript der +empfangenden Persona — brechen ihn ab und wecken den Auftraggeber mit dem Grund; `nxc withdraw` auf +der Operation zieht ihn auch im anderen Arbeitsbereich zurück. `send` und `reply` tragen einen Grenzfaden sofort hinüber, die Zustellung braucht also keinen Hintergrunddienst; den Rest trägt der Dienst in seinem Takt. Ein Fall braucht ihn doch, wie @@ -504,49 +505,58 @@ es sehen kann. ## Eine Definition für jedes Repo -Eine Persona, die Sie in jedem Repo haben wollen — denselben `pm` überall —, müssen Sie nicht in jedes -kopieren. Neben dem eigenen `.nxs-personas/` eines Workspace gibt es einen **Ordner auf -Benutzerebene**, `~/.nexusflow/personas/`, genauso aufgebaut: eine `.yaml` je Persona und eine -`channels.yaml`. Jeder Workspace auf der Maschine liest ihn. +Eine Persona, die Sie in jedem Repo haben wollen — denselben `pm` überall —, müssen Sie nicht in +jedes kopieren. Neben dem eigenen `.nxs-personas/` eines Workspace gibt es einen **Ordner auf +Benutzerebene**, `~/.nexusflow/personas/`, genauso aufgebaut: eine `.yaml` je Persona und +eine `channels.yaml`. Jeder Workspace auf der Maschine liest ihn. - **Zusammengeführt je Name.** Eine Persona oder ein Kanal, den das Repo selbst deklariert, - **verdeckt** den gleichnamigen Eintrag auf Benutzerebene; alles andere kommt hinzu. Ein Kanal kommt - mit den Personas, die er voraussetzt, und seine Mitglieder lösen gegen das zusammengeführte Team - auf — ein Kanal `planning` auf Benutzerebene, der `coder` nennt, erreicht den eigenen `coder` jedes - Repos. + **verdeckt** den gleichnamigen Eintrag auf Benutzerebene; alles andere kommt hinzu. Ein Kanal + kommt mit den Personas, die er voraussetzt, und seine Mitglieder lösen gegen das zusammengeführte + Team auf — ein Kanal `planning` auf Benutzerebene, der `coder` nennt, erreicht den eigenen `coder` + jedes Repos. - **Die Identität bleibt je Repo.** Aus einer Definition `pm` wird in jedem Repo, in dem sie läuft, ein eigener Teilnehmer, mit Board, Gedächtnis und Arbeitskopie dieses Repos. - **Woher etwas kommt, wird gezeigt.** `nxc list` markiert jeden Eintrag aus dem Ordner auf - Benutzerebene und sagt, welche Einträge das Repo verdeckt; `nxc list --json` trägt an einem solchen - Eintrag `"origin": "user"` und unter `declarations` die Felder `user_path`, `from_user` und - `shadowed`. `nxs prime` sagt dasselbe unter „Declarations". Eine Verdeckung ist nie still: ein Repo, - das noch eine alte Kopie hält, führt die alte Kopie aus, und das sollten Sie wissen. + Benutzerebene und sagt, welche Einträge das Repo verdeckt; `nxc list --json` trägt an einem + solchen Eintrag `"origin": "user"` und unter `declarations` die Felder `user_path`, `from_user` + und `shadowed`. `nxs prime` sagt dasselbe unter „Declarations". Eine Verdeckung wird dort gemeldet + und nirgends sonst — nicht beim Öffnen eines Vorgangs, nicht in der Sitzung, die er startet. Ein + Repo, das eine eigene Kopie hält, führt diese Kopie aus, mit ihren eigenen Hürden, Mitgliedern und + Zugangsregeln; der Ordner auf Benutzerebene kann das nicht verbieten. - **Wie der Ordner gefüllt wird, entscheiden Sie** — ein Git-Klon, eine symbolische Verknüpfung. Der Ordner ist der Vertrag, nicht seine Herkunft. -**Der Preis, ausgesprochen:** Eine Deklaration außerhalb des Repos ist nicht mit ihm versioniert, und -ein Klon auf einer Maschine ohne den Ordner hat keinen `pm`. Alles, was oben über Versionsverwaltung -steht, gilt für diesen Ordner genauso — halten Sie ihn in einem eigenen Repo. +**Der Preis, ausgesprochen:** Eine Deklaration außerhalb des Repos ist nicht mit ihm versioniert, +und ein Klon auf einer Maschine ohne den Ordner hat keinen `pm`. Alles, was oben über +Versionsverwaltung steht, gilt für diesen Ordner genauso — halten Sie ihn in einem eigenen Repo. Drei Dinge gelten genau wie für den eigenen Ordner des Repos: - **Er wird je Vorgang eingefroren.** Ein Vorgang läuft unter dem zusammengeführten Katalog, wie er beim Öffnen des Vorgangs stand — Hürden (`preconditions:`) aus dem Ordner auf Benutzerebene eingeschlossen. Eine Änderung dort erreicht den nächsten Vorgang, nie einen laufenden. -- **Ein relativer Pfad meint das Repo, in dem die Persona läuft.** Nichts löst einen Pfad auf, der in - einer Deklaration steht: Ein System-Prompt, der sagt „lies `knowledge/x.md`, relativ zur Wurzel - dieses Arbeitsbereichs", wird von einer Sitzung gelesen, deren Arbeitsverzeichnis das Repo ist, und - eine Hürde läuft ebenfalls dort. Dateien, die *neben* einer Deklaration auf Benutzerebene liegen, - werden so also nicht gefunden. Eine Persona aus dem Ordner auf Benutzerebene erfährt beim +- **Ein relativer Pfad meint das Repo, in dem die Persona läuft.** Nichts löst einen Pfad auf, der + in einer Deklaration steht: Ein System-Prompt, der sagt „lies `knowledge/x.md`, relativ zur Wurzel + dieses Arbeitsbereichs", wird von einer Sitzung gelesen, deren Arbeitsverzeichnis das Repo ist, + und eine Hürde läuft ebenfalls dort. Dateien, die *neben* einer Deklaration auf Benutzerebene + liegen, werden so also nicht gefunden. Eine Persona aus dem Ordner auf Benutzerebene erfährt beim Sitzungsstart, wo ihre Deklaration liegt („Declared in"), damit ihre Anweisungen dorthin zeigen können. - **Eine einbettende App liest denselben Ordner.** Sie wählt keinen anderen und kann es nicht: Die Personas, die sie startet, laufen mit `nxc`, das diesen Ordner liest, und zwei Antworten auf „wer existiert" würden eine App von ihren eigenen Sitzungen trennen. -Ein Entwicklungs-Build liest den Ordner auf Benutzerebene seiner eigenen Dienst-Instanz — -`~/.nexusflow-/personas/`, wenn `NXS_SERVICE_INSTANCE` sie benennt —, sodass ein Build im Test nie -die Personas aufgreift, mit denen Ihre installierte Suite läuft. +**Ein `external` aus dem Ordner auf Benutzerebene öffnet kein Repo von selbst.** Eine Persona auf +Benutzerebene, die Aufrufer von außen zulässt, ist in jedem Repo zulässig, das sie führt, aber nur +aus den Arbeitsbereichen, denen dieses Repo in seiner eigenen Vertrauensliste beim Namen vertraut +(`nxs sync trust add --workspace`). Ein Repo, das niemandem vertraut, lässt von außen niemanden +herein. + +Ein Entwicklungs-Build liest einen Ordner auf Benutzerebene nur unter einer benannten Dienst-Instanz +— `~/.nexusflow-/personas/`, wenn `NXS_SERVICE_INSTANCE` eine benennt. Ein Build ohne sie (CI, +eine Shell ohne `direnv`) liest keinen, sodass ein Build im Test nie die Personas aufgreift, mit +denen Ihre installierte Suite läuft. ## Wenn eine Deklaration falsch ist diff --git a/crates/chat/docs/guide/en/limits-and-safety.md b/crates/chat/docs/guide/en/limits-and-safety.md index 56eea37..6b222f7 100644 --- a/crates/chat/docs/guide/en/limits-and-safety.md +++ b/crates/chat/docs/guide/en/limits-and-safety.md @@ -59,13 +59,13 @@ anything to be attributed to. Closing that last gap needs an unforgeable credent spawned process rather than an environment variable, which is an access-control design, not this guard. -**Across a workspace border it is the caller\'s word.** A commission from another workspace on this +**Across a workspace border it is the caller's word.** A commission from another workspace on this machine carries its depth in the signed stamp its coordinator wrote, and the receiver continues from -that number: it cannot check it against a chain that lives in the other workspace\'s log. A trusted +that number: it cannot check it against a chain that lives in the other workspace's log. A trusted workspace that stamps depth 0 on every commission defeats the cap between the two. That is inside what trusting a workspace means — its coordinator is trusted to say who asks, in which role, and how -deep — so `nxs sync trust add --workspace` is for workspaces whose coordinator you would run yourself. - +deep — so `nxs sync trust add --workspace` is for workspaces whose coordinator you would run +yourself. ## The human gate **No human stands inside the flow.** A person opens an operation and a person reads its result; diff --git a/crates/chat/docs/guide/en/personas.md b/crates/chat/docs/guide/en/personas.md index 2ff921d..3bb7a31 100644 --- a/crates/chat/docs/guide/en/personas.md +++ b/crates/chat/docs/guide/en/personas.md @@ -265,14 +265,14 @@ thread in its own log, and only that thread crosses: the receiver starts its per copy, with ITS board and memory, and whatever that persona commissions to answer stays in its own workspace. A question (`--escalate`) goes back to the commissioner — never past it to the other workspace's owner — and the commissioner's answer on the same thread resumes the persona. The depth -cap holds for the whole chain across both workspaces — on the caller's word: the depth -travels in the commission's signed stamp, and the receiver cannot check it against a chain it never -sees. Trusting a workspace means trusting its coordinator, and a hostile one could stamp depth 0; -trust only workspaces whose coordinator you would run yourself. `nxc status` shows the thread in both +cap holds for the whole chain across both workspaces — on the caller's word: the depth travels in +the commission's signed stamp, and the receiver cannot check it against a chain it never sees. +Trusting a workspace means trusting its coordinator, and a hostile one could stamp depth 0; trust +only workspaces whose coordinator you would run yourself. `nxc status` shows the thread in both workspaces with the other side and one of four states: `submitted`, `working`, `input-required`, `completed` (or `rejected` / `canceled`). Two hours without a sign of life — no message on the -thread, nothing in the receiving persona's transcript — cancel it and wake the commissioner with -the reason; `nxc withdraw` on the operation takes it back in the other workspace too. +thread, nothing in the receiving persona's transcript — cancel it and wake the commissioner with the +reason; `nxc withdraw` on the operation takes it back in the other workspace too. `send` and `reply` carry a border thread across at once, so delivery does not need the background service; the service's pass carries the rest. One case does need it, as it does inside a workspace: @@ -485,8 +485,9 @@ each one. Beside a workspace's own `.nxs-personas/` there is a **user-level fold - **Where it came from is shown.** `nxc list` marks each entry from the user-level folder and says which entries the repository hides; `nxc list --json` carries `"origin": "user"` on such an entry and `user_path`, `from_user` and `shadowed` under `declarations`. `nxs prime` says the same under - "Declarations". A hiding is never silent: a repository that still keeps an old copy runs the old - copy, and you should know. + "Declarations". A hiding is reported there and nowhere else — not when an operation opens, not in + the session it starts. A repository that keeps its own copy runs that copy, with its own hurdles, + members and access rules; the user-level folder cannot forbid it. - **How the folder is filled is yours to decide** — a git clone, a symbolic link. The folder is the contract, not its history. @@ -501,17 +502,23 @@ Three things hold exactly as they do for the repository's own folder: edit there reaches the next operation, never a running one. - **A relative path means the repository the persona runs in.** Nothing resolves a path written in a declaration: a system prompt that says "read `knowledge/x.md`, relative to the root of this - workspace" is read by a session whose working directory is the repository, and a hurdle runs - there too. Files kept *beside* a user-level declaration are therefore not found that way. A - persona from the user-level folder is told at session start where its declaration lives - ("Declared in"), so its instructions can point there instead. + workspace" is read by a session whose working directory is the repository, and a hurdle runs there + too. Files kept *beside* a user-level declaration are therefore not found that way. A persona from + the user-level folder is told at session start where its declaration lives ("Declared in"), so its + instructions can point there instead. - **An embedding app reads the same folder.** It does not choose another one, and cannot: the personas it starts run `nxc`, which reads this folder, and two answers to "who exists" would split an app from its own sessions. -A development build reads the user-level folder of its own service instance — -`~/.nexusflow-/personas/` when `NXS_SERVICE_INSTANCE` names it — so a build under test never -picks up the personas your installed suite runs with. +**An `external` from the user-level folder opens no repository by itself.** A user-level persona +that admits callers from outside is admissible in every repository that runs it, but only from the +workspaces that repository trusts by name in its own trust list (`nxs sync trust add --workspace`). +A repository that trusts nobody admits nobody from outside. + +A development build reads a user-level folder only under a named service instance — +`~/.nexusflow-/personas/` when `NXS_SERVICE_INSTANCE` names one. A build without one (CI, a +shell without `direnv`) reads none, so a build under test never picks up the personas your installed +suite runs with. ## When a declaration is wrong diff --git a/crates/chat/src/border.rs b/crates/chat/src/border.rs index 33d8464..4f96753 100644 --- a/crates/chat/src/border.rs +++ b/crates/chat/src/border.rs @@ -1310,9 +1310,14 @@ fn act_outbound( } /// Whether the receiver's NEWEST message on a border thread is one this workspace cannot vouch for -/// — present in the log, absent from what may act. On a border thread the signature was verified -/// when the message was copied in, so the only way to fall out of `acting` is a signer this -/// workspace does not trust (any more). +/// and has not handed on — present in the log, absent from what may act, and not served to the +/// commissioner. On a border thread the signature was verified when the message was copied in, so +/// the only way to fall out of `acting` is a signer this workspace does not trust (any more). +/// +/// **Not served, and that is the point** (review of PR #17, Integrity #7): revoking trust also +/// takes a message that was vouched for and already DELIVERED out of `acting` — a question the +/// commissioner has been woken with and is answering. That is not an answer that arrived from an +/// untrusted workspace; cancelling on it would be irreversible and say the wrong thing. fn answered_unvouched( store: &ChatStore, thread: &str, @@ -1324,7 +1329,13 @@ fn answered_unvouched( .into_iter() .rev() .find(|m| m.sender == worker); - Ok(newest.is_some_and(|m| !acting.iter().any(|a| a.message_id == m.message_id))) + let Some(newest) = newest else { + return Ok(false); + }; + if acting.iter().any(|a| a.message_id == newest.message_id) { + return Ok(false); + } + Ok(!is_served(store, thread, &newest.message_id)?) } /// **Whether a working border thread has been quiet past its deadline** — [`DEADLINE_HOURS`] since diff --git a/crates/chat/src/cli.rs b/crates/chat/src/cli.rs index 1e58de0..88bb324 100644 --- a/crates/chat/src/cli.rs +++ b/crates/chat/src/cli.rs @@ -1198,15 +1198,12 @@ fn workspace_root(db: Option<&str>) -> Result { .to_path_buf()) } -/// The declared channels this invocation resolves through — the MERGED catalogue's (nxf 6j6v.7k58), -/// so a channel that arrives from the user-level folder gates `threads show` and `search` exactly -/// like one the workspace declares. +/// The declared channels this invocation resolves through — the workspace's merged with the +/// user-level ones (nxf 6j6v.7k58), so a channel from the user-level folder gates `threads show` and +/// `search` exactly like one the workspace declares. Channels ONLY: a malformed persona file must +/// not make a thread unreadable ([`crate::definitions::merged_channels`]). fn declared_channels(db: Option<&str>) -> Result> { - Ok( - crate::definitions::Definitions::resolve(&workspace_root(db)?)? - .channels() - .to_vec(), - ) + crate::definitions::merged_channels(&workspace_root(db)?) } /// Everything [`crate::orchestration::Ctx`] borrows, owned — the CLI adapter's job in one place. @@ -1886,18 +1883,14 @@ fn prime( // human. See `crate::persona` for why it is deliberately not the process id, and for what the // answer may and may not be used for. let defs = crate::definitions::Definitions::resolve(&workspace_root(db)?)?; - let declarations = defs - .source() - .cloned() - .expect("Definitions::resolve always records its source"); let identity = crate::persona::resolve_identity(&store, defs.roles(), persona, session().as_deref())?; - let report = facade::prime_for( + let report = facade::prime_with( &store, &consumer, identity.persona(), &resolve_now()?, - &declarations, + &defs, )?; let interactive = !is_spawned_context(); if json { diff --git a/crates/chat/src/definitions.rs b/crates/chat/src/definitions.rs index f5cf3c8..1695cac 100644 --- a/crates/chat/src/definitions.rs +++ b/crates/chat/src/definitions.rs @@ -153,15 +153,84 @@ impl DeclarationOrigin { /// The USER-LEVEL declaration folder this process reads beside every workspace's own (nxf /// 6j6v.7k58): `/personas` — `~/.nexusflow/personas` for the installed suite and for /// an embedding app, `~/.nexusflow-/personas` for a development build named so by -/// `NXS_SERVICE_INSTANCE` ([`nxs_service::ServiceHome::personas`]). +/// `NXS_SERVICE_INSTANCE` ([`nxs_service::ServiceHome::personas`]). `None` when there is none to +/// read — never an error. /// /// **One rule, the service's, and no knob.** An embedding app does not choose this location, and /// must not be able to: the personas it starts run the `nxc` CLI, which resolves the folder by this /// same rule, and an app reading a folder of its own would disagree with the very sessions it -/// spawned about who exists. Following the service home is also what keeps a build under test off -/// the personas the machine's installed suite runs with. -pub fn user_declarations_dir() -> Result { - Ok(nxs_service::ServiceHome::resolve()?.personas()) +/// spawned about who exists. +/// +/// **A build never reads the production folder** (review of PR #17, Code Quality #2 / Integrity +/// #3). The service rule sends a build that names no instance to the production home, which is +/// right for a registry nobody fills by hand and wrong here: a test binary is such a build, and +/// `cargo test` run outside `direnv` — CI, another shell — would read the owner's real personas +/// into every suite that resolves a catalogue. So a binary inside a build directory reads a +/// user-level folder only under a NAMED development instance (`~/.nexusflow-/personas`), +/// and the installed suite — the one thing that has a production folder to read — is untouched. A +/// suite that tests this folder pins the home and then names a development instance. +/// +/// **The environment cannot fail a catalogue** (review of PR #17, Code Quality #3 / Integrity #4). +/// No home directory, or an instance name that does not validate, means no user-level folder — the +/// state of every machine before this folder existed — rather than an error on every read. +pub fn user_declarations_dir() -> Option { + let instance = nxs_service::Instance::ambient().ok()?; + if instance.is_production() && nxs_service::Origin::ambient() == nxs_service::Origin::Build { + return None; + } + nxs_service::ServiceHome::for_instance(instance) + .ok() + .map(|home| home.personas()) +} + +/// The declared CHANNELS of a workspace, merged with the user-level ones — the read for a verb that +/// needs only the channel policy (`threads show`, `search`, `Engine::thread`, `Engine::search`), so +/// that it depends on the two `channels.yaml` files and not on every persona file besides (review +/// of PR #17, Code Quality #1: a malformed persona file anywhere must not make a thread unreadable). +/// Same rule as [`Definitions::resolve`]: a workspace channel hides the user-level one of its name. +pub fn merged_channels(root: &Path) -> Result> { + merged_channels_with_user_dir(root, user_declarations_dir().as_deref()) +} + +/// [`merged_channels`] against a NAMED user-level folder, or none — the test form, as +/// [`Definitions::resolve_with_user_dir`] is for the whole catalogue. +pub fn merged_channels_with_user_dir( + root: &Path, + user_dir: Option<&Path>, +) -> Result> { + let source = DeclarationSource::locate(root); + let mut channels = crate::channel::load_all_channels(&source.read_dir)?; + reject_duplicates_in(&source.read_dir, &[], &channels)?; + if let Some(user_dir) = user_folder_carrying_declarations(user_dir)? { + let user_channels = crate::channel::load_all_channels(&user_dir)?; + reject_duplicates_in(&user_dir, &[], &user_channels)?; + for channel in user_channels { + if !channels.iter().any(|c| c.name == channel.name) { + channels.push(channel); + } + } + } + Ok(channels) +} + +/// `dir` when it carries a declaration, `None` when it does not exist or carries none — and an +/// ERROR when it exists but cannot be read (review of PR #17, Integrity #5): an unreadable +/// user-level folder would otherwise make its personas vanish from every workspace without a word, +/// which is the fail-open the rest of this module refuses. +fn user_folder_carrying_declarations(dir: Option<&Path>) -> Result> { + let Some(dir) = dir else { + return Ok(None); + }; + if !dir.exists() { + return Ok(None); + } + std::fs::read_dir(dir).map_err(|e| { + NxfError::io(format!( + "reading the user-level declaration folder {}: {e}", + dir.display() + )) + })?; + Ok(carries_declarations(dir).then(|| dir.to_path_buf())) } /// The three answers to "where did this catalogue come from". @@ -552,7 +621,7 @@ impl Definitions { /// 2026-08-14; pinned on the app side by `engine-bridge/src/chat.rs` and /// `engine-server/tests/chat_wire.rs`). pub fn resolve(root: &Path) -> Result { - Definitions::resolve_with_user_dir(root, Some(&user_declarations_dir()?)) + Definitions::resolve_with_user_dir(root, user_declarations_dir().as_deref()) } /// [`resolve`](Definitions::resolve) against a NAMED user-level folder, or none — the form for @@ -561,9 +630,7 @@ impl Definitions { /// no seam lets an app pick another. pub fn resolve_with_user_dir(root: &Path, user_dir: Option<&Path>) -> Result { let mut source = DeclarationSource::locate(root); - source.user_path = user_dir - .filter(|dir| carries_declarations(dir)) - .map(Path::to_path_buf); + source.user_path = user_folder_carrying_declarations(user_dir)?; Definitions::load(source) } diff --git a/crates/chat/src/engine.rs b/crates/chat/src/engine.rs index 7e55a81..c44de37 100644 --- a/crates/chat/src/engine.rs +++ b/crates/chat/src/engine.rs @@ -87,7 +87,8 @@ use std::time::Duration; /// **Declarations are NOT on this config, and that is the point** (nxf 6j6v.dvyq step 6). They come /// from the workspace's own `.nxs-personas/` folder, merged per name with the user-level folder /// (nxf 6j6v.7k58, [`crate::definitions::user_declarations_dir`] — not configurable here either, -/// for the reason given there), and are re-read on every verb that needs them — exactly as a fresh `nxc` process re-reads the folder per invocation. There is no +/// for the reason given there), and are re-read on every verb that needs +/// them — exactly as a fresh `nxc` process re-reads the folder per invocation. There is no /// injection path any more: `DefinitionSource::Supplied` and `Engine::set_definitions` are gone, /// because personas and channels live in the folder even for an app, and a later in-app role editor /// writes into that same folder rather than into an app-owned database (owner decision, 2026-08-14, @@ -425,10 +426,11 @@ impl Engine { /// the filter is the one that went. `AllMembers` remains the honest fallback for a thread whose /// channel no declaration names. pub fn thread(&self, thread_id: &str, as_handle: &str) -> Result { - // The MERGED catalogue's channels (nxf 6j6v.7k58), read before the store lock is taken. - let defs = self.definitions()?; + // The merged CHANNELS (nxf 6j6v.7k58) — channels only, so a malformed persona file cannot + // make a thread unreadable — read before the store lock is taken. + let channels = crate::definitions::merged_channels(self.workspace().workspace_root()?)?; self.handle.try_with_state(|s| { - let policy = declared_policy_of(&s.store, defs.channels(), thread_id)?; + let policy = declared_policy_of(&s.store, &channels, thread_id)?; facade::thread(&s.store, thread_id, as_handle, policy) }) } @@ -476,9 +478,9 @@ impl Engine { /// rule and what it costs; `AllMembers` remains the honest fallback for a channel no /// declaration names, so an undeclared workspace reads exactly as it always did. pub fn search(&self, handle: &str, query: &str) -> Result> { - let defs = self.definitions()?; + let channels = crate::definitions::merged_channels(self.workspace().workspace_root()?)?; self.handle - .try_with_state(|s| facade::search(&s.store, handle, query, defs.channels())) + .try_with_state(|s| facade::search(&s.store, handle, query, &channels)) } /// **Where does this operation stand** (nxf 6j6v.a71h) — the thread TREE an operation is, across @@ -544,14 +546,12 @@ impl Engine { persona: Option<&str>, now: &str, ) -> Result { - let source = - // `resolve`, not `locate`: the report carries the declaration COUNT, and only loading - // the catalogue can answer that. `locate` leaves it at 0, which reads as "nothing is - // declared" on a workspace that declares plenty — a lie the parity differential caught - // against `nxc prime`, which had always taken the loading path. - crate::definitions::DeclarationSource::resolve(self.workspace().workspace_root()?)?; + // The whole catalogue, resolved ONCE: the report carries the declaration COUNT (which + // only loading answers — `locate` alone said 0, a lie the parity differential caught + // against `nxc prime`), and the merged team its source describes (nxf 6j6v.7k58). + let defs = self.definitions()?; self.handle - .try_with_state(|s| facade::prime_for(&s.store, consumer, persona, now, &source)) + .try_with_state(|s| facade::prime_with(&s.store, consumer, persona, now, &defs)) } /// **What a spawned persona is handed at its session start** (nxf 6j6v.k8zq) — the composed diff --git a/crates/chat/src/facade.rs b/crates/chat/src/facade.rs index b0e1735..aad0b08 100644 --- a/crates/chat/src/facade.rs +++ b/crates/chat/src/facade.rs @@ -2282,6 +2282,11 @@ pub struct PrimeRoster { /// /// It carried a third partition, the DECLARED WORKFLOWS, until 6j6v.dvyq §3 removed the run engine. /// A `workflow.yaml` left in the folder is now simply not read by anything. +/// +/// **ONE folder, and no longer what `prime` partitions** (nxf 6j6v.7k58): since the catalogue is +/// the workspace's folder merged with the user-level one, `prime` partitions the merged team +/// through [`validate_declared`]. This stays for a caller that asks about one folder on purpose, +/// and is kept rather than removed because it is public — `tests/prime_validation.rs` pins it. pub fn validate_declared_team(dir: &Path) -> Result { let roles = crate::role::load_all_roles(dir)?; let channels = crate::channel::load_all_channels(dir)?; @@ -3066,7 +3071,7 @@ fn render_finding(file: &str, what: &str) -> String { /// fan-out. (It replayed the consumer's entire unread set as well — both dispositions, because a new /// session was the catch-up moment — until nxf 6j6v.4d2z removed the unread set.) /// -/// Every read goes through this layer's own seams ([`opener_wake`], [`validate_declared_team`]), +/// Every read goes through this layer's own seams ([`opener_wake`], [`validate_declared`]), /// never the store directly: the CLI used to reach past the facade for exactly these, which is why /// none of it was reachable from the library. `now` drives only /// the wake's clock-dependent `stale` flag; `declarations` is the resolved declaration source (the @@ -3102,13 +3107,33 @@ pub fn prime_for( // is the reducer, which refuses to fold an envelope whose disposition it does not know // (`an_unknown_disposition_op_is_stored_but_never_folded`). // - // The declarations are read a second time here (`validate_declared_team` loads its own copy to - // partition them): a handful of small YAML files, once per prime, in exchange for leaving that - // function's roster contract — and every caller of it — untouched. // // Both folders, merged by the one rule (nxf 6j6v.7k58): `from_source` re-reads the workspace's - // folder and the user-level one the resolution names. - let defs = crate::definitions::Definitions::from_source(declarations)?; + // folder and the user-level one the resolution names. A caller that already holds the merged + // catalogue hands it to [`prime_with`] instead, so the report and its source come from ONE read. + prime_with( + store, + consumer, + persona, + now, + &crate::definitions::Definitions::from_source(declarations)?, + ) +} + +/// [`prime_for`] over a catalogue already resolved — `nxc prime` and +/// [`crate::engine::Engine::prime_as`] resolve once and pass it, so the directory, the roster, the brief and the source the report +/// carries cannot come from two different reads of a folder that changed in between (review of PR +/// #17, Code Quality #5). +pub(crate) fn prime_with( + store: &ChatStore, + consumer: &str, + persona: Option<&str>, + now: &str, + defs: &crate::definitions::Definitions, +) -> Result { + let declarations = defs + .source() + .expect("a resolved catalogue records its source"); let (roles, channels) = (defs.roles(), defs.channels()); let brief = persona .and_then(|handle| roles.iter().find(|r| r.handle == handle)) diff --git a/crates/chat/tests/a_persona_commissions_across_the_border.rs b/crates/chat/tests/a_persona_commissions_across_the_border.rs index 7e5a060..b3b17f8 100644 --- a/crates/chat/tests/a_persona_commissions_across_the_border.rs +++ b/crates/chat/tests/a_persona_commissions_across_the_border.rs @@ -870,11 +870,7 @@ fn an_answer_from_a_workspace_no_longer_trusted_cancels_it_with_that_reason() { let beta_pm = w.worker.of(&b, "pm")[0].internal_session.clone(); // The owner of alpha stops trusting beta while the thread is open. - { - let ws = nxs_foundation::workspace::discover(&a.root).unwrap(); - let mut store = ws.open_chat_store().unwrap(); - assert!(store.distrust_key(&key_of(&b.root)).unwrap()); - } + distrust(&a.root, &b.root); b.reply(&beta_pm, &receipt.thread_id, "With version 0.300.", false); let out = a.border_rows(); @@ -904,6 +900,61 @@ fn an_answer_from_a_workspace_no_longer_trusted_cancels_it_with_that_reason() { assert!(row.outstanding.is_empty(), "{row:?}"); } +fn distrust(root: &Path, other: &Path) { + let ws = nxs_foundation::workspace::discover(root).unwrap(); + let mut store = ws.open_chat_store().unwrap(); + assert!(store.distrust_key(&key_of(other)).unwrap()); +} + +/// The negative control of the one above: revoking trust is not itself a reason to cancel. With +/// nothing back from the receiver yet, the thread keeps waiting (its deadline still applies). +#[test] +fn revoking_trust_before_any_answer_leaves_the_commission_working() { + let tmp = TempDir::new().unwrap(); + let (a, _b, w) = two(&tmp, &[ALPHA_PM], &[BETA_PM], true, true); + let (alpha_pm, _) = alpha_session(&a, &w, "pm"); + a.as_session(&alpha_pm, "test/beta/pm", "When?").unwrap(); + distrust(&a.root, &tmp.path().join("beta")); + a.engine + .handover(Caller { + session: None, + actor: Some("handover"), + now: Some(NOW), + }) + .unwrap(); + assert_eq!(a.border_rows()[0].state, BorderState::Working); +} + +/// The second control (review of PR #17, Integrity #7): a question that was vouched for and +/// DELIVERED before trust was revoked is not "an answer from an untrusted workspace" — the +/// commissioner has it and is working on it. Revoking afterwards must not cancel on it. +#[test] +fn a_question_delivered_before_trust_was_revoked_does_not_cancel_the_commission() { + let tmp = TempDir::new().unwrap(); + let (a, b, w) = two(&tmp, &[ALPHA_PM], &[BETA_PM], true, true); + let (alpha_pm, _) = alpha_session(&a, &w, "pm"); + let receipt = a.as_session(&alpha_pm, "test/beta/pm", "When?").unwrap(); + let beta_pm = w.worker.of(&b, "pm")[0].internal_session.clone(); + b.reply(&beta_pm, &receipt.thread_id, "Which version line?", true); + let woken = w.worker.of(&a, "pm").last().unwrap().clone(); + assert!( + woken.message.contains("Which version line?"), + "{}", + woken.message + ); + + distrust(&a.root, &b.root); + a.engine + .handover(Caller { + session: None, + actor: Some("handover"), + now: Some(NOW), + }) + .unwrap(); + let out = a.border_rows(); + assert_ne!(out[0].state, BorderState::Canceled, "{out:?}"); +} + // ---- an answer for a commissioner still in its turn ------------------------------------------ #[test] diff --git a/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs index 903b786..d45aa1a 100644 --- a/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs +++ b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs @@ -11,6 +11,7 @@ use std::sync::{Arc, Mutex}; use nexus_chat::engine::{Engine, EngineConfig}; use nexus_chat::orchestration::Caller; +use nexus_chat::precondition::PreconditionOutcome; use nexus_chat::surface::{ReplyThreadRequest, SendToRefs, SendToRequest}; use nexus_chat::timer::TimerConfig; use nexus_chat::worker::{TriggerOutcome, TriggerRequest, TriggerResult, Worker, WorkerConfig}; @@ -19,14 +20,24 @@ use tempfile::TempDir; const NOW: &str = "2026-10-04T10:00:00Z"; +/// Records every start and every hurdle it is asked to run; every hurdle passes. #[derive(Default)] -struct RecordingWorker(Mutex>); +struct RecordingWorker(Mutex>, Mutex>); impl Worker for RecordingWorker { fn trigger(&self, req: TriggerRequest) -> TriggerResult { self.0.lock().unwrap().push(req); Ok(TriggerOutcome::Accepted) } + + fn run_precondition(&self, command: &str) -> PreconditionOutcome { + self.1.lock().unwrap().push(command.to_string()); + PreconditionOutcome::Ran { + status: Some(0), + stdout: String::new(), + stderr: String::new(), + } + } } impl RecordingWorker { @@ -41,6 +52,8 @@ impl RecordingWorker { } } +/// Both personas AND the channel's hurdle carry the version, so an edit changes a prompt and a +/// piece of executable text from the user-level folder at once (review of PR #17, Code Quality #6). fn declare_version(user: &std::path::Path, version: &str) { for handle in ["coder", "finisher"] { std::fs::write( @@ -49,6 +62,14 @@ fn declare_version(user: &std::path::Path, version: &str) { ) .unwrap(); } + std::fs::write( + user.join("channels.yaml"), + format!( + "- name: coding\n members: [coder, finisher]\n flow: sequential\n preconditions:\n \ + - name: gate\n run: hurdle-{version}\n" + ), + ) + .unwrap(); } fn open(engine: &Engine) -> String { @@ -78,14 +99,11 @@ fn an_edit_to_the_user_level_folder_reaches_the_next_operation_and_not_the_runni for (key, value) in nxs_test_support::pinned_home_env() { std::env::set_var(key, value); } - let user = nexus_chat::definitions::user_declarations_dir().unwrap(); + // A build reads a user-level folder only under a named development instance. + std::env::set_var("NXS_SERVICE_INSTANCE", "nexus-flow-test"); + let user = nexus_chat::definitions::user_declarations_dir().expect("a named instance's folder"); std::fs::create_dir_all(&user).unwrap(); declare_version(&user, "VERSION-ONE"); - std::fs::write( - user.join("channels.yaml"), - "- name: coding\n members: [coder, finisher]\n flow: sequential\n", - ) - .unwrap(); // A repository that declares nothing of its own. let repo = TempDir::new().unwrap(); @@ -138,10 +156,27 @@ fn an_edit_to_the_user_level_folder_reaches_the_next_operation_and_not_the_runni finisher[0].contains("VERSION-ONE"), "the running operation keeps the version it opened with: {finisher:?}" ); + // …and so does the HURDLE before that step: executable text from the user-level folder is + // frozen with the rest. + let asked = worker.1.lock().unwrap().clone(); + assert!( + !asked.iter().any(|c| c == "hurdle-VERSION-TWO"), + "the edited hurdle reached the running operation: {asked:?}" + ); + assert!( + asked.len() >= 2, + "the hurdle was asked before each step: {asked:?}" + ); + assert!(asked.iter().all(|c| c == "hurdle-VERSION-ONE"), "{asked:?}"); // The next operation takes the edit. open(&engine); let coders = worker.prompts_of("coder"); assert_eq!(coders.len(), 2); assert!(coders[1].contains("VERSION-TWO"), "{coders:?}"); + assert_eq!( + worker.1.lock().unwrap().last().map(String::as_str), + Some("hurdle-VERSION-TWO"), + "the next operation asks the edited hurdle" + ); } diff --git a/crates/chat/tests/one_definition_at_the_engine_seam.rs b/crates/chat/tests/one_definition_at_the_engine_seam.rs index e00a6a0..50cd8e4 100644 --- a/crates/chat/tests/one_definition_at_the_engine_seam.rs +++ b/crates/chat/tests/one_definition_at_the_engine_seam.rs @@ -19,10 +19,38 @@ fn the_engine_reads_the_user_level_folder_the_cli_reads_and_says_where_each_entr std::env::set_var(key, value); } let home = nxs_test_support::pinned_home(); - // Whichever instance the ambient environment names, the folder is that instance's — resolved - // the way production resolves it, not spelled out here. - let user = nexus_chat::definitions::user_declarations_dir().unwrap(); + + // **The guard** (review of PR #17, Code Quality #2 / Integrity #3): this test binary is a + // BUILD, and under the production instance — what `cargo test` outside `direnv` resolves — + // a build reads no user-level folder at all, even with personas lying in the production one. + let production = home.join(".nexusflow/personas"); + std::fs::create_dir_all(&production).unwrap(); + std::fs::write( + production.join("intruder.yaml"), + "handle: intruder\nsystem_prompt: The owner's real persona.\n", + ) + .unwrap(); + assert_eq!(nexus_chat::definitions::user_declarations_dir(), None); + let repo = TempDir::new().unwrap(); + nexus_chat::workspace::setup(repo.path(), &nexus_chat::workspace::chat_config()).unwrap(); + let engine = nexus_chat::engine::Engine::open(None, repo.path()).unwrap(); + let defs = engine.definitions().unwrap(); + assert!( + defs.role("intruder").is_err(), + "a build read the production folder" + ); + assert_eq!(defs.source().unwrap().user_path, None); + + // A NAMED development instance reads its own folder — resolved the way production resolves + // it, not spelled out here. + std::env::set_var("NXS_SERVICE_INSTANCE", "nexus-flow-test"); + let user = nexus_chat::definitions::user_declarations_dir().expect("a named instance's folder"); assert!(user.starts_with(home), "{}", user.display()); + assert!( + user.ends_with(".nexusflow-test/personas"), + "{}", + user.display() + ); std::fs::create_dir_all(&user).unwrap(); std::fs::write( user.join("pm.yaml"), @@ -35,10 +63,6 @@ fn the_engine_reads_the_user_level_folder_the_cli_reads_and_says_where_each_entr ) .unwrap(); - let repo = TempDir::new().unwrap(); - nexus_chat::workspace::setup(repo.path(), &nexus_chat::workspace::chat_config()).unwrap(); - let engine = nexus_chat::engine::Engine::open(None, repo.path()).unwrap(); - // `definitions`: the merged catalogue, with its account. let defs = engine.definitions().unwrap(); assert_eq!(defs.role("pm").unwrap().system_prompt, "The central PM."); diff --git a/crates/chat/tests/one_definition_many_repositories.rs b/crates/chat/tests/one_definition_many_repositories.rs index 8848368..e32fda6 100644 --- a/crates/chat/tests/one_definition_many_repositories.rs +++ b/crates/chat/tests/one_definition_many_repositories.rs @@ -235,3 +235,99 @@ fn from_source_reads_the_same_merged_catalogue_again() { let again = Definitions::from_source(defs.source().unwrap()).unwrap(); assert_eq!(defs, again); } + +// ---- the user-level loader's failing cases (review of PR #17, Test Quality #2) ---------------- + +#[test] +fn a_malformed_user_level_channels_file_fails_the_catalogue_with_its_path() { + let m = Machine::new(); + declare( + m.user.path(), + "channels.yaml", + "- name: planning\n members: pm\n", + ); + let err = m.resolve().unwrap_err(); + assert!( + err.msg + .contains(&m.user.path().join("channels.yaml").display().to_string()), + "{}", + err.msg + ); +} + +#[test] +fn two_user_level_channels_of_one_name_are_refused_naming_that_folder() { + let m = Machine::new(); + declare( + m.user.path(), + "channels.yaml", + "- name: planning\n members: [pm]\n- name: planning\n members: [pm]\n", + ); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + let err = m.resolve().unwrap_err(); + assert!( + err.msg.contains(&m.user.path().display().to_string()) + && err.msg.contains("duplicate channel name"), + "{}", + err.msg + ); +} + +/// Referential integrity stays advisory for the merged team exactly as for one folder: a user-level +/// channel naming a persona nobody declares still loads (`prime` reports it; a send to it refuses). +#[test] +fn a_user_level_channel_naming_a_persona_nobody_declares_still_loads() { + let m = Machine::new(); + declare(m.user.path(), "channels.yaml", PLANNING); + let defs = m.resolve().expect("advisory, not a construction error"); + assert!(defs.declared_channel("planning").unwrap().is_some()); + assert!(defs.role("coder").is_err()); +} + +#[test] +fn a_user_level_folder_that_does_not_exist_is_no_user_level_folder() { + let m = Machine::new(); + declare(&m.repo_personas(), "pm.yaml", REPO_PM); + let missing = m.user.path().join("not-there"); + let with_missing = Definitions::resolve_with_user_dir(m.repo.path(), Some(&missing)).unwrap(); + let without = Definitions::resolve_with_user_dir(m.repo.path(), None).unwrap(); + assert_eq!(with_missing, without); +} + +#[cfg(unix)] +#[test] +fn an_unreadable_user_level_folder_is_an_error_not_a_silence() { + use std::os::unix::fs::PermissionsExt as _; + let m = Machine::new(); + declare(m.user.path(), "pm.yaml", CENTRAL_PM); + std::fs::set_permissions(m.user.path(), std::fs::Permissions::from_mode(0o000)).unwrap(); + let result = m.resolve(); + std::fs::set_permissions(m.user.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + // A process that may read anything (root in a container) reads it fine; only a refused read + // is the case under test. + if std::fs::metadata("/root").is_ok_and(|_| std::fs::read_dir("/root").is_ok()) { + return; + } + let err = result.unwrap_err(); + assert!( + err.msg.contains("user-level declaration folder"), + "{}", + err.msg + ); +} + +/// The read `threads show` and `search` resolve their channel policy through depends on the two +/// `channels.yaml` files only (review of PR #17, Code Quality #1): a malformed PERSONA file in the +/// user-level folder fails the catalogue, and must not make a thread unreadable. +#[test] +fn the_channel_policy_read_does_not_fail_on_a_malformed_persona_file() { + let m = Machine::new(); + declare(m.user.path(), "channels.yaml", PLANNING); + declare(m.user.path(), "pm.yaml", "handle: [not, a, handle]\n"); + assert!(m.resolve().is_err()); + let channels = + nexus_chat::definitions::merged_channels_with_user_dir(m.repo.path(), Some(m.user.path())) + .expect("channels only"); + assert_eq!(channels.len(), 1); + assert_eq!(channels[0].name, "planning"); +} diff --git a/crates/nxs/src/peers.rs b/crates/nxs/src/peers.rs index 0b5f65c..fa5f3bf 100644 --- a/crates/nxs/src/peers.rs +++ b/crates/nxs/src/peers.rs @@ -87,6 +87,12 @@ impl Peers for ServicePeers { { use std::os::unix::process::CommandExt as _; cmd.arg0("nxs"); + // A group of its own, so a handover past its limit is stopped WITH whatever it started + // and still waits on (review of PR #17, Integrity #6). The persona session a handover + // starts is not in it: the worker puts every sidecar into a group of ITS own + // (`process_group(0)` in `crates/chat/src/worker.rs`), so it outlives this one, as it + // must. + cmd.process_group(0); } cmd.args(["chat", "handover"]) .current_dir(&peer.root) @@ -108,10 +114,18 @@ impl Peers for ServicePeers { /// It used to be left running with "it goes on by itself", and its child was then reaped by nobody /// until the calling process exited. Harmless for a short-lived CLI; the background service is /// long-lived, and it runs a border pass every few seconds, so one wedged handover per pass would -/// pile up processes for as long as the service runs. A handover only copies a thread and starts a -/// persona session it does not wait for, so one past the limit is stuck rather than busy: killing it -/// rolls back at most one SQLite transaction, and the next pass hands over whatever it had not. -/// Killed AND waited for, so no zombie is left behind either. +/// pile up processes for as long as the service runs. +/// +/// **Stopping it is safe to repeat.** A handover copies a thread and starts a persona session it +/// does not wait for. Killing it rolls back at most one SQLite transaction, and the next pass does +/// what it had not — without starting a persona twice, because admission is claimed in the +/// database before the start (`UPDATE … WHERE state='submitted'` in `crate::border`), so a +/// commission whose persona was already started is no longer `submitted` (review of PR #17, Code +/// Quality #7). +/// +/// The whole process GROUP is signalled, then the child is reaped — polled, not waited on blindly, +/// so a child that somehow survives the signal cannot hang the caller (review of PR #17, Integrity +/// #6). The cost that stays: a pass that meets a stuck peer still spends `limit` on it. fn wait_or_stop( child: &mut std::process::Child, limit: Duration, @@ -124,12 +138,16 @@ fn wait_or_stop( Ok(Some(status)) => return Err(format!("the handover in {name} exited with {status}")), Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(50)), Ok(None) => { - let _ = child.kill(); - let _ = child.wait(); + stop_group(child); + let reaped = reap_within(child, REAP_WAIT); return Err(format!( - "the handover in {name} was still running after {}s and was stopped; the \ + "the handover in {name} was still running after {}s and was stopped{}; the \ service's next pass hands over what it had not", - limit.as_secs() + limit.as_secs(), + match reaped { + true => "", + false => " (it did not exit on the signal)", + } )); } Err(e) => return Err(format!("waiting for the handover in {name}: {e}")), @@ -137,6 +155,36 @@ fn wait_or_stop( } } +/// How long a stopped handover is given to be reaped. +const REAP_WAIT: Duration = Duration::from_secs(5); + +/// Signal the child's whole process group; the child alone where there are no groups, or where the +/// group signal failed. +fn stop_group(child: &mut std::process::Child) { + #[cfg(unix)] + { + // SAFETY: `kill` with a negative pid signals that process group; the id is our own child's, + // which `process_group(0)` made the leader of its group. + let pgid = child.id() as libc::pid_t; + if unsafe { libc::kill(-pgid, libc::SIGKILL) } == 0 { + return; + } + } + let _ = child.kill(); +} + +/// Reap `child`, polling for at most `within`. `false` when it is still there. +fn reap_within(child: &mut std::process::Child, within: Duration) -> bool { + let until = Instant::now() + within; + loop { + match child.try_wait() { + Ok(Some(_)) | Err(_) => return true, + Ok(None) if Instant::now() < until => std::thread::sleep(Duration::from_millis(20)), + Ok(None) => return false, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -157,34 +205,56 @@ mod tests { #[cfg(unix)] #[test] - fn a_handover_past_its_limit_is_stopped_and_reaped_not_left_running() { - // nxf 6j6v.dcpd, item 3: the long-lived service must not collect wedged children. - let mut child = std::process::Command::new("sleep") - .arg("30") + fn a_handover_past_its_limit_is_stopped_with_its_group_and_reaped() { + // nxf 6j6v.dcpd, item 3, and the review of PR #17 (Integrity #6): the long-lived service + // must not collect wedged children, nor the grandchildren they started. + use std::io::BufRead as _; + use std::os::unix::process::CommandExt as _; + let mut child = std::process::Command::new("sh") + .args(["-c", "sleep 30 & echo $!; wait"]) + .stdout(std::process::Stdio::piped()) + .process_group(0) .spawn() .unwrap(); + let mut line = String::new(); + std::io::BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut line) + .unwrap(); + let grandchild = line.trim().to_string(); let pid = child.id(); - let started = Instant::now(); let err = wait_or_stop(&mut child, Duration::from_millis(200), "test/stuck") .expect_err("past the limit"); assert!( err.contains("test/stuck") && err.contains("stopped"), "{err}" ); + assert!(!err.contains("did not exit"), "{err}"); + let alive = |pid: &str| { + std::process::Command::new("kill") + .args(["-0", pid]) + .stderr(std::process::Stdio::null()) + .status() + .unwrap() + .success() + }; + // Reaped: the child's pid is gone, not a zombie still answering `kill -0`. + assert!(!alive(&pid.to_string()), "the child {pid} was left behind"); + // The grandchild got the signal too; once it is not ours to reap, init reaps it, so give + // that a moment by polling rather than by asserting on the clock. + let gone = (0..100).any(|_| { + let gone = !alive(&grandchild); + if !gone { + std::thread::sleep(Duration::from_millis(20)); + } + gone + }); assert!( - started.elapsed() < Duration::from_secs(10), - "it did not wait for sleep" + gone, + "the grandchild {grandchild} survived its group's stop" ); - // Reaped: the pid is gone, not a zombie still answering `kill -0`. - let alive = std::process::Command::new("kill") - .args(["-0", &pid.to_string()]) - .stderr(std::process::Stdio::null()) - .status() - .unwrap() - .success(); - assert!(!alive, "the child {pid} was left behind"); } + #[cfg(unix)] #[test] fn a_handover_that_finishes_in_time_is_ok() { let mut child = std::process::Command::new("true").spawn().unwrap(); diff --git a/crates/nxs/src/prime.rs b/crates/nxs/src/prime.rs index 01de619..7ea260a 100644 --- a/crates/nxs/src/prime.rs +++ b/crates/nxs/src/prime.rs @@ -363,8 +363,12 @@ where /// ([`nexus_chat::role::compose_system_prompt`]), so it survives that regardless. pub fn persona_block(ws: &Workspace, handle: &str) -> Result { let root = ws.workspace_root()?.to_path_buf(); - let source = nexus_chat::definitions::DeclarationSource::resolve(&root)?; + // ONE read: the source handed on below describes exactly the catalogue `decl` came from. let defs = nexus_chat::definitions::Definitions::resolve(&root)?; + let source = defs + .source() + .cloned() + .expect("Definitions::resolve always records its source"); let decl = defs.role(handle)?; if !decl.prime.is_primed() { return Ok(String::new()); diff --git a/crates/nxs/tests/two_workspaces_on_one_machine.rs b/crates/nxs/tests/two_workspaces_on_one_machine.rs index ad682d3..e5e52a3 100644 --- a/crates/nxs/tests/two_workspaces_on_one_machine.rs +++ b/crates/nxs/tests/two_workspaces_on_one_machine.rs @@ -14,6 +14,9 @@ use nxs_test_support::PinHome; use serde_json::Value; use tempfile::TempDir; +/// The development instance these tests run under — named so a build reads a user-level folder. +const INSTANCE: &str = "nexus-flow-two-workspaces"; + struct Machine { home: TempDir, _dirs: TempDir, @@ -22,9 +25,9 @@ struct Machine { } impl Machine { - /// The user-level declaration folder of the pinned home's (production) service instance. + /// The user-level declaration folder of the test's service instance under the pinned home. fn user_personas(&self) -> PathBuf { - self.home.path().join(".nexusflow/personas") + self.home.path().join(".nexusflow-two-workspaces/personas") } fn dry_log(&self) -> PathBuf { @@ -40,6 +43,9 @@ impl Machine { let mut c = Command::new(assert_cmd::cargo::cargo_bin("nxs")); c.current_dir(dir) .pin_home(self.home.path()) + // After the home, as the pinning rule asks: a build reads a user-level declaration + // folder only under a NAMED development instance (nxf 6j6v.7k58), so the test names one. + .env("NXS_SERVICE_INSTANCE", INSTANCE) .env("NXC_WORKER", "dry") .env("NXC_DRY_LOG", self.dry_log()) .env("NXC_TIMER", "dry") @@ -104,8 +110,7 @@ fn machine() -> Machine { assert!(ok, "init: {err}"); } // ONE pm for both repositories, from the user-level folder (nxf 6j6v.7k58): neither repository - // declares it. The pinned home is the production instance's, so the folder is - // `/.nexusflow/personas`. + // declares it. The folder is the test instance's, `/.nexusflow-two-workspaces/personas`. std::fs::create_dir_all(m.user_personas()).unwrap(); std::fs::write( m.user_personas().join("pm.yaml"), @@ -325,6 +330,57 @@ fn both_repositories_run_the_user_level_pm_and_a_repository_file_hides_it_out_lo assert!(list["declarations"].get("shadowed").is_none(), "{list}"); } +/// A user-level `external` does not open every repository on the machine (review of PR #17, +/// Integrity #2): admission also needs the RECEIVING repository to trust the caller's workspace by +/// name, and that trust is per repository. Here beta runs the user-level pm, which admits `*/pm` +/// from outside, but beta trusts nobody — so alpha's pm is refused, "not trusted", and nothing +/// starts in beta. +#[test] +fn a_user_level_external_admits_nobody_into_a_repository_that_trusts_nobody() { + let m = machine(); + let (ok, _, err) = m.run( + &m.alpha, + &["sync", "trust", "add", "--workspace", "test/beta"], + ); + assert!(ok, "{err}"); + let first = m.json( + &m.alpha, + &[ + "chat", + "--json", + "send", + "--to", + "pm", + "--no-ref", + "Ask test/beta.", + ], + ); + let session = first["session"].as_str().expect("a session").to_string(); + let out = m + .nxs(&m.alpha) + .env("NXC_SESSION", &session) + .args([ + "chat", + "--json", + "send", + "--to", + "test/beta/pm", + "--no-ref", + "When does it ship?", + ]) + .output() + .unwrap(); + let receipt: Value = serde_json::from_slice(&out.stdout) + .unwrap_or_else(|e| panic!("{e}: {}", String::from_utf8_lossy(&out.stderr))); + assert_eq!(receipt["border"]["state"], "rejected", "{receipt}"); + assert_eq!(receipt["border"]["reason"], "not trusted", "{receipt}"); + assert!( + !m.starts().contains("coordinator=border"), + "nothing started in beta: {}", + m.starts() + ); +} + /// **The scenario with real sessions** (nxf 6j6v.70dy, section 1 of the slice's specification) — /// the PM of one repository asks the PM of another and passes the answer on, through the real /// binary, the real sidecar and a real model. What the routing tests above cannot show: that a