diff --git a/CHANGELOG.md b/CHANGELOG.md index a4d7beb..0c8b1a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,57 @@ that may never merge. They are not releases and are not listed here. reads, for anyone whose employer does not allow piping a script into a shell. +### Changed + +- The skill `mapbox generate-skills` writes now tells an agent which of the + three ways to authenticate it can actually use. It listed all three without + comment, so an agent with no token would read "credentials stored by + `mapbox auth login`" as an option and try it — a command that opens a + browser and waits for a person, which in a coding-agent session can only + fail. It now says to have `MAPBOX_ACCESS_TOKEN` set, and to ask for one + rather than reaching for `auth login`. Reported from a real session that hit + exactly this. + +- `mapbox generate-skills` now prints the command that removes what it wrote, + in both output modes, and the JSON carries it as `remove_with`. The write is + `generate-skills` and the undo is `agent-skills uninstall`, a differently + named command belonging to a different feature, so there was no way to get + from one to the other — a reader of `generate-skills --help` saw no way back. + Reported after an agent reached for `rm -rf` instead, had it refused by its + sandbox, and left untracked directories in a git working tree. A dry run does + not print it, since nothing was written. `--help`, the generated reference + page and the README say it too. + +- Advice that is a command to paste now names every shell instead of guessing + one. The repair for a file blocking the credential directory offered `mv` and + `export NAME="$(cat …)"`, which a Windows reader cannot run; it now gives + `mv`/`Move-Item`/`move` and all four of `export`, fish's `set -gx`, + PowerShell's `$env: … Get-Content` and `cmd.exe`'s `set /p`, each labeled + with the shell it belongs to. The tip printed after a successful login no + longer offers `export MAPBOX_USERNAME=…` either; it says what to set rather + than how. + + Keying this on the operating system would have been wrong in both + directions, which is why it does not: PowerShell runs on macOS and Linux and + Git Bash runs on Windows. Each row also uses the name its shell owns rather + than one that may be aliased — `Move-Item` and `Get-Content` rather than `mv` + and `cat`, which resolve differently depending on what is installed. + +- The refusal from `mapbox auth login` with no terminal now names both ways + out. It said "Set MAPBOX_ACCESS_TOKEN for a script or a CI job", which + describes automation and assumes that is who is asking; a person working + through a coding agent hits this too, and hits it again when they try the + command themselves in that agent's shell, which has no terminal either. The + fix now leads with running it in a terminal window and keeps the token as the + answer for a script, a CI job or an agent. + +- A file sitting where the credential directory belongs now says so when it + looks like an access token, which the one older Mapbox tools left at + `~/.mapbox` does. The advice was to move it aside, with nothing to tell the + reader whether they were moving junk or a working credential; it now says + the token still works and gives the `MAPBOX_ACCESS_TOKEN` line that keeps + using it. The token itself is never printed, and a test holds that. + ### Fixed - The README described the published builds as signed. They are not diff --git a/README.md b/README.md index 23ae69e..872721e 100644 --- a/README.md +++ b/README.md @@ -266,6 +266,17 @@ Skill](https://code.claude.com/docs/en/skills): `.claude/skills` for Claude Code, `.agents/skills` for Codex. `--agent`, `--global`, `--dir`, and `--service` narrow it; `--dry-run` lists files without writing them. +Without `--global` it writes into the current project, once per agent it +finds, and it prints every directory it used. To take them out again: + +```sh +mapbox agent-skills uninstall mapbox-cli +``` + +That removes every copy this command wrote, which is more than deleting the +directories by hand usually catches — a default run writes for each agent on +the machine, not just the one you had in mind. + ### Tileset CLI ```sh diff --git a/src/auth.rs b/src/auth.rs index 2695298..bd81c80 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -246,34 +246,154 @@ pub(crate) fn config_dir_path() -> Option { #[derive(Debug)] struct DirectoryBlocked { path: PathBuf, + /// Whether the file looks like the token older Mapbox tooling left here. + holds_a_token: bool, +} + +/// Does this file look like the one-line token file older tooling wrote? +/// +/// Worth answering because the answer changes the advice. "Move it aside" is +/// the fix either way, but a reader who does not know what the file *is* +/// cannot tell whether moving it loses something — and in this case it holds +/// a working credential they can keep using in one line. +/// +/// Deliberately shallow. The read is bounded, because nothing guarantees the +/// thing in the way is small, and a wrong answer here costs a sentence of +/// advice rather than a failed command. Any read error is a `false`: this +/// runs while reporting a different problem and must not replace it. +fn looks_like_a_legacy_token_file(path: &Path) -> bool { + use std::io::Read; + + // Longer than any token Mapbox issues, short enough that a stray file + // is not worth reading. + const MOST: usize = 512; + + let Ok(mut file) = std::fs::File::open(path) else { + return false; + }; + let mut buffer = vec![0; MOST]; + let Ok(read) = file.read(&mut buffer) else { + return false; + }; + let Ok(text) = std::str::from_utf8(&buffer[..read]) else { + return false; + }; + + let token = text.trim(); + ["pk.", "sk.", "tk."].iter().any(|p| token.starts_with(p)) + && !token.contains(char::is_whitespace) } impl DirectoryBlocked { + /// Moving the file aside, in every shell this CLI can be run from. + /// + /// Each row uses the name that shell owns, not one that might be aliased + /// to it. `mv` in PowerShell is an alias for `Move-Item` on Windows and no + /// alias at all on Unix, where the native tool is found instead — and a + /// machine with GNU coreutils installed has a third answer. None of that + /// is knowable from here, so the PowerShell row says `Move-Item`, which is + /// a cmdlet in `Microsoft.PowerShell.Management` and cannot be shadowed + /// out from under the reader. + /// + /// This is what keying on the *shell* buys over keying on the operating + /// system. An earlier version picked `Move-Item` for Windows and `mv` + /// everywhere else, which handed Git Bash on Windows a cmdlet it does not + /// have and PowerShell on macOS a syntax it does not use. + /// + /// Paths are quoted because a Windows home directory routinely contains a + /// space, and an unquoted `C:\Users\Jane Smith\.mapbox` is two arguments. + fn move_aside(&self) -> Vec { + let shown = self.path.display(); + vec![ + format!("bash, zsh, fish: mv '{shown}' '{shown}.bak'"), + format!("PowerShell: Move-Item '{shown}' '{shown}.bak'"), + format!("cmd.exe: move \"{shown}\" \"{shown}.bak\""), + ] + } + + /// Setting the token, in every shell, because there is no common spelling. + /// + /// `export` with `$(…)`, fish's `set -gx` with `(…)`, PowerShell's `$env:` + /// with `Get-Content`, and `cmd.exe`'s `set /p` reading a redirect. + /// + /// The fish row is not there because `export` is missing — fish ships an + /// `export` function for bash compatibility, and the bash row does work + /// there (checked on fish 4.9). It is there because `set -gx` is what a + /// fish user writes, and because a compatibility shim in someone else's + /// shell is a thinner promise than that shell's own spelling. + /// + /// The label goes in front rather than in a trailing `# comment`, because + /// `#` does not start a comment in `cmd.exe` — a trailing label would be + /// part of the command for the one reader least able to spot it. + fn keep_the_token(&self) -> Vec { + let shown = self.path.display(); + vec![ + format!("bash, zsh: export {CLAP_TOKEN_ENV}=\"$(cat '{shown}.bak')\""), + format!("fish: set -gx {CLAP_TOKEN_ENV} (cat '{shown}.bak')"), + format!("PowerShell: $env:{CLAP_TOKEN_ENV} = Get-Content '{shown}.bak'"), + format!("cmd.exe: set /p {CLAP_TOKEN_ENV}=<\"{shown}.bak\""), + ] + } + /// The same fact in one line, fix included. /// - /// The `mv` stays. Pointing at another command to *learn* the fix would - /// send the reader to one that fails for this very reason, and "run + /// One line means one shell's spelling, so it is `mv` — the one that works + /// in four of the five. A `cmd.exe` reader gets the full form from any + /// `auth` command, which is where the repair actually belongs. + /// + /// The move command stays. Pointing at another command to *learn* the fix + /// would send the reader to one that fails for this very reason, and "run /// `auth login`" reads as "you need to log in" when logging in is exactly /// what cannot help. fn one_line(&self) -> String { let shown = self.path.display(); format!( "{shown} is a file, not a directory, so no stored credentials can be read. \ - Move it aside: mv {shown} {shown}.bak" + Move it aside: mv '{shown}' '{shown}.bak'" ) } + + /// The whole message. + /// + /// Every shell is spelled out rather than one being guessed at. The guess + /// this replaces was `cfg!(windows)`, which is the wrong question: it names + /// the operating system and the answer depends on the shell. PowerShell + /// runs on macOS and Linux, Git Bash runs on Windows, and that fork handed + /// both of them the other one's syntax. + fn rendered(&self) -> String { + let shown = self.path.display(); + let indented = |lines: Vec| { + lines + .into_iter() + .map(|line| format!(" {line}")) + .collect::>() + .join("\n") + }; + + let mut out = format!( + "{shown} is a file, but that is the directory credentials are stored in.\n\n\ + Move it aside to continue:\n\n{}\n\n\ + Or set {CONFIG_DIR_ENV} to keep credentials somewhere else entirely.", + indented(self.move_aside()) + ); + // Only once the reader knows the fix. The token is never printed: it + // is a live credential, and this text reaches logs and terminals that + // the file's permissions were protecting it from. + if self.holds_a_token { + out.push_str(&format!( + "\n\nIt holds what looks like an access token, left by older Mapbox \ + tooling. Nothing is lost by moving it — the token still works, and \ + `{CLAP_TOKEN_ENV}` is how to keep using it:\n\n{}", + indented(self.keep_the_token()) + )); + } + out + } } impl std::fmt::Display for DirectoryBlocked { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let shown = self.path.display(); - write!( - f, - "{shown} is a file, but that is the directory credentials are stored in.\n\n\ - Move it aside to continue:\n\n \ - mv {shown} {shown}.bak\n\n\ - Or set {CONFIG_DIR_ENV} to keep credentials somewhere else entirely." - ) + write!(f, "{}", self.rendered()) } } @@ -291,6 +411,7 @@ fn prepare_config_dir(dir: &Path) -> Result<()> { if dir.exists() && !dir.is_dir() { return Err(DirectoryBlocked { path: dir.to_path_buf(), + holds_a_token: looks_like_a_legacy_token_file(dir), } .into()); } @@ -987,7 +1108,7 @@ fn nothing_to_report(use_login: bool, profile: Option<&str>) -> anyhow::Error { profile_name(profile) ) } else { - format!("Run `mapbox auth login`, export {CLAP_TOKEN_ENV}, or pass `--token`.") + format!("Run `mapbox auth login`, set {CLAP_TOKEN_ENV}, or pass `--token`.") }; CliError::new("not_authenticated", "No Mapbox token available.") @@ -1722,9 +1843,19 @@ fn login_has_no_way_to_show_the_url() -> anyhow::Error { /// could never complete. A flag about confirmations has no business asserting /// that a human is present. /// -/// So the answer for a headless caller is a token, and the fix says so. A -/// login on a machine with no terminal at all wants the device authorization -/// grant, which is a feature, not an escape hatch on this one. +/// The fix names both ways out, because there are two kinds of caller here +/// and only one of them is headless. +/// +/// This said only "set MAPBOX_ACCESS_TOKEN for a script or a CI job", which +/// describes automation and quietly assumes that is who is asking. Often it +/// is not: a person working through a coding agent hits this, and so does the +/// same person when they try the command themselves in that agent's shell — +/// which has no terminal either, so it fails identically. For them a token is +/// the *workaround* and the real answer is a terminal window, which the +/// message never mentioned. Reported by somebody who went looking for it. +/// +/// A login on a machine with no terminal at all wants the device +/// authorization grant, which is a feature, not an escape hatch on this one. fn login_needs_a_terminal() -> anyhow::Error { CliError::new( "interactive_required", @@ -1733,7 +1864,12 @@ fn login_needs_a_terminal() -> anyhow::Error { ) .with_remedy( Remedy::default() - .with_fix("Set MAPBOX_ACCESS_TOKEN for a script or a CI job.") + .with_fix( + "Run it in a terminal window — a shell an agent or an editor runs \ + commands through has no terminal, so the same command fails there \ + the same way. Or set MAPBOX_ACCESS_TOKEN, which is what a script, a \ + CI job or an agent should use.", + ) .with_doc(Some(remedy::TOKENS_DOC)), ) .into() @@ -1816,7 +1952,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { let text = match &creds.username { Some(u) => format!( "Logged in as {u}{profile_note}.\n\ - Tip: export MAPBOX_USERNAME={u} to skip --username on each command." + Tip: setting MAPBOX_USERNAME to {u} skips --username on each command." ), None => format!("Logged in successfully{profile_note}."), }; @@ -2604,7 +2740,7 @@ mod tests { assert!(full.contains("is a file"), "{full}"); assert!(full.contains(&path.display().to_string()), "{full}"); assert!( - full.contains("mv "), + full.contains("mv ") || full.contains("Move-Item "), "the message has to carry the fix, not just the diagnosis: {full}" ); assert!( @@ -2614,14 +2750,14 @@ mod tests { // The same fact for a command that is not about credentials at all, // which only needs to explain why the stored ones went missing. - let brief = err + let blocked = err .downcast_ref::() - .expect("the obstruction has to survive as its own type") - .one_line(); + .expect("the obstruction has to survive as its own type"); + let brief = blocked.one_line(); assert_eq!(brief.lines().count(), 1, "{brief}"); assert!(brief.contains("not a directory"), "{brief}"); assert!( - brief.contains(&format!("mv {}", path.display())), + brief.contains(&path.display().to_string()), "the short form still carries the fix itself: {brief}" ); assert!( @@ -2630,6 +2766,92 @@ mod tests { ); } + /// Every shell gets a line it can actually run. + /// + /// The advice is meant to be pasted, so it has to be in the reader's + /// language. This started as `export …="$(cat …)"` unconditionally, became + /// a `cfg!(windows)` fork, and neither was right: the operating system does + /// not determine the shell. PowerShell runs on macOS and Linux, Git Bash + /// runs on Windows, and that fork handed each of them the other's syntax. + /// + /// So every shell is spelled out and none is guessed at. Checked by hand + /// where a shell was available: fish 4.9 runs the fish row and leaves the + /// variable exported, and pwsh 7.6 has no `mv` alias but does have + /// `Move-Item`. The `cmd.exe` rows are from its documented syntax; there + /// is no Windows machine here to run them on, and that is worth knowing + /// rather than papering over. + #[test] + fn every_shell_gets_a_line_it_can_run() { + const TOKEN: &str = "sk.eyJ1IjoiZmFrZSJ9.not-a-real-token"; + + let path = scratch("config-dir-shells").join(".mapbox"); + std::fs::write(&path, format!("{TOKEN}\n")).unwrap(); + let blocked = DirectoryBlocked { + path: path.clone(), + holds_a_token: true, + }; + let full = blocked.rendered(); + + // Moving it aside: each shell's own name for the operation, never one + // that depends on an alias being present. + for expected in [ + &format!("mv '{}'", path.display()), + &format!("Move-Item '{}'", path.display()), + &format!("move \"{}\"", path.display()), + ] { + assert!(full.contains(expected), "no row for {expected}: {full}"); + } + + // Keeping the token: four genuinely different languages. + for expected in [ + &format!("export {CLAP_TOKEN_ENV}=\"$(cat "), + &format!("set -gx {CLAP_TOKEN_ENV} (cat "), + &format!("$env:{CLAP_TOKEN_ENV} = Get-Content "), + &format!("set /p {CLAP_TOKEN_ENV}=<"), + ] { + assert!(full.contains(expected), "no row for {expected}: {full}"); + } + + // Labels lead rather than trail. `#` does not start a comment in + // `cmd.exe`, so a trailing label would be part of the command for the + // one reader least equipped to notice. + assert!( + !full.contains(" # "), + "a trailing label is part of the command in cmd.exe: {full}" + ); + + // Quoted, because a Windows home directory routinely contains a space + // and an unquoted path is two arguments. + assert!( + full.contains(&format!("'{}.bak'", path.display())) + && full.contains(&format!("\"{}.bak\"", path.display())), + "paths have to be quoted in both quoting styles: {full}" + ); + + // The one thing this must never do. A live credential in an error + // message reaches every terminal and log the file's 0600 permissions + // were keeping it out of. + assert!( + !full.contains(TOKEN), + "the token itself must never be printed: {full}" + ); + } + + /// And the sentence is earned rather than always shown. + #[test] + fn a_file_that_is_not_a_token_gets_no_token_advice() { + let path = scratch("config-dir-not-a-token").join(".mapbox"); + std::fs::write(&path, "[profile default]\nsomething = else\n").unwrap(); + + let full = prepare_config_dir(&path).unwrap_err().to_string(); + + assert!(full.contains("is a file"), "{full}"); + assert!( + !full.contains(CLAP_TOKEN_ENV), + "nothing here is a token, so the advice would be a guess: {full}" + ); + } + #[test] fn the_config_directory_is_created_and_restricted() { let path = scratch("config-dir-create").join(".mapbox"); diff --git a/src/generate_skills.rs b/src/generate_skills.rs index 4936e0c..fe3e0e2 100644 --- a/src/generate_skills.rs +++ b/src/generate_skills.rs @@ -111,7 +111,12 @@ pub fn command() -> Command { committed and a diff means the CLI changed. Nothing here reaches the \ network, and no token is needed.\n\n\ With no flags, writes into the project directory of every agent whose \ - home directory is present." + home directory is present — so a run usually writes several places, \ + all of them named in the output.\n\n\ + `mapbox agent-skills uninstall mapbox-cli` removes every copy again. \ + Reach for that rather than deleting the directories by hand: it knows \ + all of the places this command writes to, and a sandboxed agent is \ + often allowed to run it when it is not allowed to remove files." )) .args(skill_dest::args()) .arg( @@ -478,7 +483,13 @@ fn render_body( out.push_str(&format!("{}. {prose}\n", index + 1)); } out.push_str( - "\nThe account a URL asks for is separate from the token: pass `--username`/`-u`, \ + "\nIf you are an agent, step 2 is the one that works: have \ + `MAPBOX_ACCESS_TOKEN` set before running anything, and ask the person you are \ + working for to provide one from account.mapbox.com if it is missing. Do not \ + reach for `mapbox auth login` when a command reports no token — it cannot \ + succeed without a human at a browser, and asking is faster than finding that \ + out.\n\n\ + The account a URL asks for is separate from the token: pass `--username`/`-u`, \ set `MAPBOX_USERNAME`, or let a stored login supply it. A command whose URL has \ an account placeholder cannot run without one.\n\n\ `--use-login` removes step 2, and only step 2 — a `--token` typed on the command \ @@ -1210,7 +1221,10 @@ fn token_precedence() -> [(crate::auth::TokenSource, &'static str); 3] { ( TokenSource::Login, "Credentials stored by `mapbox auth login`, refreshed automatically when \ - stale. `--profile ` picks which set.", + stale. `--profile ` picks which set. **`auth login` is the one \ + command here you cannot run.** It opens a browser and waits for a person \ + to approve, so without a terminal it refuses with `interactive_required` \ + rather than hanging. Use step 2.", ), ] } @@ -1377,6 +1391,24 @@ fn write_tree(root: &Path, files: &[GeneratedFile]) -> Result<()> { Ok(()) } +/// The command that removes what this one wrote. +/// +/// Printed with the destinations, and it exists because leaving it out cost +/// somebody real work. A coding agent generated skills into a project, found +/// them redundant, and reached for `rm -rf` — which its sandbox refused, +/// leaving untracked directories in a git working tree for a human to clear +/// by hand. The undo was there the whole time; nothing pointed at it. +/// +/// Nothing *would* have. The command that writes is `generate-skills` and the +/// command that removes is `agent-skills uninstall`, which is named for a +/// different feature and documented on a different page. No amount of reading +/// `generate-skills --help` gets you there. +/// +/// It is also the more correct cleanup than the one a person would type: a +/// default run writes to every agent it detects, so the `rm -rf` in that +/// report named two directories where three had been written. +const HOW_TO_REMOVE: &str = "Remove them with: mapbox agent-skills uninstall mapbox-cli"; + /// What was written, or what would have been. fn report(mode: Mode, plans: &[Plan], dry_run: bool) -> Result<()> { let mut lines: Vec = vec![]; @@ -1400,9 +1432,20 @@ fn report(mode: Mode, plans: &[Plan], dry_run: bool) -> Result<()> { } } + // After the list rather than before it: on a dry run the reader has not + // written anything yet, and on a real one this is what they need next. + if !dry_run { + lines.push(String::new()); + lines.push(HOW_TO_REMOVE.to_string()); + } + let json = json!({ "dry_run": dry_run, "skill": SKILL_NAME, + // The same fact where a program reads, not only where a person does — + // an agent is the caller that most needs it and the least likely to + // be parsing the text rendering. + "remove_with": (!dry_run).then_some("mapbox agent-skills uninstall mapbox-cli"), "destinations": plans .iter() .map(|plan| json!({ diff --git a/tests/generate_skills.rs b/tests/generate_skills.rs index 3083686..1d9f12d 100644 --- a/tests/generate_skills.rs +++ b/tests/generate_skills.rs @@ -457,3 +457,71 @@ fn no_token_is_needed_and_the_help_says_what_it_does() { assert!(help.contains(flag), "--help does not mention {flag}"); } } + +/// Writing tells you how to unwrite, in both renderings. +/// +/// From a real report: an agent generated skills into a project, found them +/// redundant, and reached for `rm -rf` — which its sandbox refused, leaving +/// untracked directories in a git working tree for a person to clear by hand. +/// The undo existed the whole time and nothing pointed at it, and nothing +/// would have: the command that writes is `generate-skills` and the command +/// that removes is `agent-skills uninstall`, named for a different feature. +/// +/// Both renderings, because the caller who most needs this is the one least +/// likely to be reading the text one. +#[test] +fn what_was_written_carries_the_command_that_removes_it() { + const REMOVAL: &str = "mapbox agent-skills uninstall mapbox-cli"; + + let dir = scratch("removal-hint"); + let out = dir.join("out"); + std::fs::create_dir_all(&out).expect("create the destination"); + let target = out.to_str().unwrap(); + + let text = stdout(&generate( + "removal-hint-text", + &["generate-skills", "--dir", target, "-o", "text"], + )); + assert!( + text.contains(REMOVAL), + "the text output does not say how to remove what it wrote: {text}" + ); + + let json = stdout(&generate( + "removal-hint-json", + &["generate-skills", "--dir", target, "--force", "-o", "json"], + )); + let parsed: serde_json::Value = serde_json::from_str(&json).expect("JSON on stdout"); + assert_eq!( + parsed["remove_with"], REMOVAL, + "the JSON output does not carry it: {json}" + ); +} + +/// And a dry run does not, because nothing is there to remove. +/// +/// Advice to undo something that did not happen is noise, and worse, it +/// implies the write went ahead. +#[test] +fn a_dry_run_does_not_offer_to_remove_what_it_did_not_write() { + let dir = scratch("removal-hint-dry"); + let out = dir.join("out"); + std::fs::create_dir_all(&out).expect("create the destination"); + + let json = stdout(&generate( + "removal-hint-dry", + &[ + "generate-skills", + "--dry-run", + "--dir", + out.to_str().unwrap(), + "-o", + "json", + ], + )); + let parsed: serde_json::Value = serde_json::from_str(&json).expect("JSON on stdout"); + assert!( + parsed["remove_with"].is_null(), + "a dry run offered a removal for files it never wrote: {json}" + ); +} diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index 96f058b..bdb02f6 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -128,12 +128,23 @@ fn the_refusal_carries_a_code_and_a_fix() { assert_eq!(error_code(&out), "interactive_required"); let value: serde_json::Value = serde_json::from_str(stderr(&out).trim()).expect("JSON on stderr"); + let fix = value["fix"].as_str().unwrap_or_default(); assert!( - value["fix"] - .as_str() - .is_some_and(|fix| fix.contains("MAPBOX_ACCESS_TOKEN")), - "the fix is the actionable half, and the only action that helps here \ - is a token: {}", + fix.contains("MAPBOX_ACCESS_TOKEN"), + "the fix is the actionable half, and a token is what unblocks a script, \ + a CI job or an agent: {}", + stderr(&out) + ); + // The other way out, and the one this used to omit. A person working + // through a coding agent lands here, and so does that person when they try + // the command themselves in the agent's shell — which has no terminal + // either, so it fails identically. For them the token is the workaround and + // a terminal window is the answer, and a fix that names only the token + // reads as "you are automation" to somebody who is not. + assert!( + fix.contains("terminal"), + "a human who can open a terminal is told to set an environment \ + variable instead: {}", stderr(&out) ); // Both ways out are changes to how the command is invoked, so there is no