diff --git a/docs/accounts.md b/docs/accounts.md new file mode 100644 index 0000000..72d456e --- /dev/null +++ b/docs/accounts.md @@ -0,0 +1,119 @@ +# Who atgc acts as + +Every command that writes something publishes it under one account, and that +is permanent: a pull request, an issue and an SSH key are all records in that +account's own repository, signed by its credentials, and there is no edit that +changes whose they were. So the question "who is atgc acting as right now" has +to have an answer you can predict before you run anything, and one you can +check afterwards. + +This page is that answer. It matters more than it used to because one machine +now routinely holds several accounts — a person's, and one for each agent +working on that person's behalf. + +## Where the choice comes from + +Five things can decide it. The first one that answers, wins: + +1. **`--account` on the command line.** Lasts exactly one command. +2. **`ATGC_ACCOUNT` in the environment.** Lasts as long as the shell does. +3. **The checkout you are standing in**, which names an account in its git + config as `user.email`. This is also what git itself uses to author + commits, so the commits and the records agree about who made them. +4. **The default account**, set by `atgc auth default`. This is a fallback and + nothing more — anything above it wins, every time. +5. **The only account**, when exactly one is logged in. Nothing to choose + between, so nothing to configure. + +If none of them answers, atgc stops and says so rather than guessing. + +`atgc auth status` prints which account it would act as and which of these +decided it, and every command that writes says the same on its way past. If +the answer ever surprises you, that line names the thing to change. + +### A checkout that names an account you do not have + +atgc stops. It does not fall back to the next rule down. + +This is deliberate and it is the most important thing on this page. A checkout +that states whose it is has stated whose it is; quietly publishing under +somebody else because that account happens to be logged in is how work ends up +attributed to the wrong person, and it is not recoverable afterwards. Log in as +the account the checkout names, or pass `--account` to override it for one +command. + +## Worktrees + +Git worktrees share one config file. `git config --local` inside a worktree +does not write anything local to that worktree — it writes the file every +worktree of that repository reads. So without help, five worktrees are five +copies of one identity, and whoever configured last configured all of them. + +`atgc repo configure` inside a worktree writes that worktree's own config +instead, which git supports and which makes each worktree a different account +if you want it to be. Run it in the worktree, not in the main checkout: + +```text +cd path/to/worktree +atgc repo configure --account +``` + +It says which file it wrote, and `--local` forces the shared one when a repo +has a single owner and every worktree should be that owner. + +**A worktree never falls back to the default account.** The default is a +machine-wide preference for a checkout that says nothing about itself, and in a +worktree that is the wrong answer: this is where an agent works, and inheriting +whichever account the machine last defaulted to is how an agent publishes as +the person who owns the laptop. A worktree either names its account, or is on a +machine with only one, or stops and asks. + +A worktree that has not been given its own identity still works — it uses the +repository's, like any other checkout. atgc says so when it does, because it +means every worktree of that repository is the same account, which is usually +not what somebody with several worktrees wanted. + +## Agents + +An agent's identity is an account like any other: it logs in, it owns its +records, and it publishes under its own name. Two things differ. + +It has no `~/.ssh`. A person keeps push keys there, but an agent has no person +and no home directory of its own, and the machine's `~/.ssh` belongs to +whoever owns the machine. `atgc key create` makes a key for the account and +keeps it where atgc keeps its other credentials. + +It should not use the default account. The default exists so a person does not +have to say who they are in their own checkouts; an agent should always be +somewhere that names its account explicitly, which is what configuring the +worktree does. + +## Two ways to run several agents + +**Several worktrees, one account.** The agents work on your behalf, under your +name, and there is nothing to configure: one account is the only account, so it +answers everywhere. This is the simplest arrangement and it is what you get by +default. + +**Several worktrees, several accounts.** Each agent has its own identity: + +```text +atgc auth login # once per identity +atgc key create # once per identity +cd path/to/worktree # once per worktree +atgc repo configure --account +``` + +Now each agent's work is published under its own name, revocable on its own, +and visible as its own in any history. `--account` is needed on that last +command because it is the one command that deliberately ignores the checkout's +identity — it is writing the value the checkout will hold, so reading it first +would only ever re-affirm whatever was already there. + +## What is not separated yet + +An agent acting as **your** account is, to atgc, you. The credentials are the +same credentials, the records carry the same DID, and nothing marks which of +you wrote what. If that distinction matters to you today, the second +arrangement above is the way to get it: a separate account is separate all the +way down. diff --git a/docs/index.md b/docs/index.md index 317774c..a6a142f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -6,16 +6,19 @@ in its output. Why a dependency is vendored is in `Cargo.toml`. What changed and when is in `git log`. What is enforced is in `prek.toml`. None of that is repeated here, because a second copy is a second thing to be wrong. -What is left is five pages: +What is left is six pages: 1. [The model](architecture.md). Tangled's shape, which is somebody else's system and cannot be read out of this repo. Start here; nothing else makes sense first. -2. [Output contracts](output.md). The four rules that hold for every command. -3. [Module layout](module-layout.md). Where code goes, and the rules that are +2. [Who atgc acts as](accounts.md). Which account a command publishes under, + how a worktree gets one of its own, and how to give several agents + several identities. +3. [Output contracts](output.md). The four rules that hold for every command. +4. [Module layout](module-layout.md). Where code goes, and the rules that are not visible from the folder names. -4. [Testing](testing.md). What earns a test, and when to reach for a mock. -5. [Environment](environment.md). Every `ATGC_*` variable, including the ones +5. [Testing](testing.md). What earns a test, and when to reach for a mock. +6. [Environment](environment.md). Every `ATGC_*` variable, including the ones no `--help` page names. These are CommonMark under `docs/`, also compiled into rustdoc by diff --git a/plan/identity.md b/plan/identity.md index b6e8b43..c831a6c 100644 --- a/plan/identity.md +++ b/plan/identity.md @@ -41,6 +41,13 @@ re-register, so seconds is the whole safe window. `id` matches the DID asked for, and which nothing here does — is the next step and wants its own change +- [ ] The account marker for agent identities, once `bot.did.registration` + exists: read it once at login, store a bool in the registry beside + `client_id` — the one field there that is not a cache — and never + re-check, because the record is permanent. It keeps an agent account out + of `auth status`'s ordinary listing and stops a fleet's first login from + becoming the machine's default account + ## Done - [x] `auth status` looks accounts up concurrently. It read one DID document @@ -56,6 +63,27 @@ re-register, so seconds is the whole safe window. running those concurrently would trade N round trips for N waiters on one file, and it writes nothing when the cache already agrees. Measured on three live accounts: 0.47s to 0.25s +- [x] A worktree can be its own account. `git config --local` in a linked + worktree writes the file *every* worktree of the repo shares, so N agents + in N worktrees could not hold N identities — whoever ran `repo configure` + last set it for all of them, silently. Rank 3 now reads `--worktree` + first and `--local` second (never global, for the two reasons + `local_config` gives), which is also the order git itself resolves + `user.email` in: reading only the shared file had atgc acting as one + account while `git commit` authored as another, in the same directory. + `repo configure` in a worktree enables `extensions.worktreeConfig` and + writes there, `--local` forces the shared file, and a git too old to do + it errors before anything is written rather than after. `core.sshCommand` + follows the identity, since a worktree acting as its own account has to + push with that account's key +- [x] A linked worktree does not fall through to the default-account pointer. + That pointer is a machine-wide default for a checkout that says nothing, + and in a worktree it is how an agent publishes as whoever owns the + laptop. The sole-account rank is deliberately kept: a default is a choice + among several, and with one account logged in there is no choice — taking + it away would break `repo configure` in a worktree for anyone with a + single account, since that command skips rank 3 by design. A worktree + still reading the shared identity is legal and now says so - [x] `auth login` resolves handles DNS-first, as the handle spec orders it. jacquard's resolver has both steps in the right order but diff --git a/src/clients/git/config.rs b/src/clients/git/config.rs index a26e069..6cb353a 100644 --- a/src/clients/git/config.rs +++ b/src/clients/git/config.rs @@ -30,6 +30,119 @@ pub fn local_config(dir: &Path, key: &str) -> Option { .filter(|v| !v.is_empty()) } +/// Which of a checkout's two config files a value came from, or is going to. +/// +/// The distinction exists because a linked worktree has two, and they mean +/// different things: `config.worktree` belongs to this worktree alone, while +/// `.git/config` is shared with every other worktree of the repo. For an +/// identity that is the whole question — a value in the shared file makes +/// every worktree the same account. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Scope { + /// `.git/worktrees//config.worktree`, private to this worktree. + Worktree, + /// `.git/config`, shared by every worktree of the repo. + Repo, +} + +impl Scope { + /// The `git config` flag that reads and writes it. + fn flag(self) -> &'static str { + match self { + Scope::Worktree => "--worktree", + Scope::Repo => "--local", + } + } + + /// How to name it to somebody who has to act on it. + pub fn describe(self) -> &'static str { + match self { + Scope::Worktree => "this worktree's own git config", + Scope::Repo => "this repo's .git/config, shared by every worktree", + } + } +} + +/// A config value from one named scope of `dir`. +/// +/// `--worktree` fails rather than answering empty on a git that predates it +/// (2.20) and on a repo without `extensions.worktreeConfig`, and both of +/// those are "no value" here — the caller that needs to tell them apart is +/// the *writer*, which says so with a real error. +pub fn scoped_config(dir: &Path, scope: Scope, key: &str) -> Option { + git_in(dir, &["config", scope.flag(), "--get", key]) + .ok() + .filter(|v| !v.is_empty()) +} + +/// A checkout's own value for `key`, and which file it came from. +/// +/// The worktree's own file first, then the shared one — git's own order, and +/// that agreement is the point rather than a nicety. `git commit` resolves +/// `user.email` this way, so a reader that consulted only `--local` would +/// have atgc acting as one account while git authored as another, in the same +/// directory, with nothing saying so. +/// +/// Global config is still never consulted, at either rank, for the two +/// reasons [`local_config`] gives. +pub fn checkout_config(dir: &Path, key: &str) -> Option<(String, Scope)> { + scoped_config(dir, Scope::Worktree, key) + .map(|v| (v, Scope::Worktree)) + .or_else(|| scoped_config(dir, Scope::Repo, key).map(|v| (v, Scope::Repo))) +} + +/// Write a config value into one named scope. +/// +/// `--worktree` needs `extensions.worktreeConfig` switched on for the repo; +/// [`enable_worktree_config`] is what does that, and callers turn it on +/// before writing rather than discovering the refusal here. +pub fn set_scoped_config(dir: &Path, scope: Scope, key: &str, value: &str) -> Result<()> { + git_in(dir, &["config", scope.flag(), key, value]).map(|_| ()) +} + +/// Switch on per-worktree configuration for the repo containing `dir`. +/// +/// Repo-wide and therefore `--local`, which is the one thing about this that +/// looks contradictory: the switch that makes per-worktree values possible is +/// itself shared, because git has to know to look for those files from every +/// worktree. It is additive — every existing value stays in `.git/config` and +/// stays visible everywhere — and it is what `git worktree` documents for +/// exactly this case. +/// +/// Fails on a git older than 2.20, which is the answer this returns rather +/// than a version parse: asking git to do it is a more reliable test of +/// whether git can than comparing numbers, and the failure carries git's own +/// words. +pub fn enable_worktree_config(dir: &Path) -> Result<()> { + if scoped_config(dir, Scope::Repo, "extensions.worktreeConfig") + .is_some_and(|v| v.eq_ignore_ascii_case("true")) + { + return Ok(()); + } + set_scoped_config(dir, Scope::Repo, "extensions.worktreeConfig", "true")?; + // Proof, and the only reliable kind: do the thing. + // + // Reading is no test. `--worktree --get` of a key that lives in the + // shared file exits "not found" on a git that supports the scope and + // "fatal" on one that does not, and `--worktree --list` fails on both + // until `config.worktree` exists at all. So a throwaway key is written + // and removed, which fails exactly when the write that follows would — + // before an identity is half-written to the wrong file. + // + // What it leaves behind is the empty `config.worktree` the write needs + // anyway. `--unset` of a key that was just set cannot fail for a reason + // the caller could act on, so its result is not carried up. + const PROBE: &str = "atgc.worktreeconfigprobe"; + set_scoped_config(dir, Scope::Worktree, PROBE, "1").map_err(|e| { + anyhow::anyhow!( + "this git cannot write per-worktree configuration \ + (`git config --worktree`, which needs git 2.20)\n{e}" + ) + })?; + let _ = git_in(dir, &["config", "--worktree", "--unset", PROBE]); + Ok(()) +} + /// The branch a fresh `git init` here would name, out of the whole config /// chain rather than one repo's file — `init.defaultBranch` is a preference /// people set globally if they set it at all. @@ -72,6 +185,27 @@ pub fn set_local_config(dir: &Path, key: &str, value: &str) -> Result<()> { /// one: `repo configure` is run by hand in whatever the shell happens to be /// sitting in. `Usage` too — the checkout is the argument this command takes, /// and running it again from here cannot work. +/// The file one scope reads and writes for `dir`. +/// +/// `--git-path` for both, because only git knows where either lands: in a +/// linked worktree `config` resolves to the *shared* file while +/// `config.worktree` resolves under this worktree's own git directory, and +/// constructing either by hand gets one of them wrong. +pub fn scoped_config_path(dir: &Path, scope: Scope) -> Result { + match scope { + Scope::Repo => local_config_path(dir), + Scope::Worktree => Ok(PathBuf::from(git_in( + dir, + &[ + "rev-parse", + "--path-format=absolute", + "--git-path", + "config.worktree", + ], + )?)), + } +} + pub fn local_config_path(dir: &Path) -> Result { let path = git_in(dir, &["rev-parse", "--git-path", "config"]).map_err(|e| { crate::exit::fail( @@ -115,11 +249,18 @@ pub fn git_dir(dir: &Path) -> Option { /// /// Worth asking because `git config --local` in a linked worktree does not /// write anything local to that worktree: it writes the config shared by the -/// whole repo, so an identity set in one worktree is the identity of every -/// worktree. That is git's design, and the only thing to do about it is say -/// so — a project whose workflow lives in worktrees could reasonably expect -/// otherwise. `--git-common-dir` is the half that names the shared -/// directory; in an ordinary checkout the two are the same path. +/// whole repo, so an identity set there is the identity of every worktree. +/// That is git's design, and it used to be the end of the story here — this +/// comment said the only thing to do about it was say so. +/// +/// There is something to do about it: `extensions.worktreeConfig` and the +/// `--worktree` scope, which is what [`Scope::Worktree`] writes and +/// [`checkout_config`] reads first. This function is how everything else +/// knows which of the two situations it is in — whether a shared identity is +/// the only kind available, or a deliberate choice not to have one. +/// +/// `--git-common-dir` is the half that names the shared directory; in an +/// ordinary checkout the two are the same path. pub fn is_linked_worktree(dir: &Path) -> bool { let common = git_in(dir, &["rev-parse", "--git-common-dir"]) .ok() @@ -317,6 +458,72 @@ mod tests { /// silence in the case that needs it — and the first version did the /// former, because git answers `--git-path` relatively at the repo root /// and absolutely everywhere else. + /// The whole point, end to end: two worktrees of one repo, two identities. + /// + /// This is what `--local` cannot express and what the account selector + /// now depends on. The shared file keeps answering for the main checkout + /// and for any worktree that has not been given one of its own, so the + /// two ranks coexist rather than one replacing the other. + #[test] + fn a_worktree_can_hold_an_identity_of_its_own() { + let repo = TempRepo::new("worktree-scope"); + repo.commit("a.txt", "one\n", "Add a"); + set_scoped_config(here(), Scope::Repo, "user.email", "did:plc:shared").unwrap(); + repo.git(&["worktree", "add", "-q", "-b", "side", "linked"]); + let linked = Path::new("linked"); + + // Before it is given one, the worktree answers with the shared value + // and says so — that is the state the account selector warns about. + assert_eq!( + checkout_config(linked, "user.email"), + Some(("did:plc:shared".to_string(), Scope::Repo)) + ); + + enable_worktree_config(linked).unwrap(); + set_scoped_config(linked, Scope::Worktree, "user.email", "did:plc:agent").unwrap(); + + assert_eq!( + checkout_config(linked, "user.email"), + Some(("did:plc:agent".to_string(), Scope::Worktree)), + "the worktree's own file outranks the shared one" + ); + assert_eq!( + checkout_config(here(), "user.email"), + Some(("did:plc:shared".to_string(), Scope::Repo)), + "and the main checkout is untouched by it" + ); + // The agreement that matters: git resolves it the same way, so a + // commit made here is authored by the account atgc is acting as. + // + // Asked through `git_in` rather than the test harness's runner, which + // injects `-c user.email=` on every call so that commits in a temp + // repo have an author. A command-line override outranks every file + // and would answer this question with its own argument. + assert_eq!( + git_in(linked, &["config", "--get", "user.email"]).unwrap(), + "did:plc:agent" + ); + } + + /// A value the worktree does not set falls through to the shared file + /// rather than to `~/.gitconfig`. + /// + /// The exclusion `local_config` documents, restated for the two-rank + /// reader: a DID in somebody's global config must not become an identity + /// here, at either rank. + #[test] + fn the_global_config_is_still_never_consulted() { + let repo = TempRepo::new("worktree-global"); + repo.commit("a.txt", "one\n", "Add a"); + repo.git(&["worktree", "add", "-q", "-b", "side", "linked"]); + let linked = Path::new("linked"); + enable_worktree_config(linked).unwrap(); + // Nothing set at either repo rank, and the harness points + // GIT_CONFIG_GLOBAL at a file with an identity in it. + assert_eq!(checkout_config(linked, "user.email"), None); + assert_eq!(checkout_config(here(), "user.email"), None); + } + #[test] fn spots_a_linked_worktree_and_only_a_linked_worktree() { let repo = TempRepo::new("worktree"); diff --git a/src/cmd/pr/review.rs b/src/cmd/pr/review.rs index 1d2db2a..49e8071 100644 --- a/src/cmd/pr/review.rs +++ b/src/cmd/pr/review.rs @@ -409,7 +409,9 @@ async fn author_of_rkey(rkey: &str, author: Option<&str>, remote: &str) -> Resul return Ok(did); } - if let Some(did) = account::repo_did() { + // The scope it came from does not matter here — this is asking which repo + // a patch belongs to, not who is acting. + if let Some((did, _scope)) = account::repo_did() { return Ok(did); } if let Ok(selection) = account::select().await { diff --git a/src/cmd/repo/checkout.rs b/src/cmd/repo/checkout.rs index 5582f0f..12af7aa 100644 --- a/src/cmd/repo/checkout.rs +++ b/src/cmd/repo/checkout.rs @@ -134,14 +134,19 @@ impl Identity { pub(super) fn write_git_identity( dir: &Path, id: &GitIdentity, + scope: gitconfig::Scope, overwrite: Overwrite, dry_run: bool, ) -> Result { let name = format!("@{}", id.handle.trim_start_matches('@')); let email = &id.did; - let current_name = gitconfig::local_config(dir, "user.name"); - let current_email = gitconfig::local_config(dir, "user.email"); + // Read from the scope about to be written, not from the effective value. + // "Is it already set?" is a question about *this file*: a worktree + // inheriting the repo's identity has not made a choice about itself, and + // writing its own is not clobbering one. + let current_name = gitconfig::scoped_config(dir, scope, "user.name"); + let current_email = gitconfig::scoped_config(dir, scope, "user.email"); let conflicts: Vec<(&str, &str, &str)> = [ ("user.name", current_name.as_deref(), name.as_str()), @@ -225,8 +230,8 @@ pub(super) fn write_git_identity( return Ok(Identity::Written); } - gitconfig::set_local_config(dir, "user.name", &name)?; - gitconfig::set_local_config(dir, "user.email", email)?; + gitconfig::set_scoped_config(dir, scope, "user.name", &name)?; + gitconfig::set_scoped_config(dir, scope, "user.email", email)?; crate::term::say::step!(Config, "user.name = {name}, user.email = {email}"); Ok(Identity::Written) } @@ -245,6 +250,7 @@ pub(super) fn write_git_identity( pub(super) fn apply_git_identity( dir: &Path, id: Result, + scope: gitconfig::Scope, overwrite: Overwrite, no_git_config: bool, dry_run: bool, @@ -254,7 +260,7 @@ pub(super) fn apply_git_identity( return Ok(Identity::Skipped); } match id { - Ok(id) => write_git_identity(dir, &id, overwrite, dry_run), + Ok(id) => write_git_identity(dir, &id, scope, overwrite, dry_run), Err(e) => { crate::logging::debug::dump_err("git identity lookup failed", &e); // `account::select` errors already say what to do about it — @@ -699,6 +705,9 @@ pub(super) async fn clone(args: CloneArgs) -> Result<()> { let identity = apply_git_identity( &dir, git_identity, + // A clone makes a checkout of its own, never a linked worktree, so + // there is no other scope for it to land in. + gitconfig::Scope::Repo, Overwrite::Never, args.no_git_config, args.dry_run, @@ -745,6 +754,15 @@ pub(crate) struct ConfigureArgs { /// Replace an ordinary git identity, not just another Tangled one #[arg(long)] pub force: bool, + /// Write to .git/config, shared by every worktree of this repo + /// + /// In a linked worktree the identity goes into that worktree's own config + /// by default, so each one can be a different account. This forces the + /// shared file instead — which is what you want when the repo has one + /// owner, and what you have to pass on a git too old for per-worktree + /// config. + #[arg(long)] + pub local: bool, /// Show what would be written without writing anything #[arg(long)] pub dry_run: bool, @@ -807,7 +825,27 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { // rather than with git's complaint bolted onto some later step. This also // works from a subdirectory — git answers about the enclosing repo — and // in a bare repo, which has a local config like any other. - let config_path = gitconfig::local_config_path(here)?; + let repo_config_path = gitconfig::local_config_path(here)?; + + // Which of the checkout's two config files this run writes. + // + // In a linked worktree, that worktree's own — the whole point being that + // one agent per worktree can be one account per worktree, which `--local` + // cannot express because it means the file every worktree shares. The + // main checkout has only the shared one and takes it without being asked. + // + // `--local` in a worktree is the deliberate opposite: one owner for the + // repo and every worktree of it. + let linked = gitconfig::is_linked_worktree(here); + let scope = match linked && !args.local { + true => gitconfig::Scope::Worktree, + false => gitconfig::Scope::Repo, + }; + // Reported rather than assumed, for the reason `local_config_path` gives + // about `--git-path`: in a worktree the two files are in different + // directories, and `writes:` naming the wrong one is the whole confusion + // this change exists to end. + let config_path = gitconfig::scoped_config_path(here, scope).unwrap_or(repo_config_path); // Rule 3 of account selection is skipped here, and only here. See // `account::Checkout`: the checkout's `user.email` is the value about to @@ -831,13 +869,44 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { // but it is a surprise worth naming out loud, since this project's own // workflow lives in worktrees and a user could reasonably expect one // worktree to be configurable independently of the rest. - let linked = gitconfig::is_linked_worktree(here); - if linked { - crate::term::say::note!( + if scope == gitconfig::Scope::Worktree && !args.dry_run { + // Before anything is written, because the failure is "this git cannot + // do that at all" and the repair is a different command line — not + // something to discover with half an identity on disk. + // + // And not under `--dry-run`, which is the whole reason this is a + // condition rather than a plain call: switching the extension on is a + // write to `.git/config`, and a preview that edits the repository is + // not a preview. The cost is that a dry run cannot report the one + // failure that only appears on an old git — said below rather than + // discovered here. + gitconfig::enable_worktree_config(here).map_err(|e| { + crate::exit::fail( + crate::exit::Exit::Usage, + format!( + "{e}\nrun `atgc repo configure --local` to write the identity to \ + .git/config instead, which every worktree of this repo shares" + ), + ) + })?; + } + if linked && args.local { + crate::term::say::warning!( Config, - "linked worktree: this config is shared with every worktree here" + "--local in a linked worktree: this identity is shared with every worktree here, \ + so they all act as one account" ); } + match (args.dry_run, scope) { + (true, gitconfig::Scope::Worktree) => crate::term::say::step!( + Config, + "would write to {} (and switch on extensions.worktreeConfig, which \ + this run has not done)", + scope.describe() + ), + (true, _) => crate::term::say::step!(Config, "would write to {}", scope.describe()), + (false, _) => crate::term::say::step!(Config, "writing to {}", scope.describe()), + } // Setting a Tangled identity in a repo that has nothing to do with Tangled // is odd but not wrong: the DID goes in an email field git never @@ -873,7 +942,14 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { .as_ref() .ok() .map(|id| format!("@{}", id.handle.trim_start_matches('@'))); - let written = apply_git_identity(here, identity, overwrite, args.no_git_config, args.dry_run)?; + let written = apply_git_identity( + here, + identity, + scope, + overwrite, + args.no_git_config, + args.dry_run, + )?; // After the identity, though it used to run before it. An account with // no usable key now fails this command, and the repair half of it — the @@ -884,7 +960,7 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { crate::term::say::step!(Ssh, "skipped (--no-ssh-config)"); Ok(()) } else { - configure_ssh(here, &did, &who, written, args.dry_run).await + configure_ssh(here, &did, &who, written, scope, args.dry_run).await }; // The hook is the reason a stack can be created from a plain-git branch @@ -1058,6 +1134,7 @@ async fn configure_ssh( did: &str, who: &str, identity: Identity, + scope: gitconfig::Scope, dry_run: bool, ) -> Result<()> { let key = match push_key_for(did).await { @@ -1092,7 +1169,7 @@ async fn configure_ssh( }; let ssh_cmd = format!("ssh -o IdentitiesOnly=yes -i \"{}\"", identity.display()); - match gitconfig::local_config(dir, "core.sshCommand") { + match gitconfig::scoped_config(dir, scope, "core.sshCommand") { Some(current) if current == ssh_cmd => { crate::term::say::step!(Ssh, "already pinned to {}", identity.display()); return Ok(()); @@ -1105,7 +1182,7 @@ async fn configure_ssh( crate::term::say::step!(Ssh, "would set core.sshCommand = {ssh_cmd}"); return Ok(()); } - match gitconfig::set_local_config(dir, "core.sshCommand", &ssh_cmd) { + match gitconfig::set_scoped_config(dir, scope, "core.sshCommand", &ssh_cmd) { Ok(()) => { crate::term::say::step!(Ssh, "using {} (set core.sshCommand)", identity.display()) } diff --git a/src/cmd/repo/mod.rs b/src/cmd/repo/mod.rs index b7f2fca..10cdeaf 100644 --- a/src/cmd/repo/mod.rs +++ b/src/cmd/repo/mod.rs @@ -461,6 +461,7 @@ mod tests { show, text_value, }; use super::{LABEL, appview, display_repo, field, repo_url}; + use crate::clients::git::config::Scope; use crate::testutil::TempRepo; use serde_json::{Value, json}; use std::path::Path; @@ -876,7 +877,7 @@ mod tests { let repo = TempRepo::new("identity"); repo.commit("a.txt", "one\n", "Add a"); - write_git_identity(here(), &identity(), Overwrite::Never, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Never, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), @@ -898,7 +899,7 @@ mod tests { repo.commit("a.txt", "one\n", "Add a"); repo.git(&["config", "--local", "user.name", "Someone Else"]); - write_git_identity(here(), &identity(), Overwrite::Never, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Never, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), @@ -926,7 +927,7 @@ mod tests { "did:plc:nlzmjyfv6loqtxyzvdcznwgf", ]); - write_git_identity(here(), &identity(), Overwrite::Never, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Never, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), @@ -940,7 +941,7 @@ mod tests { let repo = TempRepo::new("identity-dry"); repo.commit("a.txt", "one\n", "Add a"); - write_git_identity(here(), &identity(), Overwrite::Never, true).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Never, true).unwrap(); let config = repo.local_config(); assert!(!config.contains("permadeath.com"), "config: {config}"); @@ -957,7 +958,7 @@ mod tests { repo.git(&["config", "--local", "user.name", "@someone.else"]); repo.git(&["config", "--local", "user.email", OTHER_DID]); - write_git_identity(here(), &identity(), Overwrite::Tangled, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Tangled, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), @@ -980,7 +981,7 @@ mod tests { repo.git(&["config", "--local", "user.name", "Ada Lovelace"]); repo.git(&["config", "--local", "user.email", "ada@example.com"]); - write_git_identity(here(), &identity(), Overwrite::Tangled, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Tangled, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), "Ada Lovelace" @@ -991,7 +992,7 @@ mod tests { ); // --force is the way through, and it takes both halves. - write_git_identity(here(), &identity(), Overwrite::Always, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Always, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), "@permadeath.com" @@ -1013,7 +1014,7 @@ mod tests { repo.git(&["config", "--local", "user.name", "Ada Lovelace"]); repo.git(&["config", "--local", "user.email", OTHER_DID]); - write_git_identity(here(), &identity(), Overwrite::Tangled, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Tangled, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), @@ -1038,7 +1039,7 @@ mod tests { repo.git(&["config", "--local", "user.email", "did:plc:short"]); assert!(crate::lexicon::identity::classify("did:plc:short").is_err()); - write_git_identity(here(), &identity(), Overwrite::Tangled, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Tangled, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.email"]), @@ -1054,7 +1055,7 @@ mod tests { let repo = TempRepo::new("identity-repair"); repo.commit("a.txt", "one\n", "Add a"); - write_git_identity(here(), &identity(), Overwrite::Tangled, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Tangled, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.email"]), @@ -1072,7 +1073,7 @@ mod tests { repo.git(&["config", "--local", "user.name", "@someone.else"]); repo.git(&["config", "--local", "user.email", OTHER_DID]); - write_git_identity(here(), &identity(), Overwrite::Always, true).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, Overwrite::Always, true).unwrap(); assert_eq!(repo.git(&["config", "--local", "user.email"]), OTHER_DID); } @@ -1091,7 +1092,7 @@ mod tests { ]); for policy in [Overwrite::Never, Overwrite::Tangled, Overwrite::Always] { - write_git_identity(here(), &identity(), policy, false).unwrap(); + write_git_identity(here(), &identity(), Scope::Repo, policy, false).unwrap(); assert_eq!( repo.git(&["config", "--local", "user.name"]), "@permadeath.com", diff --git a/src/cmd/repo/write.rs b/src/cmd/repo/write.rs index e01f7f3..a5f4097 100644 --- a/src/cmd/repo/write.rs +++ b/src/cmd/repo/write.rs @@ -481,7 +481,16 @@ pub(crate) async fn create(args: CreateArgs) -> Result<()> { handle, }) .with_context(|| format!("could not resolve a handle for {did}")); - apply_git_identity(&dir, identity, Overwrite::Never, args.no_git_config, false)?; + apply_git_identity( + &dir, + identity, + // `repo create` makes its checkout fresh, so there is only the one + // config file to write. + crate::clients::git::config::Scope::Repo, + Overwrite::Never, + args.no_git_config, + false, + )?; super::checkout::install_change_id_hook(&dir, args.no_hook, false, false); if !place.pushes() { diff --git a/src/config/account/selection.rs b/src/config/account/selection.rs index 754fa84..a60359a 100644 --- a/src/config/account/selection.rs +++ b/src/config/account/selection.rs @@ -34,6 +34,8 @@ pub fn init(flag: Option) { pub enum Source { Flag, Env, + /// This worktree's own git config — `config.worktree`, not shared. + WorktreeConfig, RepoConfig, Switched, Sole, @@ -47,6 +49,7 @@ impl Source { Source::Flag => "--account", Source::Env => "ATGC_ACCOUNT", Source::Named => "the account you named", + Source::WorktreeConfig => "this worktree's own git config", Source::RepoConfig => "this repo's .git/config", Source::Switched => "atgc auth default", Source::Sole => "the only logged-in account", @@ -85,6 +88,28 @@ impl Selection { self.display(), self.source.describe() ); + // One situation is legal, invisible, and almost never what was meant: + // a worktree whose identity came out of the file every worktree of the + // repo shares. Nothing is wrong yet — it is exactly what happens when + // `repo configure` was run in the main checkout and never here — but + // it means each worktree acts as the same account, which is the + // opposite of why anyone puts one agent in each. + // + // Only on the shared rank, and only in a linked worktree, so a repo + // with one owner and a person with one account never see it. The git + // call is one subprocess on a command that is already about to write + // to somebody's repository. + if shares_identity_with_every_worktree( + self.source, + crate::clients::git::config::is_linked_worktree(std::path::Path::new(".")), + ) { + crate::term::say::warning!( + Account, + "this worktree has no identity of its own, so it is acting as the account \ + .git/config names — the same one every worktree of this repo acts as\n\ + `atgc repo configure` here writes an identity for this worktree alone" + ); + } } } @@ -175,10 +200,11 @@ fn env_spec() -> Option { /// can only come from a broken Tangled checkout — that gets said out loud, /// because silently ignoring it means acting as whichever account rule 4 or /// 5 lands on, under a checkout that has already stated whose it is. -pub fn repo_did() -> Option { - let email = crate::clients::git::config::local_config(std::path::Path::new("."), "user.email")?; +pub fn repo_did() -> Option<(String, crate::clients::git::config::Scope)> { + let (email, scope) = + crate::clients::git::config::checkout_config(std::path::Path::new("."), "user.email")?; match crate::lexicon::identity::classify(&email) { - Ok(crate::lexicon::identity::Identifier::Did(did)) => Some(did), + Ok(crate::lexicon::identity::Identifier::Did(did)) => Some((did, scope)), other => { let complaint = match &other { Ok(_) => "it is not a DID".to_string(), @@ -187,8 +213,9 @@ pub fn repo_did() -> Option { if crate::lexicon::identity::looks_like_a_did(&email) { crate::term::say::warning!( Account, - "this checkout's .git/config has a malformed DID in user.email, \ - so it is being ignored for account selection\n{complaint}" + "{} has a malformed DID in user.email, so it is being ignored \ + for account selection\n{complaint}", + scope.describe() ); } else { // Debug only, and phrased without echoing the value: a user @@ -205,6 +232,22 @@ pub fn repo_did() -> Option { } } +/// Whether this selection means every worktree of the repo acts as one account. +/// +/// Pure, and split from the git call for the reason +/// [`crate::clients::git::config::checkout_config`] is split from its callers: +/// the condition is the claim being made, and a claim checkable only by +/// watching stderr in a temporary worktree is one nothing tests. +/// +/// True on exactly one combination. [`Source::WorktreeConfig`] is this +/// worktree's own answer and [`Source::RepoConfig`] in the main checkout is +/// the only file there is; it is the shared file *read from a worktree* that +/// means the identity is not this worktree's own. Ranks 1, 2 and 5 are not +/// about files at all. +fn shares_identity_with_every_worktree(source: Source, linked_worktree: bool) -> bool { + linked_worktree && source == Source::RepoConfig +} + /// Whether the current checkout's own `[user] email` gets a vote. /// /// Rule 3 of the precedence normally wins over the `auth default` pointer, @@ -245,7 +288,7 @@ pub enum Checkout { /// The one place a policy becomes a value, and therefore the one place /// selection and [`Standing`] can be seen to agree about what the checkout /// contributes. -fn checkout_did(checkout: Checkout) -> Option { +fn checkout_did(checkout: Checkout) -> Option<(String, crate::clients::git::config::Scope)> { match checkout { Checkout::Consulted => repo_did(), Checkout::Ignored => None, @@ -331,7 +374,15 @@ async fn select_inner(checkout: Checkout) -> Result { // determines whose name ends up on published work. let repo_did = checkout_did(checkout); let registry = load(); - select_local(&known, repo_did.as_deref(), registry.default.as_deref()) + // Asked once, here, for the same reason the two reads above are: the + // decision below stays a pure function of what the machine says. + let linked = crate::clients::git::config::is_linked_worktree(std::path::Path::new(".")); + select_local( + &known, + repo_did.as_ref().map(|(did, scope)| (did.as_str(), *scope)), + registry.default.as_deref(), + linked, + ) } /// Ranks 3 to 5 of the precedence: the checkout, the `auth default` pointer, @@ -343,15 +394,21 @@ async fn select_inner(checkout: Checkout) -> Result { /// exactly "the checkout said nothing". fn select_local( known: &[Known], - repo_did: Option<&str>, + repo_did: Option<(&str, crate::clients::git::config::Scope)>, active: Option<&str>, + linked_worktree: bool, ) -> Result { - if let Some(did) = repo_did { + use crate::clients::git::config::Scope; + + if let Some((did, scope)) = repo_did { return match known.iter().find(|a| a.did == did) { Some(a) => Ok(Selection { did: a.did.clone(), handle: a.handle.clone(), - source: Source::RepoConfig, + source: match scope { + Scope::Worktree => Source::WorktreeConfig, + Scope::Repo => Source::RepoConfig, + }, }), // Falling through to some other account here would be the worst // possible behaviour: the checkout states whose it is, and @@ -360,16 +417,34 @@ fn select_local( None => Err(crate::exit::fail( crate::exit::Exit::NoSession, format!( - "no session for {did}, which this checkout's .git/config names\n\ + "no session for {did}, which {} names\n\ log in with `atgc auth login `, or override with \ `--account`\n{}", + scope.describe(), known_summary(known) ), )), }; } - if let Some(active) = active + // Rule 4, and the one rank a linked worktree does not get. + // + // The pointer is a machine-wide default for a checkout that says nothing + // about whose it is. In a worktree that is exactly the wrong answer: this + // project's workflow puts one agent in each worktree, and falling through + // to whoever the *machine* last defaulted to is how an agent publishes as + // the person who owns the laptop. A worktree that has not been told whose + // it is must say so, not guess. + // + // Rule 5 below is deliberately still available, and is not an exception to + // that. A default is a choice among several; with exactly one account + // logged in there is no choice being made and nothing to get wrong, and + // taking it away would mean `atgc repo configure` — which skips rule 3 by + // design, since it is writing the value rule 3 reads — could not run in a + // worktree without `--account`, for a person with one account who has no + // ambiguity to resolve. + if !linked_worktree + && let Some(active) = active && let Some(a) = known.iter().find(|a| a.did == active) { return Ok(Selection { @@ -470,7 +545,7 @@ impl Standing { pub fn read(known: &[Known]) -> Standing { Standing { env: env_spec(), - repo: checkout_did(Checkout::Consulted).map(|did| CheckoutAccount { + repo: checkout_did(Checkout::Consulted).map(|(did, _scope)| CheckoutAccount { handle: known .iter() .find(|a| a.did == did) @@ -931,7 +1006,7 @@ mod tests { assert_eq!(super::repo_did(), None); repo.git(&["config", "--local", "user.email", DID]); - assert_eq!(super::repo_did().as_deref(), Some(DID)); + assert_eq!(super::repo_did().map(|(did, _)| did).as_deref(), Some(DID)); } /// An ordinary email is not a DID. Everyone who has not cloned through @@ -985,13 +1060,17 @@ mod tests { "did:key:z6MkhaXgBZDvotDkL5257faiz", ] { repo.git(&["config", "--local", "user.email", did]); - assert_eq!(super::repo_did().as_deref(), Some(did), "did: {did:?}"); + assert_eq!( + super::repo_did().map(|(d, _)| d).as_deref(), + Some(did), + "did: {did:?}" + ); } // And the decoration a hand-edited config can carry is removed // rather than making the DID not match. repo.git(&["config", "--local", "user.email", &format!(" {DID} ")]); - assert_eq!(super::repo_did().as_deref(), Some(DID)); + assert_eq!(super::repo_did().map(|(did, _)| did).as_deref(), Some(DID)); } /// The DID branch of `resolve_spec` returns before any request is made, @@ -1511,6 +1590,27 @@ mod tests { /// both an unconfigured checkout *and* what `Checkout::Ignored` produces /// regardless. Every row therefore says what `repo configure` does as /// well as what every other command does. + /// One combination out of ten means "every worktree of this repo is this + /// account", and it is the shared file read from a worktree. + /// + /// The main checkout reading the shared file is not it: that is the only + /// file a main checkout has, and nothing is being shared away. A worktree + /// reading its *own* file is the state this warning exists to ask for. + #[test] + fn only_the_shared_file_read_from_a_worktree_is_a_shared_identity() { + use super::{Source, shares_identity_with_every_worktree as shared}; + assert!(shared(Source::RepoConfig, true)); + assert!(!shared(Source::RepoConfig, false)); + assert!(!shared(Source::WorktreeConfig, true)); + for source in [Source::Flag, Source::Env, Source::Named, Source::Sole] { + assert!(!shared(source, true), "{source:?} is not about a file"); + assert!(!shared(source, false), "{source:?} is not about a file"); + } + // Rank 4 cannot be reached in a worktree at all, so it can never be + // the reason one is sharing an identity. + assert!(!shared(Source::Switched, true)); + } + #[test] fn the_local_ranks_decide_the_same_way_every_time() { let one = [known(DID, Some("permadeath.com"))]; @@ -1520,44 +1620,100 @@ mod tests { // (accounts, repo user.email, default pointer, expected), where a // `None` expectation means "refuse, and say what there is". type Expect<'a> = Option<(&'a str, Source)>; - type Case<'a> = (&'a [Known], Option<&'a str>, Option<&'a str>, Expect<'a>); - let cases: [Case<'_>; 10] = [ + use crate::clients::git::config::Scope; + type Case<'a> = ( + &'a [Known], + Option<(&'a str, Scope)>, + Option<&'a str>, + bool, + Expect<'a>, + ); + let repo_says = |did| Some((did, Scope::Repo)); + let worktree_says = |did| Some((did, Scope::Worktree)); + let cases: [Case<'_>; 16] = [ // Rule 3 wins outright when the checkout names an account we hold, // including over a pointer aimed somewhere else. This is the row // `Checkout::Ignored` exists to change. - (&one, Some(DID), None, Some((DID, Source::RepoConfig))), + ( + &one, + repo_says(DID), + None, + false, + Some((DID, Source::RepoConfig)), + ), ( &two, - Some(DID), + repo_says(DID), Some(OTHER), + false, Some((DID, Source::RepoConfig)), ), + // The same value out of the worktree's own file reports itself as + // that, because the two are worth telling apart: one is shared + // with every worktree of the repo and the other is not. + ( + &two, + worktree_says(DID), + Some(OTHER), + true, + Some((DID, Source::WorktreeConfig)), + ), // …and the same inputs with the checkout ignored: the pointer // takes over, which is the rewrite `repo configure` performs. - (&two, None, Some(OTHER), Some((OTHER, Source::Switched))), + ( + &two, + None, + Some(OTHER), + false, + Some((OTHER, Source::Switched)), + ), // A checkout naming an account we hold no session for is a hard // stop, never a fall-through to somebody else. - (&two, Some(unheld), Some(OTHER), None), + (&two, repo_says(unheld), Some(OTHER), false, None), // Ignoring the checkout is also what makes that case *fixable*: // the same repo, with rule 3 skipped, selects fine. - (&one, None, None, Some((DID, Source::Sole))), + (&one, None, None, false, Some((DID, Source::Sole))), // Rule 4: the pointer, when it names something we hold. - (&two, None, Some(DID), Some((DID, Source::Switched))), + (&two, None, Some(DID), false, Some((DID, Source::Switched))), // A pointer at an account that has since been logged out is not // an error — it falls through to rank 5. - (&one, None, Some(unheld), Some((DID, Source::Sole))), - (&two, None, Some(unheld), None), + (&one, None, Some(unheld), false, Some((DID, Source::Sole))), + (&two, None, Some(unheld), false, None), // Rule 5: a sole account needs no configuration at all. - (&one, None, None, Some((DID, Source::Sole))), + (&one, None, None, false, Some((DID, Source::Sole))), // Nothing to go on and more than one candidate: refuse. - (&two, None, None, None), + (&two, None, None, false, None), + // ---- and the same three questions asked inside a linked worktree. + // + // Rule 4 is gone there. A worktree that has not been told whose it + // is must say so rather than inherit whoever the machine last + // defaulted to — which is how an agent publishes as the person who + // owns the laptop. + (&two, None, Some(DID), true, None), + (&two, None, Some(OTHER), true, None), + // Rule 5 is *not* gone: one account is not a default, it is the + // only answer, and taking it away would break `repo configure` in + // a worktree for anybody with a single account. + (&one, None, None, true, Some((DID, Source::Sole))), + (&one, None, Some(unheld), true, Some((DID, Source::Sole))), + // The checkout still outranks everything either way. + ( + &two, + repo_says(DID), + Some(OTHER), + true, + Some((DID, Source::RepoConfig)), + ), ]; - for (i, (accounts, repo, active, expected)) in cases.into_iter().enumerate() { - let got = super::select_local(accounts, repo, active); + for (i, (accounts, repo, active, linked, expected)) in cases.into_iter().enumerate() { + let got = super::select_local(accounts, repo, active, linked); match (got, expected) { (Ok(s), Some((did, source))) => { - assert_eq!(s.did, did, "case {i}: repo={repo:?} active={active:?}"); + assert_eq!( + s.did, did, + "case {i}: repo={repo:?} active={active:?} linked={linked}" + ); assert_eq!(s.source, source, "case {i}"); } (Err(e), None) => { @@ -1588,14 +1744,19 @@ mod tests { let two = [known(DID, Some("permadeath.com")), known(OTHER, None)]; let unheld = "did:plc:aaaaaaaaaaaaaaaaaaaaaaaa"; - let missing = super::select_local(&one, Some(unheld), None) - .expect_err("the checkout names an account with no session"); + let missing = super::select_local( + &one, + Some((unheld, crate::clients::git::config::Scope::Repo)), + None, + false, + ) + .expect_err("the checkout names an account with no session"); assert_eq!( crate::exit::classify(&missing), crate::exit::Exit::NoSession ); - let ambiguous = super::select_local(&two, None, None) + let ambiguous = super::select_local(&two, None, None, false) .expect_err("two accounts and nothing choosing between them"); assert_eq!(crate::exit::classify(&ambiguous), crate::exit::Exit::Usage); } @@ -1607,9 +1768,14 @@ mod tests { let accounts = [known(DID, Some("permadeath.com"))]; let unheld = "did:plc:aaaaaaaaaaaaaaaaaaaaaaaa"; - let err = super::select_local(&accounts, Some(unheld), None) - .expect_err("the checkout names an account with no session") - .to_string(); + let err = super::select_local( + &accounts, + Some((unheld, crate::clients::git::config::Scope::Repo)), + None, + false, + ) + .expect_err("the checkout names an account with no session") + .to_string(); assert!(err.contains(unheld), "{err}"); assert!(err.contains("--account"), "{err}"); } @@ -1625,7 +1791,9 @@ mod tests { repo.git(&["config", "--local", "user.email", DID]); assert_eq!( - super::checkout_did(super::Checkout::Consulted).as_deref(), + super::checkout_did(super::Checkout::Consulted) + .map(|(d, _)| d) + .as_deref(), Some(DID) ); assert_eq!(super::checkout_did(super::Checkout::Ignored), None); diff --git a/src/docs.rs b/src/docs.rs index b0bec4a..05c98d7 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -14,6 +14,10 @@ pub mod architecture { #![doc = include_str!("../docs/architecture.md")] } +pub mod accounts { + #![doc = include_str!("../docs/accounts.md")] +} + pub mod output { #![doc = include_str!("../docs/output.md")] } diff --git a/src/help.rs b/src/help.rs index 042b473..40f828b 100644 --- a/src/help.rs +++ b/src/help.rs @@ -94,8 +94,12 @@ fn template(styles: &Styles) -> String { /// Render the visible subcommands as headed groups. /// -/// Hidden commands are skipped: `login` is kept working so nobody's shell -/// history breaks, and listing it would undo the point of hiding it. +/// The filter is mechanical, not a policy: nothing in this tree is hidden. +/// It used to be — a top-level `login` alias was kept out of the listing so +/// that shell history would not break — and that alias is gone, along with +/// the `auth log` one that `main`'s tests pin as a parse failure. atgc does +/// not keep hidden aliases; a spelling that changes changes, and the error +/// says where the command went. fn command_list(cmd: &Command) -> StyledStr { let styles = cmd.get_styles(); let header = styles.get_header();