mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-17 15:34:54 +00:00
test: check every surface, and test the thing that edits upstream
The branding check read `dev --help` only, which is why `dev exec` announced "OpenAI Codex" as its banner for a while — `--help` never shows it. It now reads every subcommand's help, and found four more leaks by itself: exec's usage line, its --strict-config and --skip-git-repo-check text, and `-c/--config` still naming ~/.codex/config.toml. That last one was recorded as unreachable when a copied island could not include utils/cli; editing the checkout in place made it reachable. Two things keep the word legitimately and are exempt: CODEX_HOME, an environment variable upstream owns, and `codex/...`, which names a method on the app-server wire protocol. Renaming either breaks something real. Branding is the product name a reader sees, not identifiers on a wire. hanzo-upstream had no tests, and it is the code that edits upstream in place — including an idempotence bug shipped earlier today that stacked appending edits until `hanzo-tui` appeared three times in one manifest. The anchor logic is now a pure function with six tests, one of which is that exact regression, and one of which asserts every shipped anchor is well formed. `make test` reaches the crate now; being a detached workspace, the root run could not see it. Cloud tasks are Hanzo Tasks, not the upstream service's name.
This commit is contained in:
@@ -23,8 +23,12 @@ install: release
|
||||
@$(CARGO) install --locked --path crates/hanzo-dev --bin dev
|
||||
|
||||
## test: run the Hanzo test suite
|
||||
# hanzo-upstream is a detached workspace, so the root run cannot see it — and it
|
||||
# is the code that edits upstream in place, which is the last thing to leave
|
||||
# untested.
|
||||
test: prepare
|
||||
@$(CARGO) nextest run --locked --no-fail-fast $(ARGS)
|
||||
@$(CARGO) nextest run --no-fail-fast --manifest-path crates/hanzo-upstream/Cargo.toml $(ARGS)
|
||||
|
||||
## test-upstream: run the upstream suite against the pinned submodule
|
||||
# voice-host wants GStreamer >= 1.28, which no current distribution ships, and
|
||||
|
||||
@@ -32,6 +32,13 @@ account. Then, from any repository:
|
||||
hanzo dev
|
||||
```
|
||||
|
||||
The standalone agent also supports `dev login`. Its arrow-key picker defaults
|
||||
to Hanzo and offers ChatGPT sign-in or hidden entry of a Hanzo or OpenAI API key.
|
||||
Run `dev login --hanzo` or `dev login --chatgpt` to select an account directly.
|
||||
`dev login --with-api-key` accepts a Hanzo key; add `--chatgpt` for an OpenAI key.
|
||||
Keys are hidden when typed or pasted, and can also be read from piped stdin.
|
||||
Launch `dev` to use the account selected there.
|
||||
|
||||
Give it the task on the command line to run headless instead:
|
||||
|
||||
```bash
|
||||
|
||||
@@ -51,19 +51,52 @@ fn completion_scripts_name_the_product() {
|
||||
assert!(text(&dev(home.path(), &["completion", "zsh"])).contains("compdef dev"));
|
||||
}
|
||||
|
||||
/// Every surface a user reads, not just the front page. The banner `dev exec`
|
||||
/// prints was upstream's for a while precisely because only `--help` was
|
||||
/// checked, and `--help` never shows it.
|
||||
#[test]
|
||||
fn the_interface_carries_no_upstream_product_name() {
|
||||
fn no_surface_carries_an_upstream_product_name() {
|
||||
let home = home();
|
||||
let help = text(&dev(home.path(), &["--help"]));
|
||||
for name in ["OpenAI", "Codex", "ChatGPT Codex"] {
|
||||
// `CODEX_HOME` still reaches the profile flag from a crate the island
|
||||
// cannot reach. Everything else is ours to answer for.
|
||||
let leaked: Vec<_> = help
|
||||
.lines()
|
||||
.filter(|line| line.contains(name) && !line.contains("CODEX_HOME"))
|
||||
.collect();
|
||||
assert!(leaked.is_empty(), "{name} reaches the interface: {leaked:?}");
|
||||
let mut surfaces = vec![("--help".to_string(), text(&dev(home.path(), &["--help"])))];
|
||||
for command in COMMANDS {
|
||||
surfaces.push((
|
||||
format!("{command} --help"),
|
||||
text(&dev(home.path(), &[command, "--help"])),
|
||||
));
|
||||
}
|
||||
for (surface, body) in surfaces {
|
||||
for name in ["OpenAI", "Codex", "codex"] {
|
||||
// Login names the provider whose credential the user is supplying.
|
||||
if surface == "login --help" && name == "OpenAI" {
|
||||
continue;
|
||||
}
|
||||
// Two things legitimately carry the word and must not be renamed:
|
||||
// `CODEX_HOME`, an environment variable upstream owns, and
|
||||
// `codex/…`, which names a method on the app-server wire protocol.
|
||||
// Renaming either would break something real; branding is about the
|
||||
// product name a reader sees, not identifiers on a wire.
|
||||
let leaked: Vec<_> = body
|
||||
.lines()
|
||||
.filter(|line| {
|
||||
line.contains(name)
|
||||
&& !line.contains("CODEX_HOME")
|
||||
&& !line.contains("`codex/")
|
||||
})
|
||||
.collect();
|
||||
assert!(leaked.is_empty(), "`dev {surface}` says {name}: {leaked:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The product answers for one version, everywhere it states one.
|
||||
#[test]
|
||||
fn the_product_states_one_version() {
|
||||
let home = home();
|
||||
let version = text(&dev(home.path(), &["--version"]));
|
||||
let number = version.split_whitespace().nth(1).expect("`dev <version>`").to_string();
|
||||
assert!(version.starts_with("dev "), "{version}");
|
||||
let doctor = text(&dev(home.path(), &["doctor", "--summary", "--no-color"]));
|
||||
assert!(doctor.contains(&number), "doctor reports a different version: {doctor:.120}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -23,6 +23,28 @@ struct Edit {
|
||||
count: usize,
|
||||
}
|
||||
|
||||
impl Edit {
|
||||
/// The edited text, or `None` when this edit is already in the file.
|
||||
///
|
||||
/// An edit that appends leaves its own anchor behind, so "the anchor is
|
||||
/// still there" cannot mean "not yet applied". Finding the *result* is what
|
||||
/// means done, and it is what stops a second run from stacking the edit.
|
||||
fn apply(&self, text: &str) -> Result<Option<String>> {
|
||||
if text.contains(&self.new) {
|
||||
return Ok(None);
|
||||
}
|
||||
let found = text.matches(&self.old).count();
|
||||
if found != self.count {
|
||||
bail!(
|
||||
"{}: expected {} anchors, found {found}. Upstream moved; re-anchor the edit.",
|
||||
self.file,
|
||||
self.count
|
||||
);
|
||||
}
|
||||
Ok(Some(text.replace(&self.old, &self.new)))
|
||||
}
|
||||
}
|
||||
|
||||
pub fn run(arguments: &[String]) -> Result<()> {
|
||||
if !arguments.is_empty() {
|
||||
bail!("usage: upstream prepare");
|
||||
@@ -39,25 +61,75 @@ pub fn run(arguments: &[String]) -> Result<()> {
|
||||
let path = source.join(&edit.file);
|
||||
let text =
|
||||
std::fs::read_to_string(&path).with_context(|| format!("read {}", path.display()))?;
|
||||
// An edit that appends leaves its own anchor in place, so "the anchor is
|
||||
// still there" cannot mean "not yet applied". Finding the result is what
|
||||
// means done — and it is what keeps a second run from stacking the edit.
|
||||
if text.contains(&edit.new) {
|
||||
continue;
|
||||
if let Some(edited) = edit.apply(&text)? {
|
||||
std::fs::write(&path, edited)?;
|
||||
applied += 1;
|
||||
}
|
||||
let found = text.matches(&edit.old).count();
|
||||
if found != edit.count {
|
||||
bail!(
|
||||
"{}: expected {} anchors, found {found}. Upstream moved; re-anchor the edit.",
|
||||
edit.file,
|
||||
edit.count
|
||||
);
|
||||
}
|
||||
std::fs::write(&path, text.replace(&edit.old, &edit.new))?;
|
||||
applied += 1;
|
||||
}
|
||||
|
||||
let revision = crate::git(&source, &["rev-parse", "HEAD"])?;
|
||||
println!("upstream {}: {applied} edits applied", &revision[..12]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::Edit;
|
||||
|
||||
fn edit(old: &str, new: &str, count: usize) -> Edit {
|
||||
Edit { file: "f.rs".into(), old: old.into(), new: new.into(), count }
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_replaces_every_anchor_it_promised() {
|
||||
let e = edit("Codex", "Hanzo Dev", 2);
|
||||
assert_eq!(e.apply("Codex and Codex").unwrap().unwrap(), "Hanzo Dev and Hanzo Dev");
|
||||
}
|
||||
|
||||
/// The bug this file shipped once: an appending edit leaves its own anchor
|
||||
/// behind, so a second run stacked it and produced duplicate lines.
|
||||
#[test]
|
||||
fn an_appending_edit_does_not_stack_on_a_second_run() {
|
||||
let e = edit("[dependencies]", "[dependencies]\nhanzo-tui = { path = \"x\" }", 1);
|
||||
let once = e.apply("[dependencies]\nanyhow = \"1\"").unwrap().unwrap();
|
||||
assert_eq!(once.matches("hanzo-tui").count(), 1);
|
||||
assert!(e.apply(&once).unwrap().is_none(), "already applied");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_already_applied_is_not_applied_again() {
|
||||
let e = edit("Codex", "Hanzo Dev", 1);
|
||||
assert!(e.apply("Hanzo Dev").unwrap().is_none());
|
||||
}
|
||||
|
||||
/// The whole point of the count: upstream moving the ground under an anchor
|
||||
/// must stop preparation, not brand whatever happens to match.
|
||||
#[test]
|
||||
fn a_moved_anchor_stops_preparation() {
|
||||
let e = edit("Codex", "Hanzo Dev", 2);
|
||||
let error = e.apply("Codex once only").unwrap_err().to_string();
|
||||
assert!(error.contains("expected 2 anchors, found 1"), "{error}");
|
||||
assert!(error.contains("re-anchor"), "{error}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_anchor_that_vanished_stops_preparation() {
|
||||
let e = edit("Codex", "Hanzo Dev", 1);
|
||||
assert!(e.apply("nothing to match here").is_err());
|
||||
}
|
||||
|
||||
/// Every edit we ship must be well-formed, or preparation fails at runtime
|
||||
/// in CI rather than here.
|
||||
#[test]
|
||||
fn the_shipped_edits_are_well_formed() {
|
||||
let raw = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../../patches/upstream.json"))
|
||||
.expect("patches/upstream.json is readable");
|
||||
let edits: Vec<Edit> = serde_json::from_slice(&raw).expect("valid JSON");
|
||||
assert!(!edits.is_empty());
|
||||
for e in &edits {
|
||||
assert!(e.count > 0, "{}: an edit that matches nothing is dead", e.file);
|
||||
assert!(!e.old.is_empty(), "{}: empty anchor matches everywhere", e.file);
|
||||
assert_ne!(e.old, e.new, "{}: a no-op edit", e.file);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,5 +112,41 @@
|
||||
"old": "\"Codex's Linux sandbox uses bubblewrap",
|
||||
"new": "\"The Linux sandbox uses bubblewrap",
|
||||
"count": 2
|
||||
},
|
||||
{
|
||||
"file": "utils/cli/src/config_override.rs",
|
||||
"old": "`~/.codex/config.toml`",
|
||||
"new": "`$DEV_HOME/config.toml`",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "exec/src/cli.rs",
|
||||
"old": "not recognized by this version of Codex.",
|
||||
"new": "this version of Dev does not recognize.",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "exec/src/cli.rs",
|
||||
"old": "Allow running Codex outside a Git repository.",
|
||||
"new": "Allow running Dev outside a Git repository.",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "exec/src/cli.rs",
|
||||
"old": "override_usage = \"codex exec [OPTIONS] [PROMPT]\\n codex exec [OPTIONS] <COMMAND> [ARGS]\"",
|
||||
"new": "override_usage = \"dev exec [OPTIONS] [PROMPT]\\n dev exec [OPTIONS] <COMMAND> [ARGS]\"",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "cloud-tasks/src/cli.rs",
|
||||
"old": "Codex Cloud task",
|
||||
"new": "Hanzo task",
|
||||
"count": 8
|
||||
},
|
||||
{
|
||||
"file": "cloud-tasks/src/cli.rs",
|
||||
"old": "Codex Cloud",
|
||||
"new": "Hanzo Tasks",
|
||||
"count": 2
|
||||
}
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user