diff --git a/CHANGELOG.md b/CHANGELOG.md index 6572298..429b50e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +- **fix(install): a failed Pi install exits nonzero, like a declined one.** The + script refused to call a DECLINED upgrade success — "a zero status would tell + a script that everything is ready when the thing that runs people is too old" + — and then reported a FAILED install as success anyway, through two `|| true` + sites. A caller got the happy outro and status 0 with the agent runtime + absent, which is strictly worse than the too-old Pi the decline path already + refuses. **The rationale proved more than the code implemented.** Both sites + now end in the same deliberate family as the decline, naming what did not + happen and the command that fixes it. + + **And the rule now has an instrument, which it never had.** Every Pi branch + was reachable only by running the real installer against the real network, so + nothing checked the exit code any of them produced and the rule shipped as a + comment. `installer-pi-exit-codes.test.mjs` runs the script's OWN bytes — + extracted at a marker, executed against stub `pi` and `npm` — over five + behaviours, and carries a control that restores `|| true` in a copy and + asserts the block goes green, so the passing assertions are known to measure + the defect rather than merely to pass. + +- **feat(install): the installer installs and upgrades Pi instead of printing a command and hoping.** An absent Pi is installed without asking — chief cannot run a single person without it, so there is nothing to weigh. A Pi that is merely too old is somebody's working tool, and replacing it is a different act, so that one asks: `Upgrade Pi to >= ? [Y/n]`, default yes. Declining exits nonzero and says why, because telling a script that everything is ready when the thing that runs people is too old is worse than failing. + **The floor is read, never restated.** It has one definition in the Rust source, the release process already stamps it into the manifest, and the installer reads it from the manifest it has just unpacked. A version bump needs no edit to the installer, and the repository's single-definition guard stays satisfied — a number copied into a shell script would be a second definition wearing a copy's clothes. + **The prompt reads the terminal, not its own source.** This file is piped into `sh`, so stdin is the script text; reading stdin would eat the rest of the installer. Where there is no terminal at all — CI, a container build — the default is taken and SAID, because a silent choice made on somebody's behalf is what surprises them later. Missing `npm` refuses cleanly rather than half-installing, and a Pi that npm reports as installed is CHECKED against the floor rather than announced as ready. + +- **feat(intercom): organization mail rides the same queue as the operator's own typing.** A busy person now reads an ordinary message at the next step boundary inside the running turn, instead of at the end of it. The operator typing mid-turn has always been submitted as a steering message and consumed within seconds; ordinary mail rode the follow-up queue, which Pi consumes only once the agent has no more tool calls or steering messages — so somebody an hour into a piece of work did not see a teammate's message until the hour was over, while the identical words typed by the operator arrived immediately. + **The digest is kept; only its consumption point moves.** Batching is still the answer to twenty messages arriving in one turn, and nothing about how a batch is built has changed. + **The idle and boot rows are byte-identical.** Mail arriving at somebody doing nothing still starts them; mail arriving inside the boot window is still parked. This changes WHEN a busy person reads a message, never whether an idle one is woken — and it touches nothing in the converge, activity or settle paths, so the operator wake lease is untouched by construction. + **The change was unpinnable as written, so it was made pinnable.** The delivery table was already covered, but which mode the mailbox passed into it was two string literals at two call sites — so the timing of every delivery in the product could change without one test noticing. It is one named decision now, and a test asserts the rule rather than the table. + +- **fix(hire): the guidance told agents to omit the department id; the schema required it; the model obeyed the schema and guessed.** `org_hire`'s `departmentId` was a REQUIRED field whose own description opened "DEFAULT: the department YOU head". An agent read that, reasoned correctly that it should omit the field, met a schema that would not permit it, and improvised the most salient name in context — the company's. It obeyed the instrument over the claim, which is the right thing for it to do. + **The field is optional now and the default is real.** An omitted id resolves to the department the caller heads, or failing that the one they sit in — the resolver that already existed and is word-for-word what the description promised. The promise was always implementable; it simply was not implemented. The worked example shows the omitted form first, because an example that keeps passing the field re-teaches the habit the description is trying to correct. + **The company name is still refused on the override path**, and that refusal is now the only thing standing behind an explicitly-passed id, so it is pinned by a test that passes the field deliberately rather than omitting it. Accepting the name as an alias was rejected again and for a sharper reason than before: with a default in place the alias would exist only on the override path, where a mid-level head naming the company means their OWN team — so it would resolve to the impressive-sounding wrong target rather than the semantically right one. + **A sweep asked the general question of every parameter in the file** — does the schema permit what the prose promises? — across `DEFAULT`, `omit`, `omitted`, `optional` and `leave empty`. Exactly one leaf field disagreed, this one. One other hit reads "never omit this field" and is required, so prose and schema agree. A mechanical guard was therefore not added: an instrument for a class of one is furniture. +- **feat(cards): a refusal says "refused"; a crash still says "failed".** The word is a claim about whose fault the failure was, and the two invite opposite recoveries — "refused" invites a corrected call, "failed" invites a retry. So `Hiring teammate refused` when the caller named a department that does not exist, and `Hiring teammate failed` when something actually broke. This is only implementable because the classification exists to follow; the verb reads it rather than matching on message text, which this file bans. + **The split follows the classification, so a refusal added next month gets the right word without anyone remembering to add it.** One predicate, in one place, because three renderers build a failure title and a rule copied three times is three rules waiting to disagree — two of them were bespoke titles that would otherwise have kept saying "failed" for a refusal. + **The `(system fault)` tag reads the same marker.** It measured only the ABSENCE of a status, so the verb moved to the fault marker and the tag stayed on the old instrument — a partial batch wrapping a real crash said "failed" correctly and then dropped the crash marker, leaving somebody debugging it to read a list of people already hired and reasonably conclude they had passed bad input. One classification, two surfaces, and a test that fails if either moves without the other. + **One case turned out not to be what the classification said it was.** A partial hire carries a status so the card can name the people already hired, and a retry does not double-hire them — but the error it wraps may be a genuine crash. A status carried for CONTEXT is not a claim about fault, so that path now asks the wrapped error's own type, and a producer in the same position marks itself rather than being enumerated in the renderer. Without it, a real crash mid-batch would have been labelled a refusal and invited a correction to a call that was never wrong — the earlier defect pointed the other way, and worse for it. + - **fix(install): the installer no longer prints a curl error on the first line a stranger runs.** A clean install printed `curl: (23) Failure writing output to destination` immediately under "Resolving the latest chief release…", and then completed successfully. The install was never broken; the message was, and it appeared on the very first command anyone runs against this project — where a reader has no reason to read it as anything but a failure. **The cause was a pipeline, not the request.** The release lookup piped curl into `grep -m1`, which exits on its first match; if curl is still writing when it does, curl's write fails and it says so on stderr. The tag had already been captured, which is why everything downstream worked. Whether it appeared at all depended on whether the response outran the pipe buffer, which is why it was intermittent rather than constant — and why it is the kind of thing that reaches a stranger before it reaches the people who built it. **The installer also puts chief on your PATH itself**, instead of printing a line to copy. It writes to `~/.bashrc` and `~/.zshrc` when they exist, creates the one matching your shell when neither does, and **names every file it touched** — a script that edits somebody's dotfiles and does not say which is asking to be distrusted. Running it again changes nothing: a profile that already exports that directory is recognised and left alone, matched on the PATH rather than on an exact line, so a hand-edited variant counts as done. diff --git a/DECISIONS.md b/DECISIONS.md index 1e728d3..30df4ab 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,3 +1,29 @@ +- 2026-08-28 — **An exit code is a claim about STATE, not about effort, so a + missing prerequisite may never exit zero.** install.sh already refused to call + a declined Pi upgrade success; it then reported a FAILED install as success + via `|| true`, leaving the agent runtime ABSENT — the worse state — while + telling every calling script that chief was ready. The defect class is the one + this branch keeps producing in new clothes: **the rationale proved more than + the code implemented**, and the carve-out's own words were the argument + against the line three branches away. So the rule is stated positively: every + path that ends without a usable prerequisite exits nonzero and names both what + did not happen and the command that repairs it, and no comment counts those + paths, because a count in a comment goes stale silently. Pinned by + `installer-pi-exit-codes.test.mjs`, which executes the installer's own + extracted bytes against stub `pi`/`npm` rather than asserting over its text, + REFUSES in words when its marker no longer locates the block, and carries a + control that reinstates `|| true` in a copy and requires the block to go green + — a pin nobody has flipped is a claim, not evidence. + +- 2026-08-28 — **A shell guard tests USABILITY, never EXISTENCE.** Three separate defects this week were one defect: the check asked whether a thing was THERE when the only question that matters is whether the next line can USE it. A profile file was selected by `[ -f ]` and then written to a path that was not writable; a link was accepted because it resolved to a name, and the name pointed at nothing; and `[ -r /dev/tty ]` answered true inside a container whose `/dev/tty` cannot actually be opened, so the prompt that followed killed the installer with status 2 and no message. The general form is that `-e`, `-f`, `-r` and `-w` report a directory entry and its permission bits, which are a claim about the FILESYSTEM, while the operation that follows is a claim about the KERNEL — and the two disagree for containers, dangling symlinks, read-only mounts, full disks and revoked terminals. So the guard performs the real operation and reads its status: open the file, follow the link, redirect the terminal. **And it performs it where a failure is survivable**, which is the second half of the rule and the half `/dev/tty` taught — `{ : < /dev/tty; }` is a redirect on a special built-in, and under `set -eu` a redirect error there is fatal to the whole shell, so the probe must run in a subshell, `( : < /dev/tty )`, where the status can be read instead of ending the script. A probe that cannot fail safely is not a probe, it is the bug it was written to prevent. + +- 2026-08-28 — **Install what is missing; ask before replacing what somebody already has.** The installer now handles Pi, and the asymmetry is the decision: an ABSENT prerequisite is installed with no prompt, because chief cannot run a person without it and there is nothing for the user to weigh; an EXISTING one that is merely below the floor belongs to them, and replacing a working tool without asking is a different act, so it prompts with a default of yes. Declining exits NONZERO — a zero status would tell a calling script the box is ready when the runtime every person needs is too old. Recorded with the constraint that shaped the implementation: **a number with one definition must be READ at every other site, never restated** — the floor is stamped into the release manifest by the release process, so a version bump touches no script, and the single-definition guard is satisfied rather than worked around. And two shell facts learned by running rather than reading, both of which end the installer silently if got wrong: a prompt in a `curl … | sh` script must read `/dev/tty` because stdin is the script itself; and `/dev/tty` must be tested by OPENING it inside a SUBSHELL, since the node exists in containers that have no terminal and a redirection failure on a special built-in is fatal to a non-interactive shell. + +- 2026-08-28 — **A message from a teammate reaches a busy person on the same terms as a message from the operator.** Human typing mid-turn rides Pi's steering queue and is consumed at the next step boundary; ordinary mail rode the follow-up queue and waited for the turn to end, which on an hour-long turn is an hour. Interrupt mail already rode the steering lane, so this widens a production-proven path rather than building one, and the digest stays exactly as it was — batching answers volume, and only the moment of consumption moved. Two boundaries recorded with it. **The wake lease is untouched by construction**: this is client-side delivery-mode selection, and the lease's readers key on `person_activity.operator_wake_at`, which this neither reads nor writes — the idle and boot-window rows are byte-identical, so nothing about waking a stopped person changed. And **a decision written as a literal at a call site cannot be asserted**: the delivery table was pinned while the mode passed into it was not, so the timing of every delivery could change with every test still green. Naming the decision was what made the change reviewable, and that is the general lesson — a constant repeated at two call sites is two decisions, and neither of them is testable. + +- 2026-08-28 — **A parameter description is a promise about the schema, and the schema is what the model obeys.** A documented default that the schema does not implement does not produce a caller who ignores the prose — it produces one who believes the prose, discovers the field is required anyway, and IMPROVISES a value. That is what happened: "DEFAULT: the department YOU head" over a required field, and an agent that reasoned its way to omitting the field, could not, and filled it with the most salient name in context. The instrument is the schema; the description is a claim about it; they must agree, and where they disagree the claim is what gets believed and the schema is what gets obeyed. The general question worth asking of any tool surface — **does the schema permit what the prose promises?** — is answerable mechanically, which is why the sweep was the substance of this change rather than an appendix to it. Recorded with the sharper reason the company-name alias stays rejected: with a real default in place the alias would exist ONLY on the override path, where a mid-level head naming the company means their own team — so it would resolve to the impressive-sounding wrong target rather than the semantically right one, which is worse than the refusal it would replace. +- 2026-08-28 — **The word a failure card uses is a claim about whose fault it was, so it follows the classification and never a list.** "Refused" invites a corrected call and "failed" invites a retry; using either for the other tells a reader to take the wrong action, and calling a crash a refusal is the worse direction because it sends somebody to fix a call that was right. The rule lives in ONE predicate because three renderers build failure titles, and a rule copied per renderer is a rule that will diverge at the first bespoke card. Recorded with the discovery that made it non-trivial: **a status carried for CONTEXT is not a classification.** The partial-hire card carries one so it can name the people already hired — that is data a retry needs, not an assertion about fault, and the error it wraps may be either kind. Where the two diverge, the error's own type decides and the producer marks itself; the renderer never grows a list of exceptions, because a list is where the next case is missed. + - 2026-08-28 — **Read your own pushed bytes: a claim that reads as verified is not verified, and the checks do not cover claims.** Three defects in one day were of this shape and none was catchable by any test in the repository: a comment naming THREE adapters when there were eight (a wrong number); a comment citing a guard called `CatchPathsFunnelThroughRefusalResult` that nothing answers to (a wrong referent); and a report that a group of conversions was complete on the strength of three sampled greps (a wrong scope). Each read as though somebody had checked, each was green, and each was found by looking at the pushed artifact rather than by running anything. The rule that follows is cheap and mechanical: after pushing, read what you pushed — the bytes at the SHA, not the diff you intended — and read the prose in it as sceptically as the code, because a comment is the one part of a change that no gate can falsify. Its companion, learned the same day: when a claim is disputed, settle it with the bytes at a named SHA rather than with a commit id, since a commit id proves when you pushed and only the bytes prove what is there. - 2026-08-28 — **A refusal must not lie about whose fault it is, because the label chooses the recovery.** A system fault invites the same call again; a caller error invites a corrected one. So a failure a tool DECIDED must be distinguishable from an exception it SUFFERED, all the way to the surface — and where the distinction is carried by the presence of a field, every adapter that flattens an error has to preserve it or the classification is lost in transit. It was: validation refusals were thrown as plain errors and seven catch adapters dropped them into status-less results, so a whole class of deliberate refusals rendered as crashes. The marker travels ON the error rather than being re-derived, because re-deriving it means matching message text, which is a second parser by another name. Two corollaries recorded with it. The plain error keeps its meaning — an invariant no input should reach IS a system fault and retrying it is right — and the fix is pinned from both sides, since a test asserting only "this is not a system fault" can be satisfied by labelling everything a refusal, which deletes the distinction instead of repairing it. And on the same incident: an ambiguous name is PREVENTED at the parameter that accepts it, never accepted as an alias — an alias binds everywhere the parameter appears, so a convenience at hire becomes a destructive action at remove, which is the wrong-target-from-name-confusion class arriving by a friendlier road. diff --git a/README.md b/README.md index 6e155a2..c45dc60 100644 --- a/README.md +++ b/README.md @@ -15,129 +15,135 @@ [![Discussions](https://img.shields.io/github/discussions/tribes-protocol/chief)](https://github.com/tribes-protocol/chief/discussions) [Quick start](#quick-start) · -[What is a company](docs/WHAT_IS_A_COMPANY.md) · -[Architecture](#architecture-in-sixty-seconds) · -[Examples](examples/) · -[Contributing](CONTRIBUTING.md) +[Commands](#everyday-commands) · +[How it works](#how-it-works) · +[Examples](#examples) · +[Contributing](#contributing) -![Switching between the rail and department panels](docs/assets/panels.gif) +chief creates and operates a persistent company of AI agents: a CEO, a +recursive tree of departments, and stable people who each have a name, a job +title, a mandate, and a private memory. You talk to them, and they talk to +each other. -The asciinema recording behind that GIF is -[`docs/assets/panels.cast`](docs/assets/panels.cast), and it is a **capture of -a running company** — a real `chiefd`, a real rail, real tmux panes, and real -Pi agents answering for themselves. Nothing in it is drawn by hand. The -company — Northwind Robotics — and everyone in it are fictional. +![Switching between the rail and department panels](docs/assets/panels.gif) ## Quick start -You need macOS or Linux, [`tmux`](https://github.com/tmux/tmux), and -[Pi](https://github.com/earendil-works/pi) 0.80.10 or newer -(`npm install -g --ignore-scripts @earendil-works/pi-coding-agent`). +You need macOS or Linux and [`tmux`](https://github.com/tmux/tmux). + +**1.** Install chief: ```bash curl -fsSL https://chief.zipbox.ai/install.sh | sh -export PATH="$HOME/.chief/bin:$PATH" ``` -Then found your first company. Any empty directory will do: +Every person in a company runs on [Pi](https://github.com/earendil-works/pi), +the agent runtime; the installer installs it when it is missing, and asks +before upgrading one that is older than the required version. + +**2.** Found your first company in any empty directory: ```bash mkdir acme && cd acme && chief ``` -That opens **Founder**. Founder learns exactly two things — the company's -**name** and its **purpose** — then creates the company and boots its CEO. -**The CEO is what builds the organisation**; Founder deliberately designs -nothing, and the founding boot is told so in as many words. -So tell the CEO what you want built — "a three-person research desk that -writes me a daily market brief" — and it creates the departments, appoints -the heads and hires the people. Come back later with `chief` in the same -directory. - -`chief ls` lists every company on the box; `chief upgrade` installs the latest -release over this one. +With no company in the directory, `chief` opens **Founder**. Founder asks for +exactly two things, the company's name and its purpose, then creates the +company and boots its CEO. Founder designs nothing; the CEO is what builds +the organisation. + +**3.** Talk to the CEO. Tell it what you want, for example "a three-person +research desk that writes me a daily market brief", and it creates the +departments, appoints the heads, and hires the people. Once the company +exists, Founder never appears again: from then on, `chief` in that directory +starts the company if it is stopped and attaches your terminal to the CEO. + +## Everyday commands + +| Command | What it does | +| --- | --- | +| `chief` | Found a company here, or start and attach the one that exists | +| `chief ls` | List every company on this machine and its state | +| `chief attach` | Put this terminal in this company's CEO, starting it if stopped | +| `chief stop` | Stop this company's runtime, then its daemon | +| `chief stand-down [reason]` | Stop everyone except the CEO; queued mail is held, not lost | +| `chief resume` | Let the company work again after a stand-down | +| `chief reset [--yes]` | Shed the company back to CEO-only, deleting nothing | +| `chief rm [--yes]` | Remove the company for good | +| `chief upgrade [--check\|--rollback]` | Install the latest release over this one | + +The full command surface, the disk layout, and the runtime are in +[`docs/OPERATING.md`](docs/OPERATING.md). ## Why chief -Most tools orchestrate agents as a flat pool of workers, or as panes in a -multiplexer that knows a process is running but not who it is. chief runs a -real **company**: a CEO, a recursive tree of departments, and stable people who -each have a name, a job title, a mandate, a private memory, and their own agent -home. You talk to them, and they talk to each other. - -Everything durable is a row in that directory's own SQLite database — not a -process, not a scrollback buffer, not a JSON file. A company survives being -closed, moved, and reopened, because **the company is the database**. Nothing on -disk records which window or pane anybody is in; placement is derived from the -org chart on every pass. - -And the whole thing is two programs you meet, plus **`beacond`**, a small -discovery daemon the installer puts beside them. **`chiefd`** is the backend: one daemon per -company directory, and it decides *who should be running*. **`chief`** is the -client: it owns tmux and your terminal, and it decides *where they are shown*. -The daemon cannot see a terminal and the client cannot decide policy, so an -observation is always a report from a client and never a second copy of the -truth. +Most tools orchestrate agents as a flat pool of workers. chief runs a real +organisation, and everything durable in it is a row in the directory's own +SQLite database. A company survives being closed, moved, and reopened, +because the company is the database. - A CEO and recursive departments, with named heads and specialists. - One private Pi history, workspace, inbox, and memory directory per person. -- Placement derived from the org chart, never from ad-hoc panes. -- Explicit skills and tools for every hire; models are Pi's own. +- Messages and reminders that survive a restart. - Durable assignments with acknowledgement, progress, escalation, and result - delivery — instead of relying on chat timing. -- Messages and reminders that survive a restart: a message is how work reaches - a person, and a reminder is how a person comes back to it. -- Safe stop, restart, transfer, and removal that keep a person's history until - you explicitly delete it. + delivery. +- Safe stop, restart, transfer, and removal that keep a person's history + until you explicitly delete it. - Parking: a quiet person costs no compute and loses nothing. -## Architecture in sixty seconds +## How it works -A company is a directory. Running `chief` in it starts one `chiefd` for that -directory, which opens `.chief/db/chief.db` and runs a supervisor loop of six -duties — reconcile, health, mailbox wake, deadlines, reminders, memory. That -loop decides which people should be active right now, and publishes the answer. - -The client asks for that answer over HTTP, compares it to the tmux session it -can actually see, and applies the difference: it opens panes, moves them, and -closes them. Every person in a pane is a Pi agent with its own home. `beacond` -is a small box-wide registry that answers "which daemon serves this company", -and admits exactly one daemon per company. +A company is a directory. Running `chief` in it starts one **`chiefd`** for +that directory: a daemon that opens `.chief/db/chief.db` and decides *who +should be running*. The **`chief`** client owns tmux and decides *where they +are shown*: it asks the daemon over HTTP, compares the answer to the session +it can actually see, and opens, moves, and closes panes. Every person in a +pane is a Pi agent with its own home. **`beacond`**, a small box-wide +registry, answers "which daemon serves this company" and admits exactly one +daemon per company. ![How chief, chiefd, beacond and the Pi panes fit together](docs/assets/architecture.svg) -| Where | What it owns | -| --- | --- | -| `apps/chiefd/crates/chiefd-daemon` | The backend binary — `chiefd`. | -| `apps/chiefd/crates/chiefd-core` | The typed docstore: the manifest and every ledger, as SQL. | -| `apps/chiefd/crates/chiefd-api` | The HTTP surface over that store. | -| `apps/chiefd/crates/chiefd-host` | Everything the backend touches on the machine. It names no tmux. | -| `apps/chiefd/crates/chief-cli` | The operator client — `chief`: tmux, the terminal, and every verb. | -| `apps/chiefd/crates/beacond` | The small no-auth discovery daemon. | -| `apps/web` | The browser client, and a full second host for a person. | -| `packages/chiefing` | The TypeScript client of chiefd and beacond. No business logic above it. | -| `packages/piing` | Pi artifacts: the skills and extensions copied into Pi homes. | - -> **Note:** `apps/web`, the browser host, is **not live and currently broken**. -> The terminal client is the product today. The web host stays in the tree -> because the daemon/client split is designed for a second host; see -> [`apps/web/README.md`](apps/web/README.md) for status. +The code map, crate by crate, is in +[`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md). ## Examples -Three companies you can copy and found in a minute — see +Three companies you can copy and found in a minute. See [`examples/`](examples/). | Example | The company | | --- | --- | -| [`trading-desk`](examples/trading-desk/) | **Meridian Desk** — a paper-trading research desk. Research, Execution, and Risk, with Risk reviewing every trade memo. | -| [`growth-studio`](examples/growth-studio/) | **Signal & Co.** — a growth and social agency. Content, Distribution, and a one-person Analytics unit. | -| [`oss-maintainers`](examples/oss-maintainers/) | **Patchwork Labs** — a company that maintains an open-source repo. Triage, Engineering, and Release. | +| [`trading-desk`](examples/trading-desk/) | **Meridian Desk**, a paper-trading research desk: Research, Execution, and Risk, with Risk reviewing every trade memo. | +| [`growth-studio`](examples/growth-studio/) | **Signal & Co.**, a growth and social agency: Content, Distribution, and a one-person Analytics unit. | +| [`oss-maintainers`](examples/oss-maintainers/) | **Patchwork Labs**, a company that maintains an open-source repo: Triage, Engineering, and Release. | + +## Contributing + +Contributions are welcome, and two large pieces of work are open right now. + +**1. Finish the web client.** [`apps/web`](apps/web/) is a Next.js browser +host for a company. Real code exists: the API and SSE client services, the +hooks and providers, and unit suites that run in CI on every pull request. +But it does not build a working host today, nothing a user installs contains +it, and the terminal client is the product. +[`apps/web/README.md`](apps/web/README.md) describes what it was built to do +and asks that a revival start with a +[Discussion](https://github.com/tribes-protocol/chief/discussions), with a +written account of what actually breaks. + +**2. Run departments on different machines.** Today an entire company runs on +the machine where its directory lives, and a company with many active people +pegs that one machine's CPU. The goal is to let different departments be +hosted on different machines. No design exists yet; bring proposals to +[Discussions](https://github.com/tribes-protocol/chief/discussions). + +Smaller work lives in the +[issue tracker](https://github.com/tribes-protocol/chief/issues). Setup, +conventions, and the pull-request contract are in +[`CONTRIBUTING.md`](CONTRIBUTING.md). ## Where to read next @@ -152,6 +158,7 @@ Three companies you can copy and found in a minute — see ## Licence -Apache-2.0 — see [`LICENSE`](LICENSE) and [`NOTICE`](NOTICE). Contributions are -accepted under the [Developer Certificate of Origin](CONTRIBUTING.md#developer-certificate-of-origin); +Apache-2.0. See [`LICENSE`](LICENSE) and [`NOTICE`](NOTICE). Contributions +are accepted under the +[Developer Certificate of Origin](CONTRIBUTING.md#developer-certificate-of-origin); sign your commits with `git commit --signoff`. diff --git a/install.sh b/install.sh index 4552171..e903ca7 100755 --- a/install.sh +++ b/install.sh @@ -231,12 +231,126 @@ have tmux || { say " macOS: brew install tmux" say " Debian/Ubuntu: apt-get install -y tmux" } -have pi || { - say "" - say "Pi is the agent runtime every person in a company runs, and was not found. Install it:" - say " $PI_INSTALL" +# --- Pi ------------------------------------------------------------------- +# +# chief installs and upgrades Pi rather than printing a command and hoping. +# The asymmetry is deliberate: an ABSENT Pi is installed without asking, since +# chief cannot run a single person without it and there is nothing to weigh. +# An EXISTING Pi that is merely too old is the user's, and replacing somebody's +# working tool without asking is a different act, so that one prompts. +# +# THE FLOOR IS READ, NEVER WRITTEN HERE. `pi_floor.rs` holds the single +# definition, `release-chiefd.ts` stamps it into the release manifest as +# `piFloor`, and this reads it out of the manifest already unpacked above. A +# version bump therefore needs no edit to this file, and the repository's +# single-definition guard stays satisfied — a number restated here would be a +# second definition wearing a copy's clothes. +pi_floor="$(grep -m1 '"piFloor"' "$dest/manifest.json" 2>/dev/null \ + | sed -E 's/.*"piFloor"[[:space:]]*:[[:space:]]*"([^"]+)".*/\1/')" + +# Sorts dotted versions without assuming a numeric field count. +version_below() { + [ "$1" != "$2" ] && [ "$(printf '%s\n%s\n' "$1" "$2" | sort -t. -k1,1n -k2,2n -k3,3n | head -n1)" = "$1" ] +} + +# A PROMPT IN A PIPED SCRIPT MUST NOT READ ITS OWN SOURCE. +# +# This file is `curl … | sh`, so stdin is the script text: reading stdin would +# consume the rest of the installer. The question goes to the terminal +# directly. Where there is no terminal — CI, a container build — nothing can be +# asked, so the default answer is taken and SAID, because a silent choice made +# on somebody's behalf is the thing that surprises them later. +confirm_default_yes() { + # OPENABLE, not merely present. `[ -r /dev/tty ]` is TRUE in a container with + # no controlling terminal — the device node exists and the permission bits + # allow reading — and the redirect then fails with a raw shell error that no + # `2>/dev/null` on the command can suppress, because the shell reports it + # while setting the redirection up. Measured, not reasoned: it printed + # "cannot create /dev/tty: No such device or address" twice, before the + # question. Testing the open is the only honest test of whether it will work. + # A SUBSHELL, and that is load-bearing rather than stylistic. `:` is a POSIX + # SPECIAL BUILT-IN, and a redirection error on one is fatal to a + # non-interactive shell — `{ : < /dev/tty; } 2>/dev/null` does not evaluate + # to false in a container, it ENDS THE INSTALLER, silently, with status 2 and + # no message. Measured: the run stopped dead at this line. The subshell + # contains the death so the `if` sees an ordinary false. + if ( : < /dev/tty ) 2>/dev/null; then + printf '%s [Y/n] ' "$1" > /dev/tty + read -r reply < /dev/tty || reply="" + else + reply="" + say "$1 [Y/n] — no terminal to ask on, taking the default (yes)." + fi + case "$reply" in + [Nn]*) return 1 ;; + *) return 0 ;; + esac +} + +pi_version_now() { + pi --version 2>/dev/null | tr -d 'v' | awk '{print $NF}' } +install_pi() { + have npm || die "npm is required to install Pi, and it is not on PATH. Install Node.js, then run: $PI_INSTALL" + say "Installing Pi ($PI_INSTALL)…" + # A failure here is REPORTED, never silent, and never claimed as success. + if ! $PI_INSTALL; then + say "Pi could not be installed. chief is installed; run this yourself and it will work:" + say " $PI_INSTALL" + return 1 + fi + have pi || { + say "npm reported success but pi is not on PATH yet — open a new shell, or run:" + say " $PI_INSTALL" + return 1 + } + installed_version="$(pi_version_now)" + # CHECKED, not assumed. npm exiting zero is not the same as the floor being + # met — a global install can land somewhere earlier on PATH, or resolve to a + # version that is still too old — and reporting "ready" without looking is + # the same shape as every other claim that outran its evidence. + if [ -n "$pi_floor" ] && [ -n "$installed_version" ] && version_below "$installed_version" "$pi_floor"; then + say "Pi is $installed_version, still below $pi_floor. Install it yourself with:" + say " $PI_INSTALL" + return 1 + fi + say "Pi ${installed_version:-installed} is ready." +} + +say "" +if ! have pi; then + # ABSENT: no question. chief cannot run a person without it. + say "Pi is the agent runtime every person in a company runs, and was not found." + # NONZERO, for the same reason the declined upgrade below is nonzero, and more + # strongly. `install_pi` has already told the PERSON what to run; what it + # cannot do is tell a CALLER. A failed install here leaves the thing that runs + # people ABSENT, which is strictly worse than the too-old Pi the decline path + # already refuses to call success — so this cannot be the branch that reports + # ready. `|| true` said the opposite to every script consuming this installer. + install_pi || die "chief itself is installed under $CHIEF_HOME; Pi did not install — run: $PI_INSTALL" +else + pi_version="$(pi_version_now)" + if [ -n "$pi_floor" ] && [ -n "$pi_version" ] && version_below "$pi_version" "$pi_floor"; then + say "Pi $pi_version is installed; chief needs $pi_floor or newer." + if confirm_default_yes "Upgrade Pi to >= $pi_floor?"; then + # Nonzero for the reason above: an ACCEPTED upgrade that then failed + # leaves exactly the too-old Pi the branch below refuses to call success. + # Agreeing to fix it does not make it fixed. + install_pi || die "chief itself is installed under $CHIEF_HOME; Pi did not upgrade — run: $PI_INSTALL" + else + # One of the places this script exits nonzero after chief is installed — + # countless, because a count in a comment is a fact that goes stale + # silently. It is not a failed install: it is a declined prerequisite, and + # saying so with a zero status would tell a script that everything is + # ready when the thing that runs people is too old. The failed-install + # paths above exit nonzero for the same reason, applied to a worse state. + say "" + die "chief requires Pi $pi_floor or newer, and the upgrade was declined. chief itself is installed under $CHIEF_HOME." + fi + fi +fi + say "" say "Then found your first company:" say " mkdir acme && cd acme && chief" diff --git a/packages/piing/extensions/organization-intercom.ts b/packages/piing/extensions/organization-intercom.ts index 708aa16..1192cb0 100644 --- a/packages/piing/extensions/organization-intercom.ts +++ b/packages/piing/extensions/organization-intercom.ts @@ -2449,6 +2449,52 @@ export function workResumeNeedsRedrive(prompted: boolean, pending: boolean): boo * * Once the gate opens the table below is byte-identical to what it has always * been, which `an_open_gate_restores_the_exact_busy_idle_table` pins. */ +/** + * The queue a MAILBOX ENVELOPE rides: the same one the operator's own typing + * rides. + * + * Human typing mid-turn is submitted with `streamingBehavior: "steer"` and is + * consumed at the next STEP BOUNDARY inside the running turn. Our interrupt + * mail already rode that lane. Normal mail rode `followUp`, which Pi consumes + * only when the agent has no more tool calls or steering messages — the END of + * the turn. So a person mid-way through an hour of work did not see an + * ordinary message until the hour was over, while the same words typed by the + * operator arrived within seconds. + * + * Named rather than written twice at the call sites, because it is one + * decision and two literals are two decisions waiting to drift — and because a + * literal at a call site cannot be asserted, which is why this change arrived + * with no test able to notice it. + * + * The digest is untouched: batching is still the answer to twenty messages in + * one turn. Only WHEN the batch is consumed moves. + */ +function mailboxDeliveryMode(): "steer" | "followUp" { + return "steer"; +} + +/** The work-resume prompt's delivery, as ONE definition the call site reads. + * + * This is a work-resume prompt, not a mailbox envelope: nobody is waiting on it + * and it asks the person to pick their own work back up, so arriving at the end + * of the current turn is the honest reading. Mail moved to steer; this did not. + * + * The MODE is its own function, and that is the load-bearing part rather than a + * flourish. A first attempt put the literal here AND in the test seam, so the + * two were independent copies: reverting this function to "steer" left every + * test green, which is the exact false pin the paragraph above warns about — + * shipped one function away from the warning. `workResumeDeliveryMode` is now + * the single definition both the call site and the seam read, so the mutation + * that matters has one place to happen and one test that sees it. + */ +function workResumeDeliveryMode(): "steer" | "followUp" { + return "followUp"; +} + +function workResumeDelivery(): QueuedPiDeliveryOptions { + return queuedPiDelivery(workResumeDeliveryMode()); +} + function queuedPiDelivery( mode: "steer" | "followUp", turnActive: boolean = piTurnInFlight, @@ -5884,15 +5930,23 @@ const ADD_DEPARTMENT_PARAMETERS = Type.Object({ }, { additionalProperties: false }); const HIRE_PARAMETERS = Type.Object({ - departmentId: Type.String({ + // OPTIONAL, because the description promises a default and a required field + // cannot deliver one. It was `Type.String` — required — under a description + // opening "DEFAULT: the department YOU head". An agent read the prose, + // reasoned that it should omit the field, met a schema that would not let it, + // and improvised the most salient name in context: the company. It obeyed + // the instrument over the claim, which is the correct thing for it to do. + departmentId: Type.Optional(Type.String({ description: - "Where this person lands. DEFAULT: the department YOU head — a hire joins the team that " - + "asked for it. Name a different one only when the operator named it. This call never " - + "creates a department, and a job title never asks for one: \"hire a Chief of Staff\" is a " - + "hire into your own department, not a new unit. Create a department only when the " - + "operator asked for a department in those words. " - + "The company name or slug is NEVER a department id — the root department's id is in org_roster.", - }), + "Where this person lands. OMIT IT to hire into the department you head — that is the " + + "DEFAULT and it is what you want almost always, because a hire joins the team that " + + "asked for it. Pass one only to override that, and only when the operator named a " + + "different department. This call never creates a department, and a job title never " + + "asks for one: \"hire a Chief of Staff\" is a hire into your own department, not a new " + + "unit. Create a department only when the operator asked for a department in those " + + "words. If you do pass one, the company name or slug is NEVER a department id — the " + + "root department's id is in org_roster.", + })), /** One person, the original shape. */ person: Type.Optional(PERSON_SEED), /** Several people in ONE call — see the batch note in `execute`. */ @@ -6111,6 +6165,24 @@ function organizationToolSuccessPlainText(presentation: ToolSuccessPresentation) * quirk: the specific `recipient_lookup`/`message_text_required` statuses only * produce their own label when `waiting` is also true) — `undefined` means this * is a hard failure, not a retryable/waiting/busy one. */ +/** + * Whether a failure card should say "refused" rather than "failed". + * + * ONE rule, in one place, because there are three renderers that build a + * failure title and a rule copied three times is three rules waiting to + * disagree. A classified failure is one the tool DECIDED and can explain, so + * "refused" — a word that invites a corrected call. Anything else stays + * "failed", which invites a retry. + * + * `fault: true` is what a producer sets when it carries a status for CONTEXT + * rather than as a classification: a partial batch naming what already landed, + * where the wrapped error may be a genuine crash. A marker, not a list, so the + * next producer in that position is covered without this predicate changing. + */ +function isCallerRefusalCard(detail: Record | undefined): boolean { + return typeof detail?.status === "string" && detail.fault !== true; +} + function organizationToolRetryPresentation(detail: Record): { state: CardState; title: string } | undefined { const waiting = detail.retryable === true || detail.status === "awaiting_handoff" || detail.status === "awaiting_handoffs"; if (!waiting) return undefined; @@ -6183,15 +6255,38 @@ function defaultOrganizationToolRenderResult(organization: string, name: string, // the site describes by token NAME, so renderCard colors them and no color // is hand-rolled here (AC1). #333: `opId` is the id the structured failure // record was logged under, so a cryptic card is one grep from full context. + // THE VERB FOLLOWS THE CLASSIFICATION, not a list of card kinds. + // + // A classified failure is one the tool DECIDED and can explain, so it is + // "refused" — a word that invites a corrected call. An unclassified one is + // a caught exception, so it stays "failed", which invites a retry. Getting + // this backwards in either direction is the defect: calling a crash + // "refused" tells a reader to fix a call that was never wrong. + // + // `fault: true` is the one thing a producer sets when it carries a status + // for CONTEXT rather than as a classification — a partial batch naming + // what already landed, where the wrapped error may be a real crash. It is + // a marker rather than a list, so a future producer in the same position + // is covered without this line changing. + const refused = !retry && isCallerRefusalCard(detail); const titleTags: CardTag[] = []; - if (unclassified) titleTags.push({ text: "(system fault)", token: "dim" }); + // THE TAG READS THE SAME MARKER AS THE VERB. `unclassified` alone measured + // only the ABSENCE of a status, so a result carrying one for context while + // wrapping a real crash lost the tag — the verb had moved to the fault + // marker and the tag had stayed on the old instrument. One classification, + // two surfaces, and they must not disagree: a reader debugging a mid-batch + // crash would otherwise see "failed" with no crash marker beside a list of + // people already hired, and reasonably conclude they had passed bad input. + if (unclassified || detail.fault === true) titleTags.push({ text: "(system fault)", token: "dim" }); if (typeof detail.opId === "string") titleTags.push({ text: `(ref ${detail.opId})`, token: "dim" }); if (summary.text) titleTags.push({ text: `· ${summary.text}${summary.truncated ? "…" : ""}`, token: "dim" }); if (!expanded && summary.truncated) titleTags.push({ text: CARD_EXPAND_HINT_TEXT, token: "dim", sep: " " }); return renderDefaultOrganizationToolCard(theme, { kind: "tool-failure", icon: retry ? retry.state : "failure", - title: retry ? retry.title : `${organizationToolDomainIcon(name).title} failed`, + title: retry + ? retry.title + : `${organizationToolDomainIcon(name).title} ${refused ? "refused" : "failed"}`, target: retry ? undefined : (target || undefined), mentions, titleTags, @@ -7439,7 +7534,7 @@ async function installSubtreeTools( pi.registerTool({ name: "org_hire", label: "Hire an organization person", - description: "Hire one durable worker into an EXISTING department — by DEFAULT the one you head — only after the roster shows no suitable existing person. Send person as real JSON, never a quoted string; use people: [ … ] for several at once. Example: {\"departmentId\":\"engineering\",\"person\":{\"name\":\"Rhea\",\"title\":\"Staff Engineer\",\"mandate\":\"Own the SQLite store.\"}}. name is one short first name; the job goes in title. A NEW DEPARTMENT IS THE OPERATOR'S DECISION AND NEVER YOURS TO INFER: if they asked for one in those words use org_add_department, which makes it and its head together; if they did not, this call is the whole answer. \"Chief of Staff\" and \"Head of Growth\" are TITLES, not requests for a unit. No field asks you to justify anything. Put technology requirements in mandate; a hire does not select skills, extensions, or packages. A new hire comes up on its own; you do not have to start them, and nobody is stopped at creation.", + description: "Hire one durable worker into an EXISTING department — by DEFAULT the one you head, so OMIT departmentId unless the operator named another — only after the roster shows no suitable existing person. Send person as real JSON, never a quoted string; use people: [ … ] for several at once. Example: {\"person\":{\"name\":\"Rhea\",\"title\":\"Staff Engineer\",\"mandate\":\"Own the SQLite store.\"}}; add departmentId only to override. name is one short first name; the job goes in title. A NEW DEPARTMENT IS THE OPERATOR'S DECISION AND NEVER YOURS TO INFER: if they asked for one in those words use org_add_department, which makes it and its head together; if they did not, this call is the whole answer. \"Chief of Staff\" and \"Head of Growth\" are TITLES, not requests for a unit. No field asks you to justify anything. Put technology requirements in mandate; a hire does not select skills, extensions, or packages. A new hire comes up on its own; you do not have to start them, and nobody is stopped at creation.", parameters: HIRE_PARAMETERS, prepareArguments: stringifiedArgumentRepair(context, "org_hire", HIRE_PARAMETERS) as never, async execute(_toolCallId, params) { @@ -7481,9 +7576,20 @@ async function installSubtreeTools( // department 'belfort-brothers-capital'" for a department that simply // did not exist, then followed its remediation sentence into a create // the core refuses. Both halves are derived now, never static. - const hireDenial = departmentScopeDenial(gate.manifest, hiringManager, params.departmentId); + // THE DEFAULT THE DESCRIPTION PROMISES, resolved here rather than + // demanded of the caller: the department this person heads, or failing + // that the one they sit in. That is `authorityRootDepartmentId`, which + // already existed and is character-for-character what the prose says — + // the promise was always implementable, it simply was not implemented. + const departmentId = params.departmentId ?? authorityRootDepartmentId(gate.manifest, hiringManager); + if (departmentId === undefined) { + throw new CallerRefusal( + "Could not determine which department to hire into, and none was given. Pass departmentId naming one from org_roster.", + ); + } + const hireDenial = departmentScopeDenial(gate.manifest, hiringManager, departmentId); if (hireDenial === "unknown-department") { - throw new CallerRefusal(unknownDepartmentMessage(gate.manifest, hiringManager, params.departmentId, "hire into")); + throw new CallerRefusal(unknownDepartmentMessage(gate.manifest, hiringManager, departmentId, "hire into")); } if (hireDenial) { // Name the ACCEPTED path, not just the refusal. Everyone now carries @@ -7491,7 +7597,7 @@ async function installSubtreeTools( // department it merely sits in — and the answer is to grow its own // unit first, never to loosen the scope check. throw new Error( - `'${hiringManager.id}' does not manage department '${params.departmentId}'. ${hiringPathAdvice(gate.manifest, hiringManager)}`, + `'${hiringManager.id}' does not manage department '${departmentId}'. ${hiringPathAdvice(gate.manifest, hiringManager)}`, ); } for (const seed of seeds) { @@ -7501,35 +7607,35 @@ async function installSubtreeTools( // on the operator's own defaults, like everybody else. const request = hireRequest({ slug: gate.slug, - departmentId: params.departmentId, + departmentId, hiringManagerPersonId: hiringManager.id, person: seed as unknown as Record, }); const outcome = await staffingApply(gate, "/v1/org/person/hire", request as unknown as Record, { - action: "hire", departmentId: params.departmentId, personId: request.personId || undefined, name: request.name, + action: "hire", departmentId, personId: request.personId || undefined, name: request.name, }); // A refusal mid-batch reports WHO was already hired. Silently // dropping that list is how an operator retries a batch and gets // duplicates of the people who succeeded the first time. if ("refused" in outcome) { - return routeRefusal("Hire", outcome, { departmentId: params.departmentId, hired }); + return routeRefusal("Hire", outcome, { departmentId: departmentId, hired }); } hired.push({ name: request.name }); } if (hired.length === 1) { const only = hired[0]!; - return toolResult(true, `Hired ${only.name} into '${params.departmentId}'. They come up on their own; they stop on their own once they settle after idling.`, { + return toolResult(true, `Hired ${only.name} into '${departmentId}'. They come up on their own; they stop on their own once they settle after idling.`, { status: "applied", - departmentId: params.departmentId, + departmentId: departmentId, name: only.name, hired, }); } const roster = hired.map((entry) => entry.name).join(", "); - return toolResult(true, `Hired ${hired.length} people into '${params.departmentId}': ${roster}. They come up on their own; each stops on its own once it settles after idling.`, { + return toolResult(true, `Hired ${hired.length} people into '${departmentId}': ${roster}. They come up on their own; each stops on its own once it settles after idling.`, { status: "applied", - departmentId: params.departmentId, + departmentId: departmentId, hired, }); } catch (error) { @@ -7541,7 +7647,17 @@ async function installSubtreeTools( const landed = hired.length ? ` Already hired, do NOT re-send: ${hired.map((entry) => entry.name).join(", ")}. Retry only the rest.` : ""; - if (landed) return toolResult(false, `${safeExceptionMessage(error)}${landed}`, { status: "hire_partial", hired }); + // The status here carries the already-hired list; it is NOT a claim + // about whose fault the failure was. The wrapped error can be either + // kind — a mid-batch caller refusal (an unknown department on person + // four) or a genuine crash — so the error's own type decides, exactly + // as it does everywhere else. Without this the card would call a + // crash "refused" and invite a correction to a call that was right. + if (landed) return toolResult(false, `${safeExceptionMessage(error)}${landed}`, { + status: "hire_partial", + hired, + ...(error instanceof CallerRefusal ? {} : { fault: true }), + }); return lifecycleFailure(error); } }, @@ -7769,7 +7885,9 @@ async function installSubtreeTools( // #360: this used to interpolate the raw internal verb into the // title ("⚠️ bench failed", "⚠️ recall failed") instead of a proper // sentence-case title. - const hardFailTitle = action === "bench" ? "Bench failed" : "Recall failed"; + // The same rule as the default card: a decided refusal is "refused". + const verb = isCallerRefusalCard(detail) ? "refused" : "failed"; + const hardFailTitle = action === "bench" ? `Bench ${verb}` : `Recall ${verb}`; return renderOrganizationCard(theme, { kind: "tool-failure", icon: handoff ? "handoff" : domainIcon(CARD_GLYPHS.failure, detail?.retryable ? "warning" : "error"), @@ -7970,7 +8088,8 @@ async function installSubtreeTools( const handoff = detail?.status === "awaiting_handoff" || detail?.status === "awaiting_handoffs"; // #360: this used to interpolate the raw internal verb into the // title ("⚠️ start-person failed", "⚠️ stop-person failed"). - const hardFailTitle = action === "start-person" ? "Start failed" : "Stop failed"; + const verb = isCallerRefusalCard(detail) ? "refused" : "failed"; + const hardFailTitle = action === "start-person" ? `Start ${verb}` : `Stop ${verb}`; return renderOrganizationCard(theme, { kind: "tool-failure", icon: handoff ? "handoff" : domainIcon(CARD_GLYPHS.failure, detail?.retryable ? "warning" : "error"), @@ -8262,6 +8381,55 @@ export function messageWakeDispositionForTest( * error a validation site throws, so the round trip is testable without * driving a whole tool. */ +/** + * The default `org_hire` resolves when `departmentId` is omitted — the one the + * parameter description promises. Exported so BOTH arms of it can be asserted + * without booting a company: the department a head heads, and the department a + * non-head merely sits in. + */ +export function hireDefaultDepartmentForTest( + manifest: IntercomOrganizationManifest, + person: PersonRecord, +): string | undefined { + return authorityRootDepartmentId(manifest, person); +} + +/** + * Whether the card carries the `(system fault)` tag. + * + * The SAME marker the verb reads, exposed separately so a test can prove the + * two surfaces cannot drift apart — which they had, the verb having moved to + * the fault marker while the tag still measured only the absence of a status. + */ +export function showsSystemFaultTagForTest(detail: Record | undefined): boolean { + const hasStatus = typeof detail?.status === "string"; + return !hasStatus || detail?.fault === true; +} + +/** The verb rule, for the discriminating pair. */ +export function isCallerRefusalCardForTest(detail: Record | undefined): boolean { + return isCallerRefusalCard(detail); +} + +/** + * The queue mailbox envelopes ride, and the options that follow from it — + * exposed together so the RULE can be asserted rather than the table alone. + * + * The table was already pinned; which mode the mailbox passes into it was not, + * which is why this change could alter every person's delivery timing without + * a single test noticing. + */ +/** The work-resume prompt's delivery — the boundary of the mail change, from + * the other side. Mail steers; this deliberately does not, and that asymmetry + * is the one place this change kept code rather than deleting it. */ +export function workResumeDeliveryForTest(turnActive: boolean, bootWindow: boolean): QueuedPiDeliveryOptions { + return queuedPiDelivery(workResumeDeliveryMode(), turnActive, bootWindow); +} + +export function mailboxDeliveryForTest(turnActive: boolean, bootWindow: boolean): QueuedPiDeliveryOptions { + return queuedPiDelivery(mailboxDeliveryMode(), turnActive, bootWindow); +} + export function refusalResultForTest(error: unknown): { details?: Record } { return refusalResult(error) as unknown as { details?: Record }; } @@ -8399,7 +8567,11 @@ export async function drainOrganizationMailbox( try { if (!isCurrent()) return delivered; pi.sendMessage({ customType: MESSAGE_TYPE, content: messageContext(envelope, context.personId, role), display: true, details: envelope }, - queuedPiDelivery(isInterruptDelivery ? "steer" : "followUp")); + // Always the mailbox lane: a normal envelope is routed into `normal` + // three lines above and never reaches here, so the ternary this + // replaced had an unreachable false arm even before normal urgency + // moved to steer. + queuedPiDelivery(mailboxDeliveryMode())); } catch (error) { appendOrganizationEvent(context, { event: "message-delivery-deferred", id: envelope.id, personId: context.personId, error: safeExceptionMessage(error), at: new Date().toISOString() }); logOrganizationException(context, "organization-mailbox-delivery", error, { messageId: envelope.id }); @@ -8422,7 +8594,7 @@ export async function drainOrganizationMailbox( }; try { if (!isCurrent()) return delivered; - pi.sendMessage({ customType: MESSAGE_TYPE, content: mailboxBatchContext(batch, context.personId, role), display: true, details: batch }, queuedPiDelivery("followUp")); + pi.sendMessage({ customType: MESSAGE_TYPE, content: mailboxBatchContext(batch, context.personId, role), display: true, details: batch }, queuedPiDelivery(mailboxDeliveryMode())); } catch (error) { appendOrganizationEvent(context, { event: "message-batch-delivery-deferred", batchId: batch.batchId, personId: context.personId, count: selected.length, error: safeExceptionMessage(error), at: new Date().toISOString() }); logOrganizationException(context, "organization-mailbox-batch-delivery", error, { batchId: batch.batchId, count: selected.length }); @@ -8438,7 +8610,7 @@ export async function drainOrganizationMailbox( if (!isCurrent()) return delivered; if (deliveryAttempts.has(file) || deliveryAttempts.size >= ORGANIZATION_MAILBOX_MAX_OUTSTANDING_DELIVERIES) continue; try { - pi.sendMessage({ customType: MESSAGE_TYPE, content: messageContext(envelope, context.personId, role), display: true, details: envelope }, queuedPiDelivery("followUp")); + pi.sendMessage({ customType: MESSAGE_TYPE, content: messageContext(envelope, context.personId, role), display: true, details: envelope }, queuedPiDelivery(mailboxDeliveryMode())); } catch (error) { appendOrganizationEvent(context, { event: "message-delivery-deferred", id: envelope.id, personId: context.personId, error: safeExceptionMessage(error), at: new Date().toISOString() }); logOrganizationException(context, "organization-mailbox-delivery", error, { messageId: envelope.id }); @@ -9833,9 +10005,14 @@ export async function installOrganizationIntercom(pi: ExtensionAPI, options: Ins // EVERY DELIVERY THIS TURN CONSUMED, so a turn that dies can say what it // destroyed. // - // Acceptance is at `message_start` — TURN START, not completion — and it is - // the durable pending→accepted move. That is correct and is not what this - // change touches: a message must not stay pending while a turn reads it, or a + // Acceptance is at `message_start`, not at turn completion — and it is the + // durable pending→accepted move. `message_start` is NOT the same thing as the + // start of a turn, and saying so was this comment's old error: a steered + // message fires `message_start` in the MIDDLE of a turn already running, which + // is the whole point of steering. The rule the code keeps is the one that + // survives either case — a message is accepted when a turn begins READING it, + // whenever in that turn's life that happens. That is correct and is not what + // this change touches: a message must not stay pending while a turn reads it, or a // crash re-delivers work that was already begun. The consequence is what was // wrong: a turn that then FAILS has consumed the envelope and answered // nothing, and before this the sender was never told, so an operator's @@ -11374,7 +11551,11 @@ export async function installOrganizationIntercom(pi: ExtensionAPI, options: Ins content: workResumePrompt(person, details), display: true, details, - }, queuedPiDelivery("followUp")); + // CONSIDERED AND KEPT — and now PINNED, via the same one-definition + // shape mail uses. A literal at a call site cannot be asserted, so + // "considered and kept" was a claim no test could check and a revert + // to steer would have passed everything. + }, workResumeDelivery()); appendOrganizationEvent(context, { event: "work-resume-prompt-requested", personId: context.personId, diff --git a/packages/piing/test/CallerRefusalClassification.test.ts b/packages/piing/test/CallerRefusalClassification.test.ts index 7b37416..7d1ffe1 100644 --- a/packages/piing/test/CallerRefusalClassification.test.ts +++ b/packages/piing/test/CallerRefusalClassification.test.ts @@ -20,7 +20,12 @@ import { readFileSync } from 'node:fs' import { fileURLToPath } from 'node:url' -import { callerRefusalForTest, refusalResultForTest } from '@test-assets/organization-intercom' +import { + callerRefusalForTest, + isCallerRefusalCardForTest, + refusalResultForTest, + showsSystemFaultTagForTest +} from '@test-assets/organization-intercom' import { describe, expect, test } from 'vitest' describe('a decided refusal keeps its classification through the adapters', () => { @@ -105,3 +110,87 @@ describe('every catch path funnels through refusalResult', () => { expect(handFlattenedResults(offending)).toHaveLength(1) }) }) + +/** + * THE VERB FOLLOWS THE CLASSIFICATION. + * + * "refused" invites a corrected call; "failed" invites a retry. Which word a + * card uses is therefore a claim about whose fault the failure was, and the + * two must not be interchangeable — a crash called "refused" tells a reader to + * fix a call that was never wrong, which is the #11 defect pointed the other + * way and worse for it. + */ +describe('a card says refused only when the tool decided it', () => { + test('a classified refusal is refused', () => { + expect(isCallerRefusalCardForTest({ status: 'refused' })).toBe(true) + expect(isCallerRefusalCardForTest({ status: 'incumbent_disposition_required' })).toBe(true) + }) + + /** + * THE DISCRIMINATING HALF. Without it the rule above passes by returning + * true for everything — which would relabel every crash a refusal and delete + * the distinction rather than using it. + */ + test('an unclassified failure is NOT refused', () => { + expect(isCallerRefusalCardForTest({})).toBe(false) + expect(isCallerRefusalCardForTest(undefined)).toBe(false) + }) + + /** + * A status carried for CONTEXT is not a classification. The partial-hire card + * names what already landed so a retry does not double-hire; the error it + * wraps may be a genuine crash, and only the error's own type knows. + */ + test('a status carried as context with fault:true is NOT refused', () => { + expect(isCallerRefusalCardForTest({ status: 'hire_partial', fault: true })).toBe(false) + expect(isCallerRefusalCardForTest({ status: 'hire_partial' })).toBe(true) + }) +}) + +/** + * THE VERB AND THE TAG READ THE SAME MARKER. + * + * They diverged: the verb moved to the fault marker while the tag still + * measured only the absence of a status. A partial batch wrapping a real crash + * therefore said "failed" — correctly — with no crash marker beside the list of + * people already hired, which reads as bad input to anyone debugging it. + * + * One classification, two surfaces, and a test that fails if either moves + * without the other. + */ +describe('the system-fault tag reads the same marker as the verb', () => { + test('an unclassified failure carries the tag', () => { + expect(showsSystemFaultTagForTest({})).toBe(true) + expect(showsSystemFaultTagForTest(undefined)).toBe(true) + }) + + test('a decided refusal does NOT carry the tag', () => { + expect(showsSystemFaultTagForTest({ status: 'refused' })).toBe(false) + expect(showsSystemFaultTagForTest({ status: 'hire_partial' })).toBe(false) + }) + + test('a context-carried status wrapping a crash DOES carry the tag', () => { + expect(showsSystemFaultTagForTest({ status: 'hire_partial', fault: true })).toBe(true) + }) + + test('verb and tag never disagree about the same detail', () => { + // The property that matters is not either rule alone, it is that they are + // complementary: anything called "refused" must not be tagged a fault, and + // anything tagged a fault must not be called "refused". + const cases: Array<{ label: string; detail: Record }> = [ + { label: 'no status', detail: {} }, + { label: 'a decided refusal', detail: { status: 'refused' } }, + { label: 'a context status', detail: { status: 'hire_partial' } }, + { + label: 'a context status wrapping a crash', + detail: { status: 'hire_partial', fault: true } + } + ] + for (const { label, detail } of cases) { + expect( + isCallerRefusalCardForTest(detail) && showsSystemFaultTagForTest(detail), + `${label} must not be both a refusal and a system fault` + ).toBe(false) + } + }) +}) diff --git a/packages/piing/test/HireDefaultDepartment.test.ts b/packages/piing/test/HireDefaultDepartment.test.ts new file mode 100644 index 0000000..c945973 --- /dev/null +++ b/packages/piing/test/HireDefaultDepartment.test.ts @@ -0,0 +1,128 @@ +/** + * THE SCHEMA MUST PERMIT WHAT THE DESCRIPTION PROMISES. + * + * `org_hire`'s `departmentId` was a REQUIRED field whose own description opened + * "DEFAULT: the department YOU head". An agent read that, reasoned correctly + * that it should omit the field, met a schema that would not allow it, and + * improvised the most salient name in context — the company's. + * + * It obeyed the instrument over the claim, which is the right thing for it to + * do. The description was a promise about the schema that the schema did not + * implement, and prose is the one part of a tool surface no gate falsifies. + */ +import { isNullish } from '@test/support/Nullish' +import type { IntercomOrganizationManifest, PersonRecord } from '@test-assets/organization-intercom' +import { + departmentScopeDenial, + hireDefaultDepartmentForTest, + unknownDepartmentMessage +} from '@test-assets/organization-intercom' +import { describe, expect, test } from 'vitest' + +function company(): IntercomOrganizationManifest { + return { + schemaVersion: 1, + kind: 'organization', + slug: 'acme-capital', + name: 'Acme Capital', + rootDepartmentId: 'executive', + departmentOrder: ['executive', 'engineering'], + peopleOrder: ['chief', 'eng-head', 'worker'], + departments: { + executive: { + id: 'executive', + name: 'Executive', + headPersonId: 'chief', + parentDepartmentId: undefined, + purpose: 'Run the company.', + state: 'active' as const + }, + engineering: { + id: 'engineering', + name: 'Engineering', + headPersonId: 'eng-head', + parentDepartmentId: 'executive', + purpose: 'Ship it.', + state: 'active' as const + } + }, + people: { + chief: person('chief', 'Ada', 'executive'), + 'eng-head': person('eng-head', 'Priya', 'engineering'), + worker: person('worker', 'Dana', 'engineering') + } + } +} + +function person(id: string, name: string, departmentId: string): PersonRecord { + return { + id, + name, + title: 'Person', + kind: 'worker' as const, + departmentId, + employmentState: 'active', + createdAt: '2026-01-01T00:00:00.000Z' + } +} + +describe('a hire with no departmentId lands in the caller’s own department', () => { + test('a HEAD gets the department they head', () => { + const manifest = company() + + expect(hireDefaultDepartmentForTest(manifest, manifest.people['eng-head'])).toBe('engineering') + expect(hireDefaultDepartmentForTest(manifest, manifest.people.chief)).toBe('executive') + }) + + /** + * THE OTHER ARM. A person who heads nothing still has a department — the one + * they sit in — and the resolver has always had both branches. Asserting only + * the head case would pass against a resolver that returned the headed + * department or nothing. + */ + test('a NON-head gets the department they sit in', () => { + const manifest = company() + + expect(hireDefaultDepartmentForTest(manifest, manifest.people.worker)).toBe('engineering') + }) + + test('the resolved default is a department this person may actually hire into', () => { + // The default is only worth having if it survives the scope check the hire + // then applies to it — otherwise omitting the field would trade a guess for + // a refusal. + const manifest = company() + const head = manifest.people['eng-head'] + const resolved = hireDefaultDepartmentForTest(manifest, head) + if (isNullish(resolved)) throw new Error('the default must resolve for a head') + + expect(departmentScopeDenial(manifest, head, resolved)).toBeUndefined() + }) +}) + +describe('the override path still refuses a company name', () => { + /** + * REACHABILITY, not merely behaviour. With a default in place, the refusal is + * the only thing standing behind an EXPLICIT departmentId — so this drives + * the explicit path on purpose. The fixture passes the field rather than + * omitting it, or the test would quietly become a default-path test the day + * the default landed and stop guarding anything. + */ + test('an explicit company name is still refused, and the refusal names the root id', () => { + const manifest = company() + const explicitlyPassed = manifest.slug + + expect(departmentScopeDenial(manifest, manifest.people.chief, explicitlyPassed)).toBe( + 'unknown-department' + ) + + const refusal = unknownDepartmentMessage( + manifest, + manifest.people.chief, + explicitlyPassed, + 'hire into' + ) + expect(refusal).toContain("The root department id is 'executive'") + expect(refusal).toContain('acme-capital') + expect(explicitlyPassed).not.toBe(hireDefaultDepartmentForTest(manifest, manifest.people.chief)) + }) +}) diff --git a/packages/piing/test/IntercomSeamClassification.test.ts b/packages/piing/test/IntercomSeamClassification.test.ts index 08b27be..8669a2c 100644 --- a/packages/piing/test/IntercomSeamClassification.test.ts +++ b/packages/piing/test/IntercomSeamClassification.test.ts @@ -281,10 +281,15 @@ const CLASSIFICATION: Readonly> = { // Both are B: they are about what a person is SHOWN and what an agent may // type, not about where a decision lives. recipientsForTest: 'B', + hireDefaultDepartmentForTest: 'B', // The refusal classification: which failures are the CALLER's and which are // the system's. Presentation, because the whole subject is what the card // tells a reader about whose fault it is. refusalResultForTest: 'B', + isCallerRefusalCardForTest: 'B', + showsSystemFaultTagForTest: 'B', + mailboxDeliveryForTest: 'A', + workResumeDeliveryForTest: 'A', callerRefusalForTest: 'B', messageWakeDispositionForTest: 'B', primeManifestForTest: 'B', diff --git a/packages/piing/test/QueuedDelivery.test.ts b/packages/piing/test/QueuedDelivery.test.ts index 7230471..6f1e94b 100644 --- a/packages/piing/test/QueuedDelivery.test.ts +++ b/packages/piing/test/QueuedDelivery.test.ts @@ -16,7 +16,9 @@ */ import { firstRunGateForTest, + mailboxDeliveryForTest, queuedPiDeliveryForTest, + workResumeDeliveryForTest, workResumeNeedsRedrive } from '@test-assets/organization-intercom' import { beforeEach, describe, expect, it } from 'vitest' @@ -211,3 +213,72 @@ describe('the boot gate', () => { expect(queuedPiDeliveryForTest('steer', false).deliverAs).toBe('steer') }) }) + +describe('mail rides the same queue as the operator’s own typing', () => { + /** + * The operator typing mid-turn is submitted with `streamingBehavior: "steer"` + * and consumed at the next STEP BOUNDARY inside the running turn. Mail rode + * `followUp`, which Pi consumes only when the agent has no more tool calls or + * steering messages — the END of the turn. A person mid-way through an hour + * of work therefore did not see an ordinary message until the hour was over, + * while the same words typed by the operator arrived in seconds. + * + * This asserts the RULE, not the table. The table was already pinned; which + * mode the mailbox passed into it was not, which is exactly why the timing of + * every delivery could change without one test noticing. + */ + it('a normal message on a busy pane rides the steering queue', () => { + expect(mailboxDeliveryForTest(true, false)).toEqual({ + deliverAs: 'steer', + streamingBehavior: 'steer' + }) + }) + + it('an idle pane still starts a turn, unchanged', () => { + // BEHAVIOUR-identical, not byte-identical — and the assertion below is why + // the distinction has to be made here rather than glossed. `deliverAs` DID + // change on this row, to 'steer'; it is inert whenever `triggerTurn` fires, + // because a turn that is being started has nothing to steer into. So the + // person is woken exactly as before while the shape is new, and a comment + // claiming the bytes did not move would be contradicted by the very line + // under it. This change is about WHEN a busy person reads mail, never about + // whether an idle one is woken. + const idle = mailboxDeliveryForTest(false, false) + expect(idle.triggerTurn, 'an idle pane is still started').toBe(true) + expect(idle.deliverAs).toBe('steer') + }) + + it('the work-resume prompt does NOT steer — the boundary of this change', () => { + // Sanchez's finding: "considered and kept" was a claim no test could check. + // A revert of the work-resume prompt to steer passed every test in this + // file, because the only thing pinning it was a comment saying it had been + // thought about. This asserts the boundary from the OTHER side: mail steers, + // work-resume does not, and the two now disagree in a way a test can see. + const busy = workResumeDeliveryForTest(true, false) + expect(busy.deliverAs, 'a work-resume prompt waits for the current turn').toBe('followUp') + expect(busy.streamingBehavior).toBe('followUp') + expect(busy.triggerTurn, 'a busy person is not interrupted by a resume prompt').toBeUndefined() + + // And it is genuinely a DIFFERENT answer from mail's on the same inputs, so + // the pin cannot be satisfied by reverting both sides together. + // + // THE CONTROL FOR THIS TEST FLIPS `workResumeDeliveryMode` AND NOTHING ELSE, + // and that precision is the finding rather than a detail. The first version + // of this pin had the mode literal in the product AND in the test seam — two + // independent copies — so a revert at the call site left every test green. + // The control missed it by flipping BOTH literals: the seam moved with the + // product, the test bit, and the run looked like proof. It measured the + // adjacent mutation. A control that flips the wrong set is a control that + // reports on an instrument nobody is going to use. + expect(workResumeDeliveryForTest(true, false).deliverAs).not.toBe( + mailboxDeliveryForTest(true, false).deliverAs + ) + }) + + it('the boot window still parks, unchanged', () => { + // Byte-identical to before: inside the boot window nothing is delivered as + // a stream disposition at all, and nothing starts a turn. + expect(mailboxDeliveryForTest(true, true)).toEqual({ deliverAs: 'nextTurn' }) + expect(mailboxDeliveryForTest(false, true)).toEqual({ deliverAs: 'nextTurn' }) + }) +}) diff --git a/scripts/guard-wiring-manifest.mjs b/scripts/guard-wiring-manifest.mjs index aa0f6e7..7fde89e 100644 --- a/scripts/guard-wiring-manifest.mjs +++ b/scripts/guard-wiring-manifest.mjs @@ -478,6 +478,10 @@ export const GUARD_WIRING_MANIFEST = { // --filter=./packages/*` before it shards, and `--ignore-scripts` does not // skip devDependencies. A lane of its own was written first and deleted: // it would have been a second place to keep those prerequisites correct. + // The installer's exit-code contract: a missing prerequisite may not be + // reported as success. Runs the script's own bytes against stub `pi`/`npm`, + // and carries its own control proving the assertions bite. + 'installer-pi-exit-codes.test.mjs': { status: 'wired' }, 'installed-release-extensions-load-under-pi.test.mjs': { status: 'wired' }, // The minimum Pi version, and the documents that quote it. Same shape as the // beacond port above and for the same measured reason: a compiled-in constant diff --git a/scripts/test/installer-pi-exit-codes.test.mjs b/scripts/test/installer-pi-exit-codes.test.mjs new file mode 100644 index 0000000..43da2ca --- /dev/null +++ b/scripts/test/installer-pi-exit-codes.test.mjs @@ -0,0 +1,154 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { chmodSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { test } from 'node:test' + +// WHAT THIS EXISTS FOR. +// +// install.sh refuses to report success when a prerequisite is missing, and that +// rule had no instrument at all: every Pi branch in the script was reachable +// only by running the real installer against the real network, so nothing +// checked the EXIT CODE any of them produced. The rule shipped as a comment, +// and a comment is exactly what the two `|| true` sites contradicted — the +// script printed its happy outro and exited zero with the agent runtime absent. +// +// So this runs the script's OWN BYTES. It does not re-implement the decision +// block or assert over its text; it extracts the block by marker, supplies the +// handful of names the block reads from earlier in the file, and executes it +// against stub `pi` and `npm` on a private PATH. + +const installer = fileURLToPath(new URL('../../install.sh', import.meta.url)) +const START = /^version_below\(\) \{$/m + +/** The block under test, cut from the real file at a marker rather than at a + * line number, plus the definitions it inherits from earlier in the script. + * + * It REFUSES rather than passing when the marker is gone: a guard that cannot + * find its subject has not verified the subject, and reporting that as green + * is the failure this whole file is written against. */ +function decisionBlock() { + const src = readFileSync(installer, 'utf8') + const at = src.search(START) + assert.notEqual( + at, -1, + 'CANNOT CHECK: install.sh no longer contains a `version_below() {` line, so the Pi ' + + 'decision block cannot be located. Re-derive the marker and update this guard; do not ' + + 'delete it, and do not let it pass on a file it could not read.', + ) + const tail = src.slice(at) + for (const needed of ['install_pi()', 'confirm_default_yes()', 'if ! have pi; then']) { + assert.ok( + tail.includes(needed), + `CANNOT CHECK: the extracted block is missing ${needed}; the marker no longer bounds the ` + + 'Pi section. Re-derive it rather than trusting this run.', + ) + } + return [ + 'set -eu', + 'CHIEF_HOME="$HOME/.chief"', + 'PI_INSTALL="npm install -g --ignore-scripts @earendil-works/pi-coding-agent"', + "say() { printf '%s\\n' \"$*\"; }", + "die() { printf 'chief install: %s\\n' \"$*\" >&2; exit 1; }", + 'have() { command -v "$1" >/dev/null 2>&1; }', + 'pi_floor="${TEST_PI_FLOOR:-0.80.10}"', + tail, + ].join('\n') +} + +/** Run the block with stub binaries. `pi` absent when version is null. */ +function run({ piVersion, npmSucceeds, npmInstallsVersion = '0.90.0', source = undefined }) { + const dir = mkdtempSync(join(tmpdir(), 'chief-installer-guard-')) + const bin = join(dir, 'bin') + execFileSync('mkdir', ['-p', bin]) + const piPath = join(bin, 'pi') + + const writeStub = (p, body) => { writeFileSync(p, `#!/bin/sh\n${body}\n`); chmodSync(p, 0o755) } + + if (piVersion !== null) writeStub(piPath, `echo "pi ${piVersion}"`) + // npm "installing" means it drops a pi stub at the requested version. + writeStub( + join(bin, 'npm'), + npmSucceeds + ? `cat > '${piPath}' <<'S'\n#!/bin/sh\necho "pi ${npmInstallsVersion}"\nS\nchmod 755 '${piPath}'\nexit 0` + : 'echo "npm: failed" >&2; exit 1', + ) + + const script = join(dir, 'block.sh') + writeFileSync(script, source ?? decisionBlock()) + + try { + const stdout = execFileSync('sh', [script], { + encoding: 'utf8', + // No controlling terminal: the no-tty default path, which is the one CI takes. + stdio: ['ignore', 'pipe', 'pipe'], + env: { PATH: `${bin}:/usr/bin:/bin`, HOME: dir, TEST_PI_FLOOR: '0.80.10' }, + }) + return { status: 0, output: stdout } + } catch (error) { + return { status: error.status ?? -1, output: `${error.stdout ?? ''}${error.stderr ?? ''}` } + } +} + +test('an absent Pi that fails to install does not report success', () => { + const { status, output } = run({ piVersion: null, npmSucceeds: false }) + assert.notEqual( + status, 0, + 'install.sh exited ZERO with the agent runtime absent. A caller — a CI job, a provisioning ' + + 'script — is told chief is ready when nothing can run a person. The declined-upgrade path ' + + 'already refuses to call a too-old Pi success; an absent one is strictly worse.', + ) + assert.match(output, /Pi did not install/, 'the failure must say what did not happen') + assert.match(output, /npm install -g/, 'and must name the command that fixes it') +}) + +test('an accepted upgrade that fails to install does not report success', () => { + const { status, output } = run({ piVersion: '0.70.0', npmSucceeds: false }) + assert.notEqual( + status, 0, + 'agreeing to upgrade did not make the upgrade happen: the too-old Pi the decline path ' + + 'refuses to call success is exactly what is left behind, so this cannot exit zero either.', + ) + assert.match(output, /Pi did not upgrade/) +}) + +test('an absent Pi that installs cleanly reports success', () => { + const { status, output } = run({ piVersion: null, npmSucceeds: true, npmInstallsVersion: '0.90.0' }) + assert.equal(status, 0, `the happy path must still be green:\n${output}`) + assert.match(output, /is ready/) +}) + +test('a Pi already at or above the floor is left alone, and nothing is asked', () => { + const { status, output } = run({ piVersion: '0.90.0', npmSucceeds: false }) + assert.equal(status, 0, `a satisfactory Pi must not be touched:\n${output}`) + assert.doesNotMatch(output, /\[Y\/n\]/, 'nothing should be asked about a Pi that already qualifies') +}) + +test('with no terminal the upgrade prompt takes its default and SAYS so', () => { + // The default is yes, so a working npm upgrades without a tty and without hanging. + const { status, output } = run({ piVersion: '0.70.0', npmSucceeds: true, npmInstallsVersion: '0.90.0' }) + assert.equal(status, 0, `the no-tty default must proceed, not hang or abort:\n${output}`) + assert.match( + output, /no terminal to ask on/, + 'a choice made on somebody\'s behalf must be stated, not silent', + ) +}) + +test('the exit-code assertions BITE — proven against the defect they were written for', () => { + // A pin nobody flipped is a claim, not evidence. This reverts the fix inside + // the extracted copy only (the file on disk is untouched) and asserts the + // guard above would have caught the original bug. + const reverted = decisionBlock().replace( + /\|\| die "chief itself is installed under \$CHIEF_HOME; Pi did not install[^"]*"/, + '|| true', + ) + assert.ok(reverted.includes('|| true'), 'the control could not construct the defect; re-derive it') + const { status } = run({ piVersion: null, npmSucceeds: false, source: reverted }) + assert.equal( + status, 0, + 'CONTROL FAILED: with `|| true` restored the block should exit zero, which is the bug. It did ' + + 'not, so the passing assertions above are not measuring what they claim to measure.', + ) +})