From d44788a666c54bcc7eeb8541b376ca378e0ba2ff Mon Sep 17 00:00:00 2001 From: @permadeath.com Date: Mon, 17 Aug 2026 16:14:37 +0000 Subject: [PATCH] refactor(doctor)!: split into local and remote, one knot probe `doctor` and `status tangled` answered one question — can what I am about to do work — from the two ends of it, and only the first word said which end. They are `doctor local` and `doctor remote` now, in the vocabulary git spent thirty years teaching. `doctor` is a container and runs nothing itself, which is how every other family here works. Whether a knot answers is a fact about the knot, so it is asked once, by `remote`. `local` names the knot its origin resolves to and stops there. The surviving probe is also the one that outlives an index outage: it reads the knot from the repo's DID document rather than off the repo record, whose address only Bobbin can supply. The two routes look like duplication and are not. `local`'s `remote` row is `origin` now — one word cannot mean both the git remote a row is about and the end of the wire a sibling command is about. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: Idebfd9047fe8d2a8aafa449917ec2229866b32d1 --- README.md | 79 +++++++++++++++++++++++++++++++++++++++++++------------------------------------ TODO.md | 34 +++++++++++++++++++++++++--------- docs/output.md | 20 ++++++++++---------- src/main.rs | 30 ++++++++++++++---------------- tests/exit_status.rs | 21 ++++++++++----------- src/clients/probe.rs | 2 +- src/cmd/agent_notes.txt | 10 +++++----- src/cmd/doctor.rs | 609 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------- src/cmd/status.rs | 490 ++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ src/term/checks.rs | 34 ++++++++++++++++++---------------- 10 file(s) changed, 678 insertion(s)(+), 651 deletion(s)(-) diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -48,7 +48,8 @@ ``` atgc about # info about atgc atgc agent # working notes for AI agents (humans welcome) -atgc doctor # is this machine, account and checkout set up to work? +atgc doctor local # is this machine, account and checkout set up to work? +atgc doctor remote # are Tangled's services answering, and what are they running? ``` `atgc agent` prints a briefing on what Tangled does differently from the @@ -56,14 +57,18 @@ bodies, stacks, identity. Like `completion`, it is generated from the binary that prints it, so it cannot describe features you do not have. -`atgc doctor` asks eight questions at once and prints the answers side by -side: whether the directory holding your OAuth sessions is readable by anyone -but you, which account commands here act as and whether its session is usable, -the scopes its login predates, whether a push from this machine can be +`atgc doctor` has two ends, and which end a report is about is the whole of +the difference. `local` asks whether *you* are set up, and every bad row hands +you a command to run; `remote` asks whether *they* are up, so no row is about +you and none has a remedy — the answer to a bad one is to wait. + +`atgc doctor local` asks seven questions at once and prints the answers side +by side: whether the directory holding your OAuth sessions is readable by +anyone but you, which account commands here act as and whether its session is +usable, the scopes its login predates, whether a push from this machine can be authenticated at all, what this checkout says about whose commits these are, -which repo its `origin` resolves to *and from which directory*, whether that -repo's knot answers, and — when you have opted an index in — how far behind -it has fallen. +which repo its `origin` resolves to *and from which directory*, and — when you +have opted an index in — how far behind it has fallen. ``` config dir ok /home/you/.config/atgc is 0700, and its 5 file(s) are yours alone @@ -71,7 +76,7 @@ access token valid for 47m ssh key error no SSH key is registered for @you, so a knot will refuse every push from it publish this machine's key first: atgc key add - remote ok origin → you/thing (did:plc:…) on knot1.tangled.sh + origin ok origin → you/thing (did:plc:…) on knot1.tangled.sh resolved from /home/you/src/thing (branch main) ``` @@ -79,34 +84,11 @@ rows read `n/a` and the account rows still answer, and logged out it is the other way round. It exits non-zero when a check found something *broken* rather than merely worth mentioning, carrying the status the command that -needs it would have exited with, so `atgc doctor >/dev/null || echo broken` -is a usable health check. `--json` prints the same thing as one object. +needs it would have exited with, so `atgc doctor local >/dev/null || echo +broken` is a usable health check. `--json` prints the same thing as one object. -#### Status - -``` -atgc status pr # your PRs across every repo (--author for someone else's) -atgc status tangled # are Tangled's services answering, and what are they running? -``` - -Everything else in atgc acts on the repo you are standing in; these do not. -`atgc pr list` is this repo's pull requests and `atgc status pr` is yours, -wherever you filed them — the largest difference between two listings that -otherwise print nearly the same columns, and one that was invisible while -both were verbs of `pr`. So the scope is in the first word now: `atgc pr …` -is this repo, `atgc status …` is not. - -It reads the subject's own PDS rather than an index, which is what makes it -complete and immediate for one account without a session, and it works from -a directory that is not a checkout at all. `--author` asks the same of -somebody else. The old spelling, `atgc pr status`, still works and prints -where it went. - -`atgc status tangled` is the other half of `atgc doctor`. That one asks -whether *you* are set up and hands you a command for every bad row; this one -asks whether *they* are up, so no row is about you and none has a remedy — -the answer to a bad one is to wait. It is the question left over when -`doctor` says everything is fine and the thing you were doing still fails. +`atgc doctor remote` is the other end. It is the question left over when +`local` says everything is fine and the thing you were doing still fails. ``` appview ok tangled.org answered in 1181ms @@ -125,6 +107,31 @@ outside a checkout — and the PDS row is your own, which belongs in a report about Tangled precisely because a pull request is a record in it. Needs no session; every request is a public read. + +Only `remote` probes a knot, and the two ends find one by different routes on +purpose: `local` reads it off the repo record, which only Bobbin can address, +while `remote` reads it out of the repo's own DID document. The record route +goes dark exactly when the index does, and an index outage is one of the +things you run `remote` to confirm. + +#### Status + +``` +atgc status pr # your PRs across every repo (--author for someone else's) +``` + +Everything else in atgc acts on the repo you are standing in; this does not. +`atgc pr list` is this repo's pull requests and `atgc status pr` is yours, +wherever you filed them — the largest difference between two listings that +otherwise print nearly the same columns, and one that was invisible while +both were verbs of `pr`. So the scope is in the first word now: `atgc pr …` +is this repo, `atgc status …` is not. + +It reads the subject's own PDS rather than an index, which is what makes it +complete and immediate for one account without a session, and it works from +a directory that is not a checkout at all. `--author` asks the same of +somebody else. The old spelling, `atgc pr status`, still works and prints +where it went. #### Accounts diff --git a/TODO.md b/TODO.md --- a/TODO.md +++ b/TODO.md @@ -1631,9 +1631,25 @@ - [x] `status tangled` — whether Tangled's services are answering, and what they are running. The other half of `doctor`, which asks whether *you* are set up and hands you a command per bad row; this asks whether - *they* are up, so no row is about you and none has a remedy. It is also - the question `doctor` structurally cannot answer, every network row it - has being scoped to the repo you are standing in + *they* are up, so no row is about you and none has a remedy +- [x] Both halves under one word, as `doctor local` and `doctor remote`. The + pair was two top-level commands answering one question — "can what I am + about to do work" — from the two ends of it, and only the first word + said which end. `local`/`remote` is the vocabulary a reader arrives + with, git having spent thirty years teaching it. `doctor` is a container + now and runs nothing itself, which is how every other family here works; + being both a verb and a parent would have made it the only one +- [x] One knot probe, and it is `remote`'s. Whether a knot answers is a fact + about the knot, so `local` names the knot its origin resolves to and + stops there — the row that judged it was never local. The `remote` row + also survives an index outage, which the one it replaced could not: it + reads the knot out of the repo's DID document rather than off the repo + record, whose address only Bobbin can supply +- [x] `doctor`'s `remote` row renamed `origin`. One word cannot mean both the + git remote a row is about and the end of the wire a sibling command is + about, least of all in a tool whose readers arrive knowing the first + sense. `origin` is the more honest name anyway: the row is about that + one remote and says so on every line - [x] Each service asked the question atgc's own use of it depends on — the appview's site, Bobbin's search, `sh.tangled.knot.version`, the PDS's `_health` — rather than one health endpoint for all four. Tangled's @@ -1651,13 +1667,13 @@ addition — its 76-line budget was already spent, which is the point of the number. The scope rule earns a "details that surprise" line because `gh pr status` is the muscle memory an agent arrives with and - it means something else here; `status tangled` rides in the `doctor` - paragraph, being its other half. What went is the `logs git` line, the + it means something else here; the services half rides in the `doctor` + paragraph, being its other end. What went is the `logs git` line, the one fact in the file that `atgc logs --help` answers as well -- [ ] `status tangled` says nothing about how far behind the index is, which +- [ ] `doctor remote` says nothing about how far behind the index is, which is the other half of "is Bobbin working" and the failure that is silent - rather than loud. `doctor`'s `index` row measures it against your PDS - for one repo; a service-wide measure wants a newest-indexed-record + rather than loud. `doctor local`'s `index` row measures it against your + PDS for one repo; a service-wide measure wants a newest-indexed-record timestamp, and `sh.tangled.search.query` ranks by relevance with no way to ask for the newest - [x] `pr status` kept as a hidden alias that prints the new spelling. The @@ -2211,7 +2227,7 @@ itself: it claimed nothing produced `4` or `7` while also saying the sites "already hold the status when they refuse". Driven against the real binary, both were already reachable before this branch. `4`: - `atgc doctor` exits it on a scope gap or an unregistered push key, + `atgc doctor local` exits it on a scope gap or an unregistered push key, `repo default-branch` on a repo you do not own exits it through `repo/mod.rs`'s `own_repo`, and `tests/issue_flows.rs` had been pinning it since the issue family was classified. `7`: hold diff --git a/docs/output.md b/docs/output.md --- a/docs/output.md +++ b/docs/output.md @@ -51,19 +51,19 @@ The numbers are a public interface: `crate::exit::Exit` spells them out as a match so that inserting a variant cannot renumber the rest. -`atgc doctor` and `atgc status tangled` are the two commands with an answer -*and* a non-zero status. The report is the answer, so it goes on stdout, and +`atgc doctor local` and `atgc doctor remote` are the two reports with an +answer *and* a non-zero status. The report is the answer, so it goes on stdout, and the status summarises the report rather than standing in for it: each failing row carries the status the operation it stands for would have exited with — a missing session is `3`, a scope gap or an unregistered push key `4`, a host that did not answer `6` — and with more than one, the first row wins. A -warning never decides it, or `doctor` would fail in every checkout that has -never been near Tangled. +warning never decides it, or `doctor local` would fail in every checkout that +has never been near Tangled. The two ask opposite questions and only one of them can say what to do about -the answer. `doctor` orders its rows by what depends on what, so the first -broken row is the one to fix first and it says so; `status tangled` reports on -four services that need nothing from each other, so it names the row the -status came from and stops there. That difference is why every `remedy` in a -`status tangled` report is `null`: there is no command here that mends -somebody else's host. +the answer. `doctor local` orders its rows by what depends on what, so the +first broken row is the one to fix first and it says so; `doctor remote` +reports on four services that need nothing from each other, so it names the +row the status came from and stops there. That difference is why every +`remedy` in a `doctor remote` report is `null`: there is no command here that +mends somebody else's host. diff --git a/src/main.rs b/src/main.rs --- a/src/main.rs +++ b/src/main.rs @@ -207,22 +207,20 @@ /// --input sync.json --dry-run #[command(verbatim_doc_comment)] Api(cmd::api::ApiArgs), - /// Check that this machine, account and checkout are set up to work + /// Check whether this end and the other end are working /// - /// Eight questions asked at once, with nothing else going on: who else on - /// this machine can read the stored credentials, which account commands - /// here act as and whether its session is usable, the scopes its login - /// predates, whether a push from this machine can be authenticated at all, - /// what this checkout says about whose commits these are, which repo its - /// `origin` resolves to and from where, whether that repo's knot answers, - /// and how far behind the appview's index is. + /// Two reports, split by which end they are about. `doctor local` asks + /// whether *you* are set up — this machine, this account, this checkout — + /// and every bad row carries a command you can run. `doctor remote` asks + /// whether *they* are up, so no row is about you and none has a remedy. /// - /// Every row works when the others do not. Outside a checkout the - /// repo-scoped rows read `n/a` and the rest still answer; logged out it is - /// the other way round. Exits non-zero when a check found something - /// broken — a warning is not — carrying the status the command that - /// needs it would have exited with. - Doctor(cmd::doctor::DoctorArgs), + /// `remote` is the question left over when `local` says everything is fine + /// and the thing you were doing still fails. Both exit non-zero when a + /// check found something broken — a warning is not. + Doctor { + #[command(subcommand)] + command: cmd::doctor::Command, + }, /// Work with issues /// /// A Tangled issue is a record in the PDS of whoever filed it, naming the @@ -258,7 +256,7 @@ }, /// Report on things that are not one repo /// - /// The rest of atgc acts on the repo you are standing in. These do not: + /// The rest of atgc acts on the repo you are standing in. This does not: /// `status pr` is your pull requests wherever you filed them, however /// many repos that spans, and it works from a directory that is not a /// checkout at all. @@ -487,7 +485,7 @@ Command::Repo { command } => cmd::repo::run(command).await, Command::Report(args) => cmd::report::report(args).await, Command::Key { command } => cmd::key::run(command).await, - Command::Doctor(args) => cmd::doctor::doctor(args).await, + Command::Doctor { command } => cmd::doctor::run(command).await, } } diff --git a/tests/exit_status.rs b/tests/exit_status.rs --- a/tests/exit_status.rs +++ b/tests/exit_status.rs @@ -267,8 +267,8 @@ let _ = std::fs::remove_dir_all(&dir); } -/// `atgc doctor` is the command somebody runs *because* something is wrong, -/// so the state it has to survive is the worst one available: no config +/// `atgc doctor local` is the command somebody runs *because* something is +/// wrong, so the state it has to survive is the worst one available: no config /// directory, no session, and no checkout. It must print a report and exit on /// it — not crash, not refuse, and not ask for any of the things that are /// missing. @@ -276,14 +276,14 @@ /// That state is also the only one this can be run in hermetically, and the /// two facts are the same fact. With no account there is no PDS to look up /// and no keys to match; outside a checkout there is no remote to resolve, no -/// knot to reach and no index to compare against. Every row that would open a +/// index to compare against. Every row that would open a /// socket is a row whose subject is absent, so the whole command runs without /// one — which is what lets it be tested at this level at all, and is exactly /// the case worth pinning. #[test] fn doctor_outside_a_checkout_with_no_session_reports_rather_than_failing() { let dir = outside_a_checkout("doctor-report"); - let out = atgc_in(&dir, &["doctor"]); + let out = atgc_in(&dir, &["doctor", "local"]); let _ = std::fs::remove_dir_all(&dir); assert_eq!( @@ -300,8 +300,7 @@ "scopes", "ssh key", "git identity", - "remote", - "knot", + "origin", "index", ] { assert!(report.contains(row), "no `{row}` row in:\n{report}"); @@ -310,13 +309,13 @@ report.contains("no account is logged in"), "the session row is the finding:\n{report}" ); - // Seven of the eight have no subject here, and each says so instead of + // Six of the seven have no subject here, and each says so instead of // erroring — the whole reason this command is usable when things are // broken. `config dir` is among them: `HOME` points nowhere, so there is // no directory whose permissions could be read. assert_eq!( report.matches("n/a").count(), - 7, + 6, "every row but the session should be skipped:\n{report}" ); assert!( @@ -329,7 +328,7 @@ // because here the status summarizes the answer rather than replacing it. let stderr = String::from_utf8_lossy(&out.stderr); assert!( - stderr.contains("error: doctor found a problem: session"), + stderr.contains("error: doctor local found a problem: session"), "{stderr}" ); } @@ -342,7 +341,7 @@ #[test] fn doctor_json_is_one_parseable_object_even_when_it_fails() { let dir = outside_a_checkout("doctor-json"); - let out = atgc_in(&dir, &["doctor", "--json"]); + let out = atgc_in(&dir, &["doctor", "local", "--json"]); let _ = std::fs::remove_dir_all(&dir); assert_eq!(code(&out), Some(3)); @@ -352,7 +351,7 @@ serde_json::from_str(&stdout).unwrap_or_else(|e| panic!("not JSON ({e}):\n{stdout}")); assert_eq!(value["ok"], false); let checks = value["checks"].as_array().expect("a checks array"); - assert_eq!(checks.len(), 8, "{stdout}"); + assert_eq!(checks.len(), 7, "{stdout}"); assert_eq!(checks[0]["name"], "config dir"); assert_eq!(checks[1]["name"], "session"); assert_eq!(checks[1]["status"], "error"); diff --git a/src/clients/probe.rs b/src/clients/probe.rs --- a/src/clients/probe.rs +++ b/src/clients/probe.rs @@ -6,7 +6,7 @@ //! may be a refusal, a body that may not be JSON, and neither of those being //! a failure. Only a host that never answered at all is an `Err`. //! -//! That inversion is the whole module. `atgc status tangled` exists to tell +//! That inversion is the whole module. `atgc doctor remote` exists to tell //! "the service is down" apart from "the service is up and said no", and a //! client that turns the second into an error has thrown the answer away //! before the command can read it. diff --git a/src/cmd/agent_notes.txt b/src/cmd/agent_notes.txt --- a/src/cmd/agent_notes.txt +++ b/src/cmd/agent_notes.txt @@ -11,11 +11,11 @@ record; opening either needs no permission on the repo. Machine, account and checkout - doctor: eight checks on you -- account and session, the scopes that - login predates, push auth, git identity, origin, knot, index; a bad - row exits with the status that failure earns. status tangled is the - other half: whether their services answer at all. - atgc doctor # the first move in a new checkout + doctor has two ends, and a bad row exits the status that failure + earns. local: seven checks on you -- session, the scopes a login + predates, push auth, git identity, origin, index. remote: theirs. + atgc doctor local # the first move in a new checkout + atgc doctor remote # when local is clean and it still fails atgc auth login # when there is no session yet Pull requests diff --git a/src/cmd/doctor.rs b/src/cmd/doctor.rs --- a/src/cmd/doctor.rs +++ b/src/cmd/doctor.rs @@ -1,18 +1,42 @@ -//! `atgc doctor` — whether this machine, this account and this checkout are in -//! a state where atgc will work. +//! `atgc doctor` — whether the thing you are about to do can work, asked of +//! each end of it in turn. //! -//! Every fact here is already computed somewhere else in the tree, by the +//! Two reports, and what splits them is which end they are about. `doctor +//! local` asks whether *you* are set up — this machine, this account, this +//! checkout — and every bad row it prints carries a command you can run. +//! `doctor remote` asks whether *they* are up, so no row is about you and none +//! has a remedy: the answer to a bad one is to wait. That is the question left +//! over when `local` says everything is fine and the thing you were doing +//! still fails. +//! +//! Every fact in `local` is already computed somewhere else in the tree, by the //! command that needs it, at the moment it needs it. That is the problem this //! command exists for: the tool tells you a push was refused, or a write got a //! 403, or a listing came back short, at the moment you were trying to do -//! something else, and each of those answers is one eighth of a diagnosis. -//! `doctor` asks all eight questions with nothing else going on and prints the -//! answers side by side. +//! something else, and each of those answers is one seventh of a diagnosis. +//! `doctor local` asks all seven questions with nothing else going on and +//! prints the answers side by side. //! //! One row is not like the others: `config dir` asks who else on this machine //! can read the credentials, which is not a question any command answers in //! passing, because every command works perfectly well when the answer is //! "everyone". See [`config_check`]. +//! +//! # One knot probe, and it belongs to `remote` +//! +//! Whether a knot answers is a fact about the knot, so it is asked once, by +//! `doctor remote`. `local` still *names* the knot its origin resolves to, +//! because which host this checkout points at is a fact about this checkout — +//! but it no longer judges whether that host is up, which was that row's real +//! subject and was never local at all. +//! +//! The two halves find a knot by different routes, and the difference is load +//! bearing. `local` reads it off the `sh.tangled.repo` record, which sits in +//! the owner's PDS at an address only Bobbin can supply. `remote` reads it out +//! of the repo's own DID document, which the knot mints and which answers +//! whoever asks. The record route goes dark exactly when the index does — and +//! an index outage is one of the things somebody runs `doctor remote` to +//! confirm. They look like duplication and are not: do not unify them. //! //! So it holds no logic of its own. [`crate::clients::tangled::scope`] decides //! what a scope gap is, [`crate::clients::git::ssh::PushKey`] decides whether a @@ -55,6 +79,7 @@ use crate::clients::git::config as gitconfig; use crate::clients::git::run as git; use crate::clients::git::ssh; +use crate::clients::probe; use crate::clients::tangled::{bobbin, resolve, scope}; use crate::config::account; use crate::exit::Exit; @@ -63,8 +88,78 @@ // See [`crate::term::checks`]. use crate::term::checks::{Check, CheckJson, Status, exit_status, render, summary}; +/// The two ends `doctor` can be pointed at. +#[derive(clap::Subcommand, Debug)] +pub(crate) enum Command { + /// Check that this machine, account and checkout are set up to work + /// + /// Seven questions asked at once, with nothing else going on: whether the + /// directory holding your OAuth sessions is readable by anyone but you, + /// which account commands here act as and whether its session is usable, + /// the scopes its login predates, whether a push from this machine can be + /// authenticated at all, what this checkout says about whose commits these + /// are, which repo its `origin` resolves to and from which directory, and + /// — when you have opted an index in — how far behind it has fallen. + /// + /// Every row works when the others do not. Outside a checkout the + /// repo-scoped rows read `n/a` and the account rows still answer; logged + /// out it is the other way round. A bad row carries a command you can run. + /// + /// It exits non-zero when a check found something *broken* rather than + /// merely worth mentioning, carrying the status the command that needs it + /// would have exited with, so `atgc doctor local >/dev/null || echo + /// broken` is a usable health check. + /// + /// Examples: + /// atgc doctor local + /// atgc doctor local --json | jq -r '.checks[] | select(.status != "ok")' + #[command(verbatim_doc_comment)] + Local(LocalArgs), + /// Check whether Tangled's services are answering, and what they run + /// + /// The other end from `doctor local`. That one asks whether *you* are set + /// up and hands you a command for every bad row; this one asks whether + /// *they* are up, so no row is about you and none has a remedy. + /// + /// Four rows, each asked the question atgc's own use of that service + /// depends on rather than a health endpoint invented for the purpose: + /// + /// appview tangled.org, which serves every `view:` link and the + /// pull numbers atgc cannot get anywhere else + /// bobbin api.tangled.org, the index behind `--source bobbin` and + /// `atgc search` + /// knot the git host this checkout's repo names, asked for its + /// version and the protocol capabilities it declares + /// pds your own PDS — not Tangled's, and the reason it belongs + /// here: a pull request is a record in it, so it is as much + /// a part of "is Tangled working" as the appview is + /// + /// A row goes `n/a` when there is nothing to ask: no knot outside a + /// checkout unless `--knot` names one, no PDS with no account selected. + /// A host that never answered is an error and exits 6, the status that + /// means retry unchanged; a host that answered a refusal is a warning, + /// because a service that refuses is a service that is running. + /// + /// Needs no session and no checkout. Every request is a public read. + /// + /// Examples: + /// atgc doctor remote + /// atgc doctor remote --knot knot1.tangled.sh + /// atgc doctor remote --json | jq -r '.checks[] | "\(.name) \(.status)"' + #[command(verbatim_doc_comment)] + Remote(RemoteArgs), +} + +/// Run whichever end was named. +pub(crate) async fn run(command: Command) -> Result<()> { + match command { + Command::Local(args) => local(args).await, + Command::Remote(args) => remote(args).await, + } +} + #[derive(clap::Args, Debug)] -pub struct DoctorArgs { +pub struct LocalArgs { /// Print one JSON object instead of the report: every check with its /// status, plus what the repo-scoped rows resolved to #[arg(long)] @@ -571,14 +666,21 @@ /// exits non-zero in all of them is a `doctor` nobody runs. The exception is /// a lookup that failed because a host never answered, which is a real /// finding about this machine and keeps its status. -fn remote_check( +/// +/// The row is called `origin` and not `remote`, which is what it printed while +/// it was the only report here. `doctor remote` is now a sibling command about +/// the other end entirely, and one word cannot mean both — least of all in a +/// tool where a reader arrives already knowing what a git remote is. `origin` +/// is also the more honest name: this row is about that one remote and no +/// other, and it says so on every line it prints. +fn origin_check( root: Option<&Path>, branch: Option<&str>, remote: &Result, facts: &Option>, ) -> Check { let Some(root) = root else { - return Check::na("remote", "not in a git checkout"); + return Check::na("origin", "not in a git checkout"); }; let where_from = format!( "resolved from {} (branch {})", @@ -587,19 +689,19 @@ ); let Ok(url) = remote else { return Check::na( - "remote", + "origin", format!("this checkout has no `origin` remote\n{where_from}"), ); }; match facts { - None => Check::na("remote", format!("origin is {url}\n{where_from}")), + None => Check::na("origin", format!("origin is {url}\n{where_from}")), Some(Err(e)) if crate::exit::classify(e) == Exit::Unreachable => Check::bad( - "remote", + "origin", Exit::Unreachable, format!("origin is {url}, and the host did not answer\n{where_from}"), ), Some(Err(_)) => Check::na( - "remote", + "origin", format!("origin is {url}, which names no Tangled repo\n{where_from}"), ), Some(Ok(facts)) => { @@ -622,32 +724,8 @@ facts.did ), }; - Check::ok("remote", format!("{named}\n{where_from}")) + Check::ok("origin", format!("{named}\n{where_from}")) } - } -} - -/// Whether the knot hosting this repo's git data answers. -/// -/// One unauthenticated query, through the same reader `repo view` uses, and -/// aimed at the knot named on the repo record rather than at `api.tangled.org` -/// — which will proxy the method and rate-limits unauthenticated proxy traffic -/// hard enough to answer a health check with a refusal. The default branch -/// comes back for free and is worth printing: it is what a pull request -/// targets by default. -async fn knot_check(facts: Option<&RepoFacts>) -> Check { - let Some(facts) = facts else { - return Check::na("knot", "no Tangled repo here to find a knot for"); - }; - let Some(knot) = facts.knot.as_deref() else { - return Check::na("knot", "nothing here names a knot for this repo"); - }; - match crate::cmd::repo::knot_query(knot, "sh.tangled.repo.getDefaultBranch", &facts.did).await { - Ok(body) => { - let default = body["name"].as_str().unwrap_or("?"); - Check::ok("knot", format!("{knot} answered; default branch {default}")) - } - Err(e) => Check::bad("knot", crate::exit::classify(&e), format!("{e:#}")), } } @@ -782,8 +860,8 @@ // Running them // --------------------------------------------------------------------------- -/// Run every check and print the report. -pub async fn doctor(args: DoctorArgs) -> Result<()> { +/// Run every check about this end and print the report. +pub async fn local(args: LocalArgs) -> Result<()> { crate::term::jsonout::init(args.json); let here = Path::new("."); @@ -819,8 +897,8 @@ }); let resolved = facts.as_ref().and_then(|f| f.as_ref().ok()); - // And the two that depend on the repo the first pass resolved. - let (knot, index) = tokio::join!(knot_check(resolved), index_check(acting, resolved)); + // And the one that depends on the repo the first pass resolved. + let index = index_check(acting, resolved).await; // Report order is dependency order: see `exit_status`, which reads the // first broken row and nothing else. `config dir` heads it because the @@ -832,8 +910,7 @@ scopes_check(acting), key, identity_check(root.as_deref(), acting), - remote_check(root.as_deref(), branch.as_deref(), &remote, &facts), - knot, + origin_check(root.as_deref(), branch.as_deref(), &remote, &facts), index, ]; @@ -864,8 +941,289 @@ // docs for why this one command has both an answer and a non-zero exit. Err(crate::exit::fail( exit, - summary(&checks, "doctor", Some("which is the one to fix first")), + summary( + &checks, + "doctor local", + Some("which is the one to fix first"), + ), )) +} + +// --------------------------------------------------------------------------- +// doctor remote +// --------------------------------------------------------------------------- + +#[derive(clap::Args, Debug)] +pub(crate) struct RemoteArgs { + /// Ask this knot, rather than the one this checkout's repo names + #[arg(long, value_name = "HOST")] + pub knot: Option, + /// Print one JSON object instead of the report: every service with its + /// status, plus which host each row was asked + #[arg(long)] + pub json: bool, +} + +/// The Bobbin query the `bobbin` row is a probe of. +/// +/// `sh.tangled.search.query` and not a health endpoint, for the reason the +/// command's help gives: what is worth knowing is whether the method atgc +/// depends on answers, and Tangled's `_health` route is absent from the +/// build running in production, so asking it would report a 404 for every +/// service in every state. +/// +/// The term is chosen to match nothing. A probe wants the query path +/// exercised, not a page of results carried across the network, and a +/// recognisable string is a courtesy to anyone reading Bobbin's own logs +/// wondering what keeps asking. +const BOBBIN_PROBE: &str = "sh.tangled.search.query?q=atgc-doctor-probe&limit=1"; + +/// The knot method the `knot` row is a probe of: public, cheap, and the only +/// one that answers with something worth printing even when all is well — +/// the knot's version, and the protocol capabilities it declares. A knot too +/// old to declare them omits the field, which its own lexicon says to read as +/// legacy rather than as broken. +const KNOT_PROBE: &str = "sh.tangled.knot.version"; + +/// Which host each row was asked, as `--json` reports it. +/// +/// The counterpart of `doctor local`'s subject block and there for the same +/// reason: a report that says `bobbin: ok` without saying which Bobbin is a +/// report that cannot be told apart from one taken against a local instance. +/// Every one of these moves — see [`crate::clients::endpoints`] — and two of +/// them are read out of a checkout and a session rather than compiled in. +#[derive(serde::Serialize, Debug, PartialEq)] +pub(crate) struct RemoteSubjectJson { + pub appview: String, + pub bobbin: String, + /// The knot named by `--knot`, or the one this checkout's repo's DID + /// document names. `null` when neither said. + pub knot: Option, + /// The acting account's PDS, `null` with no account selected. An + /// authority like the two above it and not the endpoint URL the DID + /// document carries: one object naming its four hosts two different ways + /// is one a caller has to special-case. + pub pds: Option, + pub account_did: Option, +} + +/// `doctor remote --json`'s whole object. Deliberately `doctor local --json`'s +/// shape — `ok`, a subject block, then the rows — because a caller that +/// learned to read one half's report should not have to learn the other's. +#[derive(serde::Serialize, Debug, PartialEq)] +pub(crate) struct RemoteJson { + /// Whether the process is about to exit `0`. + pub ok: bool, + pub subject: RemoteSubjectJson, + /// Every service, in report order. + pub checks: Vec, +} + +/// Ask each service whether it is there, and print what came back. +pub(crate) async fn remote(args: RemoteArgs) -> Result<()> { + crate::term::jsonout::init(args.json); + + let appview = crate::clients::endpoints::appview(); + let bobbin_base = crate::clients::endpoints::bobbin(); + + // Four things at once: the two compiled-in services, and the two lookups + // that decide whether there is a knot and a PDS to ask at all. None of + // them can inform another, and doing them in turn would cost four + // timeouts on a machine with no network instead of one — the same reason + // `local` overlaps its two chains. + let (appview_row, bobbin_row, knot_host, acting) = tokio::join!( + service_row("appview", host_of(&appview), format!("{appview}/")), + service_row( + "bobbin", + host_of(&bobbin_base), + format!("{bobbin_base}/xrpc/{BOBBIN_PROBE}") + ), + knot_here(args.knot.clone()), + acting_account(), + ); + + // And the two that could not be addressed until those resolved. + let pds_host = match &acting { + Some(did) => did::pds_from_did_doc(did).await, + None => None, + }; + let (knot_row, pds_row) = tokio::join!( + async { + match &knot_host { + Some(host) => { + let url = format!( + "{}/xrpc/{KNOT_PROBE}", + crate::clients::endpoints::knot(host) + ); + service_row("knot", host.clone(), url).await + } + None => Check::na( + "knot", + "no Tangled repo here to name a knot, and --knot named none", + ), + } + }, + async { + match &pds_host { + Some(endpoint) => { + service_row("pds", host_of(endpoint), format!("{endpoint}/xrpc/_health")).await + } + None => Check::na( + "pds", + "no account is selected, so there is no PDS to ask (atgc auth login)", + ), + } + }, + ); + + // Report order is widest blast radius first: the appview and the index + // serve every repo, a knot serves one, and your PDS serves you. That is + // not a dependency chain — none of the four needs another — so unlike + // `local` this report makes no claim about which to deal with first. + let checks = vec![appview_row, bobbin_row, knot_row, pds_row]; + + let exit = exit_status(&checks); + if args.json { + crate::term::jsonout::emit(&RemoteJson { + ok: exit == Exit::Ok, + subject: RemoteSubjectJson { + appview: host_of(&appview), + bobbin: host_of(&bobbin_base), + knot: knot_host, + pds: pds_host.as_deref().map(host_of), + account_did: acting, + }, + checks: checks.iter().map(CheckJson::of).collect(), + })?; + } else { + print!("{}", render(&checks)); + } + + if exit == Exit::Ok { + return Ok(()); + } + // The report is on stdout and this is the line that carries the status, + // exactly as `local` does it — see this module's documentation for why a + // report may answer and still exit non-zero. + Err(crate::exit::fail( + exit, + summary(&checks, "doctor remote", None), + )) +} + +/// One service, asked one question, as a row. +async fn service_row(name: &'static str, host: String, url: String) -> Check { + verdict(name, &host, probe::get(&url).await) +} + +/// What one answer is worth, with no request in sight. +/// +/// Pure, and split from [`service_row`] for the reason +/// [`crate::docs::testing`] gives: the request is four lines of plumbing and +/// this is the whole of the judgement. +/// +/// The three outcomes are the point of the report and are not the three a +/// client would give you. A host that never answered is broken and carries the +/// retry-unchanged status; a host that answered *badly* is running, and a +/// service that is running is not an outage even when it is useless to you; +/// and a host that answered is asked what it is, because a version and a +/// capability list cost nothing extra and are the two things you cannot find +/// out any other way. +fn verdict(name: &'static str, host: &str, answer: Result) -> Check { + let answer = match answer { + Ok(answer) => answer, + Err(err) => { + return Check::bad( + name, + crate::exit::classify(&err), + format!("{host} did not answer\n{err:#}"), + ); + } + }; + + let took = format!("in {}ms", answer.elapsed.as_millis()); + let status = answer.status; + + if status.is_server_error() { + // The tree's one status table, which makes exactly the split this + // needs: a gateway that never reached the service behind it is worth + // retrying unchanged, and any other 5xx is a service broken in a way + // waiting may not mend. Only 5xx reaches here, so the authenticated + // and public readings of the table agree. + return Check::bad( + name, + crate::exit::from_status(status), + format!( + "{host} answered {status} {took}\nit is reachable and the service behind it is not serving" + ), + ); + } + if !status.is_success() { + return Check::warn( + name, + format!( + "{host} answered {status} {took}\nit is running, but did not serve what atgc asked it for" + ), + ); + } + + let mut said = format!("{host} answered {took}"); + if let Some(version) = answer.version() { + said.push_str(&format!(", running {version}")); + } + // Only a knot declares these today. Reading the field on every row rather + // than only on that one costs a branch nobody has to maintain, and the + // day a second service declares capabilities the row will print them. + let capabilities = answer.strings("capabilities"); + if !capabilities.is_empty() { + said.push_str(&format!("\ncapabilities: {}", capabilities.join(", "))); + } + Check::ok(name, said) +} + +/// The knot to ask about: the one named, or the one this checkout's repo +/// belongs to. +/// +/// Read from the repo's *own DID document* rather than from its +/// `sh.tangled.repo` record, which is how `local` finds the knot it names. The +/// record sits in the owner's PDS at an address only Bobbin can supply, so that +/// route goes dark exactly when the index does — and an index outage is one of +/// the things somebody runs this half to confirm. The document is minted by the +/// knot and answers whoever asks. See this module's header: the two routes look +/// like duplication and are not. +/// +/// Every step is allowed to come up empty. Standing outside a checkout, in +/// somebody else's forge, or in a Tangled repo whose document names no knot +/// are all `n/a` rather than failures: this half must work when everything is +/// wrong, the same rule `local` holds itself to. +async fn knot_here(named: Option) -> Option { + if let Some(host) = named { + return Some(host); + } + let remote = git::remote_url("origin").ok()?; + let repo = resolve::repo_ref(&remote).await.ok()?; + did::knot_from_did_doc(&repo.did).await +} + +/// The DID of the account atgc would act as, or `None` when there is none. +/// +/// A failed selection is not an error here. Logged out is a legitimate way to +/// run this — every other row is a public read — and it makes exactly one row +/// `n/a`. +async fn acting_account() -> Option { + account::select().await.ok().map(|selection| selection.did) +} + +/// The authority of a base URL, for the report's left-hand sentence. +/// +/// `https://api.tangled.org` reads as `api.tangled.org`, and a local instance +/// at `http://127.0.0.1:8080` keeps its port, because the port is what tells +/// two of them apart. +fn host_of(base: &str) -> String { + base.split_once("://") + .map_or(base, |(_scheme, rest)| rest) + .trim_end_matches('/') + .to_string() } #[cfg(test)] @@ -875,11 +1233,15 @@ #[cfg(unix)] use std::fs::Permissions; - /// The shared summary, as `doctor` calls it. These tests are about what - /// this command promises — the rows are its eight and the wording names + /// The shared summary, as `doctor local` calls it. These tests are about + /// what that half promises — the rows are its seven and the wording names /// it — so the subject is bound here rather than repeated in each body. fn summary_of(checks: &[Check]) -> String { - summary(checks, "doctor", Some("which is the one to fix first")) + summary( + checks, + "doctor local", + Some("which is the one to fix first"), + ) } fn checks(statuses: &[Status]) -> Vec { @@ -894,8 +1256,7 @@ "scopes", "ssh key", "git identity", - "remote", - "knot", + "origin", "index", ][i], *status, @@ -913,19 +1274,18 @@ /// could not then be trusted when it did. #[test] fn warnings_and_skips_are_not_failures() { - assert_eq!(exit_status(&checks(&[Status::Ok; 8])), Exit::Ok); + assert_eq!(exit_status(&checks(&[Status::Ok; 7])), Exit::Ok); assert_eq!( exit_status(&checks(&[ // `config dir`, which is warn-only by construction: a // readable credential store is a confidentiality problem and // not a broken install, and exiting non-zero over one would - // cost `doctor >/dev/null || echo broken` its meaning. + // cost `doctor local >/dev/null || echo broken` its meaning. Status::Warn, Status::Ok, Status::Na, Status::Warn, Status::Warn, - Status::Na, Status::Na, Status::Warn, ])), @@ -952,7 +1312,6 @@ Status::Warn, Status::Ok, Status::Bad(Exit::Unreachable), - Status::Na, ])), Exit::NoSession ); @@ -966,7 +1325,6 @@ Status::Warn, Status::Na, Status::Bad(Exit::Unreachable), - Status::Na, ])), Exit::Unreachable ); @@ -987,9 +1345,8 @@ Status::Warn, Status::Ok, Status::Ok, - Status::Ok, ])); - assert_eq!(one, "doctor found a problem: scopes"); + assert_eq!(one, "doctor local found a problem: scopes"); let several = summary_of(&checks(&[ Status::Ok, @@ -999,9 +1356,8 @@ Status::Ok, Status::Ok, Status::Ok, - Status::Ok, ])); - assert!(several.starts_with("doctor found 3 problems: session, scopes, ssh key")); + assert!(several.starts_with("doctor local found 3 problems: session, scopes, ssh key")); assert!( several.contains("the exit status is session's"), "{several}" @@ -1011,8 +1367,8 @@ // never reaches this at all — and if it somehow did, it must not // claim a row that is fine. assert_eq!( - summary_of(&checks(&[Status::Warn; 8])), - "doctor found a problem" + summary_of(&checks(&[Status::Warn; 7])), + "doctor local found a problem" ); } @@ -1215,5 +1571,136 @@ seconds_between("not a time", "2026-08-13T15:03:00.000Z"), None ); + } + + // ----------------------------------------------------------------------- + // doctor remote + // ----------------------------------------------------------------------- + + fn answered(code: u16, body: serde_json::Value) -> Result { + Ok(probe::Answer { + status: reqwest::StatusCode::from_u16(code).expect("a real status code"), + elapsed: std::time::Duration::from_millis(7), + body: Some(body), + }) + } + + /// The distinction the whole half exists for: a host that is silent is + /// broken, and a host that answers a refusal is not. + /// + /// It is the difference between "wait, it is them" and "it is up, and + /// this particular thing is wrong", and getting it backwards makes the + /// report worse than useless — a `warn` on an outage is a report somebody + /// acts on by rechecking their own machine, which is where they came + /// from. + #[test] + fn a_refusal_is_a_running_service_and_silence_is_not() { + // 2xx: up, and answering what was asked. + let up = verdict( + "bobbin", + "api.tangled.org", + answered(200, serde_json::json!({"hits": []})), + ); + assert_eq!(up.status, Status::Ok); + + // 4xx: still up. A method it does not implement, a parameter it did + // not like, a rate limit — none of those is an outage. + for code in [400, 404, 429] { + let refused = verdict( + "bobbin", + "api.tangled.org", + answered(code, serde_json::json!({})), + ); + assert_eq!(refused.status, Status::Warn, "{code} is not an outage"); + assert!(refused.detail.contains("is running"), "{}", refused.detail); + } + + // 5xx: the address answers and the service behind it does not. A + // gateway failure is the retry-unchanged status; anything else is + // unclassified, because waiting may not mend it. + assert_eq!( + verdict( + "appview", + "tangled.org", + answered(503, serde_json::json!({})) + ) + .status, + Status::Bad(Exit::Unreachable) + ); + assert_eq!( + verdict( + "appview", + "tangled.org", + answered(500, serde_json::json!({})) + ) + .status, + Status::Bad(Exit::Failure) + ); + + // Nothing at all. `classify` reads the status off the error, so an + // ordinary one lands on the unclassified code rather than claiming a + // connect failure this test cannot manufacture. + let silent = verdict( + "knot", + "knot1.tangled.sh", + Err(anyhow::anyhow!("connection refused")), + ); + assert_eq!(silent.status, Status::Bad(Exit::Failure)); + assert!(silent.detail.starts_with("knot1.tangled.sh did not answer")); + + // No row here has a remedy, ever: there is no command that mends + // somebody else's host, and advice that cannot work is worse than + // none. This is the whole of what separates the two halves. + for row in [up, silent] { + assert_eq!(row.remedy, None); + } + } + + /// A healthy row says what it reached, how long it took, and what the + /// service says it is — the last being the part no other command can tell + /// you, and the reason the probe reads a body it does not need. + #[test] + fn a_healthy_row_carries_the_version_and_capabilities_when_offered() { + let knot = verdict( + "knot", + "knot1.tangled.sh", + answered( + 200, + serde_json::json!({ + "version": "v1.15.0", + "capabilities": ["knot-acl", "repo-did-input"], + }), + ), + ); + assert_eq!(knot.status, Status::Ok); + assert!(knot.detail.contains("knot1.tangled.sh answered in 7ms")); + assert!(knot.detail.contains("running v1.15.0"), "{}", knot.detail); + assert!( + knot.detail + .contains("capabilities: knot-acl, repo-did-input"), + "{}", + knot.detail + ); + + // A service that offers neither says neither, rather than saying + // `null` or `unknown` at somebody. + let plain = verdict( + "appview", + "tangled.org", + answered(200, serde_json::json!({})), + ); + assert_eq!(plain.detail, "tangled.org answered in 7ms"); + } + + /// The left-hand label is an authority and not a URL, and a port is part + /// of one — two local instances on one host are told apart by nothing + /// else, which is exactly the case the integration rig runs in. + #[test] + fn a_host_label_drops_the_scheme_and_keeps_the_port() { + assert_eq!(host_of("https://api.tangled.org"), "api.tangled.org"); + assert_eq!(host_of("https://tangled.org/"), "tangled.org"); + assert_eq!(host_of("http://127.0.0.1:8080"), "127.0.0.1:8080"); + // Already an authority: a knot is named that way in a DID document. + assert_eq!(host_of("knot1.tangled.sh"), "knot1.tangled.sh"); } } diff --git a/src/cmd/status.rs b/src/cmd/status.rs --- a/src/cmd/status.rs +++ b/src/cmd/status.rs @@ -1,12 +1,10 @@ //! `atgc status` — the commands whose subject is not a repo. //! //! Every other family here takes one: `pr list` lists this repo's pull -//! requests, `issue list` this repo's issues, `repo view` this repo. The -//! verbs under `status` deliberately do not. `status pr` is your pull -//! requests wherever you filed them, `status tangled` is whether the services -//! underneath all of it are answering, and neither has a repo to be scoped to -//! — both are things you reach for from a directory that may not be a -//! checkout at all. +//! requests, `issue list` this repo's issues, `repo view` this repo. The verbs +//! under `status` deliberately do not. `status pr` is your pull requests +//! wherever you filed them, which has no repo to be scoped to — it is a thing +//! you reach for from a directory that may not be a checkout at all. //! //! # Why a group, and why it took a name that was in use //! @@ -17,11 +15,6 @@ //! one a reader has to be *told*, because neither word says it — `list` and //! `status` are both just "show me some pull requests" until somebody //! explains which ones. -//! -//! Meanwhile the command that wanted to be called `status` — an overview -//! spanning repos, which TODO.md has carried since before `pr status` existed -//! — had nowhere to go, the name being taken one level down by something that -//! was already most of it. //! //! So the scope moved into the first word, where it is read first: `atgc pr //! …` is this repo, `atgc status …` is not. Everything else about the command @@ -37,25 +30,6 @@ //! two siblings to sit under the word that invokes it would be filing by //! spelling. What was wrong was the command surface, and the command surface //! is what moved. -//! -//! `status tangled` is here in full, by the same rule read the other way: it -//! shares its machinery with nothing. What it needs from outside is one -//! request that treats a refusal as an answer, which is -//! [`crate::clients::probe`], and the report vocabulary `doctor` also uses, -//! which is [`crate::term::checks`]. -//! -//! # `status tangled` against `doctor` -//! -//! They look alike and ask opposite questions, which is the only thing to -//! know about either. `doctor` asks whether *you* are set up: every row is -//! about this machine, this account or this checkout, and a bad one carries a -//! command you can run. `status tangled` asks whether *they* are up: no row -//! is about you, no row has a remedy, and the answer to a bad one is to wait. -//! -//! That is the question left over when `doctor` says everything is fine and -//! the thing you were doing still does not work — and it is the one `doctor` -//! structurally cannot answer, because every network row it has is scoped to -//! the repo you are standing in and goes `n/a` the moment you are not. /// The `atgc status` subjects. #[derive(clap::Subcommand, Debug)] @@ -80,467 +54,11 @@ /// atgc status pr --state all --json | jq 'group_by(.repo) | length' #[command(verbatim_doc_comment)] Pr(crate::cmd::pr::read::StatusArgs), - /// Whether Tangled's services are answering, and what they are running - /// - /// The other half of `atgc doctor`. That command asks whether *you* are - /// set up — this machine, this account, this checkout — and every bad - /// row it prints carries a command you can run. This one asks whether - /// *they* are up, so no row is about you and none has a remedy: the - /// answer to a bad one is to wait. It is the question left when `doctor` - /// says everything is fine and the thing you were doing still fails. - /// - /// Four rows, each asked the question atgc's own use of that service - /// depends on rather than a health endpoint invented for the purpose: - /// - /// appview tangled.org, which serves every `view:` link and the - /// pull numbers atgc cannot get anywhere else - /// bobbin api.tangled.org, the index behind `--source bobbin` and - /// `atgc search` - /// knot the git host this checkout's repo names, asked for its - /// version and the protocol capabilities it declares - /// pds your own PDS — not Tangled's, and the reason it belongs - /// here: a pull request is a record in it, so it is as much - /// a part of "is Tangled working" as the appview is - /// - /// A row goes `n/a` when there is nothing to ask: no knot outside a - /// checkout unless `--knot` names one, no PDS with no account selected. - /// A host that never answered is an error and exits 6, the status that - /// means retry unchanged; a host that answered a refusal is a warning, - /// because a service that refuses is a service that is running. - /// - /// Needs no session and no checkout. Every request is a public read. - /// - /// Examples: - /// atgc status tangled - /// atgc status tangled --knot knot1.tangled.sh - /// atgc status tangled --json | jq -r '.checks[] | "\(.name) \(.status)"' - #[command(verbatim_doc_comment)] - Tangled(TangledArgs), } /// Run whichever `status` subject was parsed. pub(crate) async fn run(command: Command) -> anyhow::Result<()> { match command { Command::Pr(args) => crate::cmd::pr::read::status(args).await, - Command::Tangled(args) => tangled(args).await, - } -} - -// --------------------------------------------------------------------------- -// status tangled -// --------------------------------------------------------------------------- - -use crate::clients::probe; -use crate::exit::Exit; -use crate::term::checks::{Check, CheckJson, exit_status, render, summary}; - -#[derive(clap::Args, Debug)] -pub(crate) struct TangledArgs { - /// Ask this knot, rather than the one this checkout's repo names - #[arg(long, value_name = "HOST")] - pub knot: Option, - /// Print one JSON object instead of the report: every service with its - /// status, plus which host each row was asked - #[arg(long)] - pub json: bool, -} - -/// The Bobbin query the `bobbin` row is a probe of. -/// -/// `sh.tangled.search.query` and not a health endpoint, for the reason the -/// command's help gives: what is worth knowing is whether the method atgc -/// depends on answers, and Tangled's `_health` route is absent from the -/// build running in production, so asking it would report a 404 for every -/// service in every state. -/// -/// The term is chosen to match nothing. A probe wants the query path -/// exercised, not a page of results carried across the network, and a -/// recognisable string is a courtesy to anyone reading Bobbin's own logs -/// wondering what keeps asking. -const BOBBIN_PROBE: &str = "sh.tangled.search.query?q=atgc-status-probe&limit=1"; - -/// The knot method the `knot` row is a probe of: public, cheap, and the only -/// one that answers with something worth printing even when all is well — -/// the knot's version, and the protocol capabilities it declares. A knot too -/// old to declare them omits the field, which its own lexicon says to read as -/// legacy rather than as broken. -const KNOT_PROBE: &str = "sh.tangled.knot.version"; - -/// Which host each row was asked, as `--json` reports it. -/// -/// The counterpart of `doctor`'s subject block and there for the same reason: -/// a report that says `bobbin: ok` without saying which Bobbin is a report -/// that cannot be told apart from one taken against a local instance. Every -/// one of these moves — see [`crate::clients::endpoints`] — and two of them -/// are read out of a checkout and a session rather than compiled in. -#[derive(serde::Serialize, Debug, PartialEq)] -pub(crate) struct TangledSubjectJson { - pub appview: String, - pub bobbin: String, - /// The knot named by `--knot`, or the one this checkout's repo's DID - /// document names. `null` when neither said. - pub knot: Option, - /// The acting account's PDS, `null` with no account selected. An - /// authority like the two above it and not the endpoint URL the DID - /// document carries: one object naming its four hosts two different ways - /// is one a caller has to special-case. - pub pds: Option, - pub account_did: Option, -} - -/// `status tangled --json`'s whole object. Deliberately `doctor --json`'s -/// shape — `ok`, a subject block, then the rows — because a caller that -/// learned to read one report should not have to learn the other. -#[derive(serde::Serialize, Debug, PartialEq)] -pub(crate) struct TangledJson { - /// Whether the process is about to exit `0`. - pub ok: bool, - pub subject: TangledSubjectJson, - /// Every service, in report order. - pub checks: Vec, -} - -/// Ask each service whether it is there, and print what came back. -pub(crate) async fn tangled(args: TangledArgs) -> anyhow::Result<()> { - crate::term::jsonout::init(args.json); - - let appview = crate::clients::endpoints::appview(); - let bobbin = crate::clients::endpoints::bobbin(); - - // Four things at once: the two compiled-in services, and the two lookups - // that decide whether there is a knot and a PDS to ask at all. None of - // them can inform another, and doing them in turn would cost four - // timeouts on a machine with no network instead of one — the same reason - // `doctor` overlaps its two chains. - let (appview_row, bobbin_row, knot_host, account) = tokio::join!( - service_row("appview", host_of(&appview), format!("{appview}/")), - service_row( - "bobbin", - host_of(&bobbin), - format!("{bobbin}/xrpc/{BOBBIN_PROBE}") - ), - knot_here(args.knot.clone()), - acting_account(), - ); - - // And the two that could not be addressed until those resolved. - let pds_host = match &account { - Some(did) => crate::clients::atproto::did::pds_from_did_doc(did).await, - None => None, - }; - let (knot_row, pds_row) = tokio::join!( - async { - match &knot_host { - Some(host) => { - let url = format!( - "{}/xrpc/{KNOT_PROBE}", - crate::clients::endpoints::knot(host) - ); - service_row("knot", host.clone(), url).await - } - None => Check::na( - "knot", - "no Tangled repo here to name a knot, and --knot named none", - ), - } - }, - async { - match &pds_host { - Some(endpoint) => { - service_row("pds", host_of(endpoint), format!("{endpoint}/xrpc/_health")).await - } - None => Check::na( - "pds", - "no account is selected, so there is no PDS to ask (atgc auth login)", - ), - } - }, - ); - - // Report order is widest blast radius first: the appview and the index - // serve every repo, a knot serves one, and your PDS serves you. That is - // not a dependency chain — none of the four needs another — so unlike - // `doctor` this report makes no claim about which to deal with first. - let checks = vec![appview_row, bobbin_row, knot_row, pds_row]; - - let exit = exit_status(&checks); - if args.json { - crate::term::jsonout::emit(&TangledJson { - ok: exit == Exit::Ok, - subject: TangledSubjectJson { - appview: host_of(&appview), - bobbin: host_of(&bobbin), - knot: knot_host, - pds: pds_host.as_deref().map(host_of), - account_did: account, - }, - checks: checks.iter().map(CheckJson::of).collect(), - })?; - } else { - print!("{}", render(&checks)); - } - - if exit == Exit::Ok { - return Ok(()); - } - // The report is on stdout and this is the line that carries the status, - // exactly as `doctor` does it — see that command's module documentation - // for why a report may answer and still exit non-zero. - Err(crate::exit::fail( - exit, - summary(&checks, "status tangled", None), - )) -} - -/// One service, asked one question, as a row. -async fn service_row(name: &'static str, host: String, url: String) -> Check { - verdict(name, &host, probe::get(&url).await) -} - -/// What one answer is worth, with no request in sight. -/// -/// Pure, and split from [`service_row`] for the reason -/// [`crate::docs::testing`] gives: the request is four lines of plumbing and -/// this is the whole of the judgement. -/// -/// The three outcomes are the point of the command and are not the three a -/// client would give you. A host that never answered is broken and carries -/// the retry-unchanged status; a host that answered *badly* is running, and a -/// service that is running is not an outage even when it is useless to you; -/// and a host that answered is asked what it is, because a version and a -/// capability list cost nothing extra and are the two things you cannot find -/// out any other way. -fn verdict(name: &'static str, host: &str, answer: anyhow::Result) -> Check { - let answer = match answer { - Ok(answer) => answer, - Err(err) => { - return Check::bad( - name, - crate::exit::classify(&err), - format!("{host} did not answer\n{err:#}"), - ); - } - }; - - let took = format!("in {}ms", answer.elapsed.as_millis()); - let status = answer.status; - - if status.is_server_error() { - // The tree's one status table, which makes exactly the split this - // needs: a gateway that never reached the service behind it is worth - // retrying unchanged, and any other 5xx is a service broken in a way - // waiting may not mend. Only 5xx reaches here, so the authenticated - // and public readings of the table agree. - return Check::bad( - name, - crate::exit::from_status(status), - format!( - "{host} answered {status} {took}\nit is reachable and the service behind it is not serving" - ), - ); - } - if !status.is_success() { - return Check::warn( - name, - format!( - "{host} answered {status} {took}\nit is running, but did not serve what atgc asked it for" - ), - ); - } - - let mut said = format!("{host} answered {took}"); - if let Some(version) = answer.version() { - said.push_str(&format!(", running {version}")); - } - // Only a knot declares these today. Reading the field on every row rather - // than only on that one costs a branch nobody has to maintain, and the - // day a second service declares capabilities the row will print them. - let capabilities = answer.strings("capabilities"); - if !capabilities.is_empty() { - said.push_str(&format!("\ncapabilities: {}", capabilities.join(", "))); - } - Check::ok(name, said) -} - -/// The knot to ask about: the one named, or the one this checkout's repo -/// belongs to. -/// -/// Read from the repo's *own DID document* rather than from its -/// `sh.tangled.repo` record, which is how `doctor` finds a knot. The record -/// sits in the owner's PDS at an address only Bobbin can supply, so that -/// route goes dark exactly when the index does — and an index outage is one -/// of the things somebody runs this command to confirm. The document is -/// minted by the knot and answers whoever asks. -/// -/// Every step is allowed to come up empty. Standing outside a checkout, in -/// somebody else's forge, or in a Tangled repo whose document names no knot -/// are all `n/a` rather than failures: this command must work when everything -/// is wrong, the same rule `doctor` holds itself to. -async fn knot_here(named: Option) -> Option { - if let Some(host) = named { - return Some(host); - } - let remote = crate::clients::git::run::remote_url("origin").ok()?; - let repo = crate::clients::tangled::resolve::repo_ref(&remote) - .await - .ok()?; - crate::clients::atproto::did::knot_from_did_doc(&repo.did).await -} - -/// The DID of the account atgc would act as, or `None` when there is none. -/// -/// A failed selection is not an error here. Logged out is a legitimate way to -/// run this — every other row is a public read — and it makes exactly one row -/// `n/a`. -async fn acting_account() -> Option { - crate::config::account::select() - .await - .ok() - .map(|selection| selection.did) -} - -/// The authority of a base URL, for the report's left-hand sentence. -/// -/// `https://api.tangled.org` reads as `api.tangled.org`, and a local instance -/// at `http://127.0.0.1:8080` keeps its port, because the port is what tells -/// two of them apart. -fn host_of(base: &str) -> String { - base.split_once("://") - .map_or(base, |(_scheme, rest)| rest) - .trim_end_matches('/') - .to_string() -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::term::checks::Status; - use std::time::Duration; - - fn answered(code: u16, body: serde_json::Value) -> anyhow::Result { - Ok(probe::Answer { - status: reqwest::StatusCode::from_u16(code).expect("a real status code"), - elapsed: Duration::from_millis(7), - body: Some(body), - }) - } - - /// The distinction the whole command exists for: a host that is silent is - /// broken, and a host that answers a refusal is not. - /// - /// It is the difference between "wait, it is them" and "it is up, and - /// this particular thing is wrong", and getting it backwards makes the - /// report worse than useless — a `warn` on an outage is a report somebody - /// acts on by rechecking their own machine, which is where they came - /// from. - #[test] - fn a_refusal_is_a_running_service_and_silence_is_not() { - // 2xx: up, and answering what was asked. - let up = verdict( - "bobbin", - "api.tangled.org", - answered(200, serde_json::json!({"hits": []})), - ); - assert_eq!(up.status, Status::Ok); - - // 4xx: still up. A method it does not implement, a parameter it did - // not like, a rate limit — none of those is an outage. - for code in [400, 404, 429] { - let refused = verdict( - "bobbin", - "api.tangled.org", - answered(code, serde_json::json!({})), - ); - assert_eq!(refused.status, Status::Warn, "{code} is not an outage"); - assert!(refused.detail.contains("is running"), "{}", refused.detail); - } - - // 5xx: the address answers and the service behind it does not. A - // gateway failure is the retry-unchanged status; anything else is - // unclassified, because waiting may not mend it. - assert_eq!( - verdict( - "appview", - "tangled.org", - answered(503, serde_json::json!({})) - ) - .status, - Status::Bad(Exit::Unreachable) - ); - assert_eq!( - verdict( - "appview", - "tangled.org", - answered(500, serde_json::json!({})) - ) - .status, - Status::Bad(Exit::Failure) - ); - - // Nothing at all. `classify` reads the status off the error, so an - // ordinary one lands on the unclassified code rather than claiming a - // connect failure this test cannot manufacture. - let silent = verdict( - "knot", - "knot1.tangled.sh", - Err(anyhow::anyhow!("connection refused")), - ); - assert_eq!(silent.status, Status::Bad(Exit::Failure)); - assert!(silent.detail.starts_with("knot1.tangled.sh did not answer")); - - // No row here has a remedy, ever: there is no command that mends - // somebody else's host, and advice that cannot work is worse than - // none. - for row in [up, silent] { - assert_eq!(row.remedy, None); - } - } - - /// A healthy row says what it reached, how long it took, and what the - /// service says it is — the last being the part no other command can tell - /// you, and the reason the probe reads a body it does not need. - #[test] - fn a_healthy_row_carries_the_version_and_capabilities_when_offered() { - let knot = verdict( - "knot", - "knot1.tangled.sh", - answered( - 200, - serde_json::json!({ - "version": "v1.15.0", - "capabilities": ["knot-acl", "repo-did-input"], - }), - ), - ); - assert_eq!(knot.status, Status::Ok); - assert!(knot.detail.contains("knot1.tangled.sh answered in 7ms")); - assert!(knot.detail.contains("running v1.15.0"), "{}", knot.detail); - assert!( - knot.detail - .contains("capabilities: knot-acl, repo-did-input"), - "{}", - knot.detail - ); - - // A service that offers neither says neither, rather than saying - // `null` or `unknown` at somebody. - let plain = verdict( - "appview", - "tangled.org", - answered(200, serde_json::json!({})), - ); - assert_eq!(plain.detail, "tangled.org answered in 7ms"); - } - - /// The left-hand label is an authority and not a URL, and a port is part - /// of one — two local instances on one host are told apart by nothing - /// else, which is exactly the case the integration rig runs in. - #[test] - fn a_host_label_drops_the_scheme_and_keeps_the_port() { - assert_eq!(host_of("https://api.tangled.org"), "api.tangled.org"); - assert_eq!(host_of("https://tangled.org/"), "tangled.org"); - assert_eq!(host_of("http://127.0.0.1:8080"), "127.0.0.1:8080"); - // Already an authority: a knot is named that way in a DID document. - assert_eq!(host_of("knot1.tangled.sh"), "knot1.tangled.sh"); } } diff --git a/src/term/checks.rs b/src/term/checks.rs --- a/src/term/checks.rs +++ b/src/term/checks.rs @@ -1,18 +1,18 @@ //! A report made of checks: the four outcomes, the row, and the two shapes a //! row is printed in. //! -//! atgc has two commands that answer by *reporting* rather than by acting, and -//! they ask opposite questions. [`crate::cmd::doctor`] asks whether this -//! machine, this account and this checkout are set up — every row is about -//! you, and a bad one has a remedy you can run. [`crate::cmd::status`]'s -//! `tangled` asks whether the services are answering — no row is about you, -//! and a bad one has nothing to run at all. +//! atgc has two reports that answer by *reporting* rather than by acting, and +//! they ask opposite questions. [`crate::cmd::doctor`]'s `local` asks whether +//! this machine, this account and this checkout are set up — every row is +//! about you, and a bad one has a remedy you can run. Its `remote` asks +//! whether the services are answering — no row is about you, and a bad one +//! has nothing to run at all. //! //! Different questions, one shape of answer, and the shape is the part worth //! writing once. A row is a name, one of four outcomes, a sentence, and //! sometimes a command; the report is those rows padded into columns, with a -//! status derived from the worst of them. Both halves of that were `doctor`'s -//! private business until there was a second reporter, and a second copy of +//! status derived from the worst of them. Both halves of that were one +//! report's private business until there was a second, and a second copy of //! `Status` would have been a second vocabulary for `--json` to promise — //! which is the failure [`crate::term::style`] was extracted to stop, one //! module over. @@ -30,8 +30,9 @@ /// How one check came out. /// /// Four outcomes rather than a boolean, because two of the four are the ones -/// that keep a report usable. `Na` is what makes running `doctor` outside a -/// checkout, or logged out, a report instead of a refusal; `Warn` is what +/// that keep a report usable. `Na` is what makes running `doctor local` +/// outside a checkout, or logged out, a report instead of a refusal; `Warn` is +/// what /// keeps a lagging index or an unconfigured git identity from being dressed /// up as a broken install. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -122,8 +123,8 @@ /// The status a whole report exits with. /// -/// `Ok` when nothing is broken — a warning is not, or `doctor` would exit -/// non-zero in every checkout that has never been near Tangled. Otherwise the +/// `Ok` when nothing is broken — a warning is not, or `doctor local` would +/// exit non-zero in every checkout that has never been near Tangled. Otherwise the /// status of the **first** broken check, which is not arbitrary: a report /// orders its rows by what depends on what, so the first one broken is the one /// whose remedy comes first. A session that is missing makes the scope gap @@ -157,8 +158,9 @@ /// scrollback with three commands above it. /// /// `tail` is what the first row being chosen *means*, and only one of the two -/// reports has an answer. `doctor` orders its rows by what depends on what, so -/// the first broken one is the one to fix first and saying so is the whole +/// reports has an answer. `doctor local` orders its rows by what depends on +/// what, so the first broken one is the one to fix first and saying so is the +/// whole /// value of the line. A report about four independent services has no such /// claim to make and passes `None` rather than inventing one — advice that /// does not help being worse than none, the same rule [`Check::remedy`] @@ -238,8 +240,8 @@ /// One check, as `--json` prints it. #[derive(serde::Serialize, Debug, PartialEq)] pub struct CheckJson { - /// The row's label — `doctor`'s `config dir`, `session`, `scopes`, `ssh - /// key`, `git identity`, `remote`, `knot`, `index`; `status tangled`'s + /// The row's label — `doctor local`'s `config dir`, `session`, `scopes`, + /// `ssh key`, `git identity`, `origin`, `index`; `doctor remote`'s /// `appview`, `bobbin`, `knot`, `pds`. Stable per command; the sentence /// beside it is not. pub name: &'static str, -- tangled.sh