diff --git a/changes/7k58-one-definition-many-repos.md b/changes/7k58-one-definition-many-repos.md new file mode 100644 index 0000000..291d220 --- /dev/null +++ b/changes/7k58-one-definition-many-repos.md @@ -0,0 +1,42 @@ +--- +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 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`, `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 +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 `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`, +`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/changes/dcpd-border-follow-ups.md b/changes/dcpd-border-follow-ups.md new file mode 100644 index 0000000..49a92b5 --- /dev/null +++ b/changes/dcpd-border-follow-ups.md @@ -0,0 +1,20 @@ +--- +type: fixed +facade: changed +--- +[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..dc19e06 100644 --- a/crates/chat/docs/guide/de/limits-and-safety.md +++ b/crates/chat/docs/guide/de/limits-and-safety.md @@ -63,6 +63,14 @@ 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/de/personas.md b/crates/chat/docs/guide/de/personas.md index 6678f18..ca9ae97 100644 --- a/crates/chat/docs/guide/de/personas.md +++ b/crates/chat/docs/guide/de/personas.md @@ -280,17 +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. `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. +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. `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 @@ -499,6 +503,61 @@ 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 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. + +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 `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 Eine fehlerhafte Datei ist ein lauter `validation`-Fehler, der den Pfad nennt — nie eine still diff --git a/crates/chat/docs/guide/en/limits-and-safety.md b/crates/chat/docs/guide/en/limits-and-safety.md index 9aff66c..6b222f7 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/docs/guide/en/personas.md b/crates/chat/docs/guide/en/personas.md index a86832f..3bb7a31 100644 --- a/crates/chat/docs/guide/en/personas.md +++ b/crates/chat/docs/guide/en/personas.md @@ -265,11 +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. `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: @@ -466,6 +469,57 @@ 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 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. + +**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. + +**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 A malformed file is a loud `validation` error naming the path — never a silently skipped persona. A diff --git a/crates/chat/src/border.rs b/crates/chat/src/border.rs index 4788144..4f96753 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,35 @@ fn act_outbound( Ok(()) } +/// Whether the receiver's NEWEST message on a border thread is one this workspace cannot vouch for +/// 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, + worker: &str, + acting: &[MessageRow], +) -> Result { + let newest = store + .messages_in_thread(thread)? + .into_iter() + .rev() + .find(|m| m.sender == worker); + 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 /// 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/src/cli.rs b/crates/chat/src/cli.rs index f73275f..88bb324 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,12 @@ 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 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> { + crate::definitions::merged_channels(&workspace_root(db)?) } /// Everything [`crate::orchestration::Ctx`] borrows, owned — the CLI adapter's job in one place. @@ -1881,19 +1882,15 @@ 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 report = facade::prime_for( + let defs = crate::definitions::Definitions::resolve(&workspace_root(db)?)?; + let identity = + crate::persona::resolve_identity(&store, defs.roles(), persona, session().as_deref())?; + let report = facade::prime_with( &store, &consumer, identity.persona(), &resolve_now()?, - &declarations, + &defs, )?; let interactive = !is_spawned_context(); if json { @@ -1915,13 +1912,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 +1951,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 +3915,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 +4006,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..1695cac 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,151 @@ 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`]). `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. +/// +/// **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". @@ -147,6 +309,9 @@ impl DeclarationSource { legacy_path, read_dir, count: 0, + user_path: None, + from_user: Vec::new(), + shadowed: Vec::new(), } } @@ -166,12 +331,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 +366,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 +621,83 @@ 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, user_declarations_dir().as_deref()) + } + + /// [`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_folder_carrying_declarations(user_dir)?; + 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 +866,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..c44de37 100644 --- a/crates/chat/src/engine.rs +++ b/crates/chat/src/engine.rs @@ -85,8 +85,10 @@ 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 +302,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 +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 { - let decl_dir = self.workspace().declaration_dir()?; + // 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, &decl_dir, thread_id)?; + let policy = declared_policy_of(&s.store, &channels, thread_id)?; facade::thread(&s.store, thread_id, as_handle, policy) }) } @@ -475,7 +478,7 @@ 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 channels = crate::definitions::merged_channels(self.workspace().workspace_root()?)?; self.handle .try_with_state(|s| facade::search(&s.store, handle, query, &channels)) } @@ -543,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 @@ -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..aad0b08 100644 --- a/crates/chat/src/facade.rs +++ b/crates/chat/src/facade.rs @@ -2282,10 +2282,26 @@ 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)?; - 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 +2316,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 +2971,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()), } } @@ -3051,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 @@ -3087,18 +3107,49 @@ 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. - 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. 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)) - .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 +3200,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_persona_commissions_across_the_border.rs b/crates/chat/tests/a_persona_commissions_across_the_border.rs index 4be38ea..b3b17f8 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,103 @@ 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. + distrust(&a.root, &b.root); + 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:?}"); +} + +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 new file mode 100644 index 0000000..d45aa1a --- /dev/null +++ b/crates/chat/tests/a_user_level_declaration_is_frozen_per_operation.rs @@ -0,0 +1,182 @@ +//! **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::precondition::PreconditionOutcome; +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"; + +/// Records every start and every hurdle it is asked to run; every hurdle passes. +#[derive(Default)] +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 { + 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() + } +} + +/// 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( + user.join(format!("{handle}.yaml")), + format!("handle: {handle}\nsystem_prompt: {version}\n"), + ) + .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 { + 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() { + // 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); + } + // 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"); + + // 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:?}" + ); + // …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 new file mode 100644 index 0000000..50cd8e4 --- /dev/null +++ b/crates/chat/tests/one_definition_at_the_engine_seam.rs @@ -0,0 +1,121 @@ +//! **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() { + // 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(); + + // **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"), + "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(); + + // `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..e32fda6 --- /dev/null +++ b/crates/chat/tests/one_definition_many_repositories.rs @@ -0,0 +1,333 @@ +//! **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); +} + +// ---- 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 2b95654..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) @@ -99,28 +105,82 @@ 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. +/// +/// **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, + 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) => { + 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 \ + service's next pass hands over what it had not", + 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}")), + } + } +} + +/// 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, } } } @@ -142,4 +202,65 @@ 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_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 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!( + gone, + "the grandchild {grandchild} survived its group's stop" + ); + } + + #[cfg(unix)] + #[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(()) + ); + } } 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 efe7ddd..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,6 +25,11 @@ struct Machine { } impl Machine { + /// 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-two-workspaces/personas") + } + fn dry_log(&self) -> PathBuf { self.home.path().join("starts.log") } @@ -35,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") @@ -98,16 +109,12 @@ 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(); - 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(); + // ONE pm for both repositories, from the user-level folder (nxf 6j6v.7k58): neither repository + // 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.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 +259,134 @@ 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}"); +} + +/// 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 /// 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 +430,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 +465,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)