Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions crates/webcodex-admin/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl FlagParser {
}

pub fn is_admin_group(arg: &str) -> bool {
matches!(arg, "users" | "tokens" | "runner-tokens" | "agent-tokens")
matches!(arg, "users" | "tokens" | "runner-tokens")
}

pub fn usage() -> &'static str {
Expand All @@ -74,6 +74,11 @@ pub fn usage() -> &'static str {
}

pub fn parse_admin_cli(args: &[String]) -> Result<AdminCliCommand, String> {
if args.first().map(String::as_str) == Some("agent-tokens") {
return Err(
"webcodex agent-tokens was removed; use webcodex runner-tokens instead".to_string(),
);
}
if args.len() < 2 {
return Err(format!("missing admin subcommand\n{}", usage()));
}
Expand All @@ -87,12 +92,10 @@ pub fn parse_admin_cli(args: &[String]) -> Result<AdminCliCommand, String> {
("tokens", "register-hash") => parse_tokens_register_hash(rest),
("tokens", "list") => parse_tokens_list(rest),
("tokens", "revoke") => parse_tokens_revoke(rest),
("runner-tokens" | "agent-tokens", "create") => parse_runner_tokens_create(rest),
("runner-tokens" | "agent-tokens", "register-hash") => {
parse_runner_tokens_register_hash(rest)
}
("runner-tokens" | "agent-tokens", "list") => parse_runner_tokens_list(rest),
("runner-tokens" | "agent-tokens", "revoke") => parse_runner_tokens_revoke(rest),
("runner-tokens", "create") => parse_runner_tokens_create(rest),
("runner-tokens", "register-hash") => parse_runner_tokens_register_hash(rest),
("runner-tokens", "list") => parse_runner_tokens_list(rest),
("runner-tokens", "revoke") => parse_runner_tokens_revoke(rest),
_ => Err(format!(
"unknown admin command: {} {}\n{}",
group,
Expand Down
13 changes: 13 additions & 0 deletions crates/webcodex-admin/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ fn admin_usage_keeps_rest_registration_commands_but_not_create_local() {
assert!(!stdout.contains("webcodex agent-token register-hash"));
}

#[test]
fn admin_parser_rejects_retired_agent_tokens_group_with_migration_guidance() {
assert!(!is_admin_group("agent-tokens"));
for args in [
vec!["agent-tokens".to_string()],
vec!["agent-tokens".to_string(), "list".to_string()],
] {
let error = parse_admin_cli(&args).unwrap_err();
assert!(error.contains("agent-tokens was removed"), "{error}");
assert!(error.contains("runner-tokens"), "{error}");
}
}

#[test]
fn users_create_builds_request_path_and_body() {
let req = request(&[
Expand Down
43 changes: 21 additions & 22 deletions crates/webcodex-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,10 @@ where
"agent-token" => cli_parse_error(
"`webcodex agent-token` was removed; use `webcodex runner-tokens ...`".to_string(),
),
"runner-tokens" | "agent-tokens" => parse_runner_token_subcommand(&args[1..]),
"agent-tokens" => cli_parse_error(
"`webcodex agent-tokens` was removed; use `webcodex runner-tokens ...`".to_string(),
),
"runner-tokens" => parse_runner_token_subcommand(&args[1..]),
"token" => cli_parse_error(
"`webcodex token` was removed; use `webcodex tokens ...`".to_string(),
),
Expand Down Expand Up @@ -2265,9 +2268,14 @@ fn parse_runner_status_with_identity(
"--user-token-file" => {
opts.user_token_file = Some(PathBuf::from(next_value(&mut iter, arg)?))
}
"--runner-token-file" | "--agent-token-file" => {
"--runner-token-file" => {
opts.runner_token_file = Some(PathBuf::from(next_value(&mut iter, arg)?))
}
"--agent-token-file" => {
return Err(
"--agent-token-file is retired; use --runner-token-file instead".to_string(),
)
}
"--json" => opts.json = true,
_ => return Err(format!("unknown runner status flag: {}", arg)),
}
Expand Down Expand Up @@ -2470,8 +2478,11 @@ fn parse_pairing_create(args: &[String]) -> Result<PairingCreateOptions, String>
.map_err(|_| "--ttl-secs must be an integer".to_string())?;
}
"--user-token-name" => opts.user_token_name = Some(next_value(&mut iter, arg)?),
"--runner-token-name" | "--agent-token-name" => {
opts.runner_token_name = Some(next_value(&mut iter, arg)?)
"--runner-token-name" => opts.runner_token_name = Some(next_value(&mut iter, arg)?),
"--agent-token-name" => {
return Err(
"--agent-token-name is retired; use --runner-token-name instead".to_string(),
)
}
"--json" => opts.json = true,
_ => return Err(format!("unknown pairing create flag: {}", arg)),
Expand Down Expand Up @@ -2517,7 +2528,6 @@ fn parse_cli_runner_init(args: &[String]) -> Result<RunnerInitOptions, String> {
let mut profile: Option<String> = None;
let mut output_explicit = false;
let mut project_registry_dir_explicit = false;
let mut legacy_projects_dir_explicit = false;
let mut iter = args.iter();
while let Some(arg) = iter.next() {
match arg.as_str() {
Expand All @@ -2536,24 +2546,13 @@ fn parse_cli_runner_init(args: &[String]) -> Result<RunnerInitOptions, String> {
.map_err(|_| "--poll-interval-ms must be an integer".to_string())?;
}
"--project-registry-dir" => {
if legacy_projects_dir_explicit {
return Err(
"use only one of --project-registry-dir or legacy --projects-dir"
.to_string(),
);
}
opts.project_registry_dir = PathBuf::from(next_value(&mut iter, arg)?);
project_registry_dir_explicit = true;
}
"--projects-dir" => {
if project_registry_dir_explicit {
return Err(
"use only one of --project-registry-dir or legacy --projects-dir"
.to_string(),
);
}
opts.project_registry_dir = PathBuf::from(next_value(&mut iter, arg)?);
legacy_projects_dir_explicit = true;
return Err(
"--projects-dir is retired; use --project-registry-dir instead".to_string(),
)
}
"--allowed-root" => opts
.allowed_roots
Expand All @@ -2578,17 +2577,17 @@ fn parse_cli_runner_init(args: &[String]) -> Result<RunnerInitOptions, String> {
if !output_explicit {
opts.output = client_profile_runner_config(&profile)?;
}
if !project_registry_dir_explicit && !legacy_projects_dir_explicit {
if !project_registry_dir_explicit {
opts.project_registry_dir = client_profile_project_registry_dir(&profile)?;
}
} else {
if !output_explicit && opts.output.as_os_str().is_empty() {
let profile = validate_client_profile(&opts.client_id)?;
opts.output = client_profile_runner_config(&profile)?;
if !project_registry_dir_explicit && !legacy_projects_dir_explicit {
if !project_registry_dir_explicit {
opts.project_registry_dir = client_profile_project_registry_dir(&profile)?;
}
} else if !project_registry_dir_explicit && !legacy_projects_dir_explicit {
} else if !project_registry_dir_explicit {
let default = Path::new(DEFAULT_INIT_PROJECT_REGISTRY_DIR);
let base = default.parent().ok_or_else(|| {
"default Runner project registry path has no parent directory".to_string()
Expand Down
13 changes: 8 additions & 5 deletions crates/webcodex-cli/src/webcodex_cli/connect/disconnect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ mod tests {
}

#[tokio::test]
async fn offline_disconnect_accepts_legacy_agent_toml_only() {
async fn offline_disconnect_rejects_retired_agent_toml_only() {
let tmp = canonical_test_tempdir();
let config = tmp.path().join("config");
let state = tmp.path().join("state");
Expand All @@ -733,18 +733,20 @@ mod tests {
)
.unwrap();

let result = run_disconnect(DisconnectOptions {
let error = run_disconnect(DisconnectOptions {
project,
profile: Some("legacy".to_string()),
config_base: Some(config),
state_base: Some(state),
server_http: ServerHttpOptions::default(),
})
.await
.unwrap();
assert_eq!(result.outcome, "local_unregistered");
.unwrap_err();
assert!(error.contains("retired Runner config"), "{error}");
assert!(error.contains("rename it to runner.toml"), "{error}");
assert!(profile_dir.join("agent.toml").is_file());
assert!(!profile_dir.join("runner.toml").exists());
assert!(profile_dir.join("project-registry/repo.toml").is_file());
}

#[tokio::test]
Expand Down Expand Up @@ -774,7 +776,8 @@ mod tests {
.unwrap_err();
assert!(error.contains("runner.toml"));
assert!(error.contains("agent.toml"));
assert!(error.contains("refusing to guess"));
assert!(error.contains("retired"), "{error}");
assert!(error.contains("remove or archive"), "{error}");
assert!(profile_dir.join("project-registry/repo.toml").is_file());
}

Expand Down
51 changes: 26 additions & 25 deletions crates/webcodex-cli/src/webcodex_cli/connect/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,12 @@ pub(super) fn render_runner_document(
canonical_project: &Path,
) -> Result<String, String> {
let mut root = read_runner_document(path)?;
if root.contains_key("projects_dir") {
return Err(
"Runner config field 'projects_dir' is retired; use 'project_registry_dir' instead"
.to_string(),
);
}
root.insert(
"server_url".to_string(),
TomlValue::String(server_url.to_string()),
Expand All @@ -655,10 +661,6 @@ pub(super) fn render_runner_document(
TomlValue::String(client_id.to_string()),
);
root.remove("owner");
// A hosted connect update is also a config-spelling migration. Keeping the
// legacy key while inserting the canonical one would create a Runner config
// that the load-time dual-field fence correctly rejects.
root.remove("projects_dir");
root.insert(
"project_registry_dir".to_string(),
TomlValue::String(project_registry_dir.to_string_lossy().to_string()),
Expand Down Expand Up @@ -795,7 +797,7 @@ mod tests {
}

#[test]
fn omitted_key_is_generated_once_then_recovered_from_matching_legacy_registry_profile() {
fn omitted_key_is_generated_once_then_recovered_from_matching_legacy_registry_layout() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path().join("project");
std::fs::create_dir(&project).unwrap();
Expand Down Expand Up @@ -827,12 +829,11 @@ mod tests {
assert!(first.generated);
let profile = derived_profile("https://example.test", &first.value);
let profile_dir = config_base.join("clients").join(&profile);
// Deliberately exercise a pre-normalization hosted profile. New
// profiles use project-registry/, but a sole projects.d/ remains a
// supported compatibility layout.
// A sole physical projects.d/ registry remains supported even though
// the agent.toml filename and projects_dir config field are retired.
std::fs::create_dir_all(profile_dir.join("projects.d")).unwrap();
std::fs::write(
profile_dir.join("agent.toml"),
profile_dir.join("runner.toml"),
format!(
"server_url = \"https://example.test\"\ntoken = {:?}\nclient_id = \"client\"\n",
first.value
Expand All @@ -852,18 +853,20 @@ mod tests {
recovered.recovered_profile.as_deref(),
Some(profile.as_str())
);
assert!(!profile_dir.join("runner.toml").exists());
assert!(profile_dir.join("runner.toml").is_file());
std::fs::write(profile_dir.join(KEY_DISCLOSED_FILE), "disclosed = true\n").unwrap();
let disclosed =
resolve_key(&options, &config_base, "https://example.test", &project).unwrap();
assert!(!disclosed.generated);

std::fs::write(profile_dir.join("runner.toml"), "conflicting = true\n").unwrap();
std::fs::write(profile_dir.join("agent.toml"), "retired = true\n").unwrap();
let error =
resolve_key(&options, &config_base, "https://example.test", &project).unwrap_err();
assert!(error.contains("runner.toml"));
assert!(error.contains("agent.toml"));
assert!(error.contains("refusing to guess"));
assert!(
error.contains("both runner.toml and retired agent.toml"),
"{error}"
);
assert!(error.contains("remove or archive agent.toml"), "{error}");
}

#[cfg(unix)]
Expand Down Expand Up @@ -958,9 +961,9 @@ mod tests {
}

#[test]
fn runner_document_migrates_legacy_projects_dir_without_dual_fields() {
fn runner_document_rejects_retired_projects_dir_with_migration_guidance() {
let tmp = tempfile::tempdir().unwrap();
let config = tmp.path().join("agent.toml");
let config = tmp.path().join("runner.toml");
let project = tmp.path().join("project");
let legacy_registry = tmp.path().join("projects.d");
std::fs::create_dir(&project).unwrap();
Expand All @@ -974,22 +977,20 @@ mod tests {
)
.unwrap();

let rendered = render_runner_document(
let error = render_runner_document(
&config,
"https://example.test",
"shared",
"client",
&legacy_registry,
&project.canonicalize().unwrap(),
)
.unwrap();
let parsed: TomlValue = toml::from_str(&rendered).unwrap();
assert!(parsed.get("projects_dir").is_none());
assert_eq!(
parsed["project_registry_dir"].as_str(),
Some(legacy_registry.to_string_lossy().as_ref())
);
assert_eq!(parsed["custom_field"].as_str(), Some("preserved"));
.unwrap_err();
assert!(error.contains("'projects_dir' is retired"), "{error}");
assert!(error.contains("'project_registry_dir'"), "{error}");
let unchanged = std::fs::read_to_string(&config).unwrap();
assert!(unchanged.contains("projects_dir"));
assert!(unchanged.contains("custom_field"));
}

#[cfg(unix)]
Expand Down
37 changes: 19 additions & 18 deletions crates/webcodex-cli/src/webcodex_cli/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ struct RegistrationRunnerConfig {
#[serde(default)]
project_registry_dir: Option<PathBuf>,
#[serde(default, rename = "projects_dir")]
legacy_projects_dir: Option<PathBuf>,
removed_projects_dir: Option<toml::Value>,
#[serde(default)]
policy: RegistrationPolicy,
}
Expand Down Expand Up @@ -348,16 +348,15 @@ fn persist_registration_allowed_roots(path: &Path, roots: &[PathBuf]) -> Result<
}

fn registration_project_registry_dir(config: &RegistrationRunnerConfig) -> Result<PathBuf, String> {
match (
config.project_registry_dir.as_ref(),
config.legacy_projects_dir.as_ref(),
) {
(Some(_), Some(_)) => Err(
"project_registry_dir and legacy projects_dir cannot both be configured; keep exactly one Runner project registry setting"
if config.removed_projects_dir.is_some() {
return Err(
"Runner config field 'projects_dir' is retired; use 'project_registry_dir' instead"
.to_string(),
),
(Some(path), None) | (None, Some(path)) => Ok(path.clone()),
(None, None) => {
);
}
match config.project_registry_dir.as_ref() {
Some(path) => Ok(path.clone()),
None => {
let base = webcodex_runner_config::paths::default_client_config_base_dir()?;
webcodex_runner_config::paths::select_project_registry_dir(&base)
}
Expand Down Expand Up @@ -1671,7 +1670,7 @@ mod tests {
let _guard = crate::webcodex_cli::test_support::env_test_guard();
let config = RegistrationRunnerConfig {
project_registry_dir: None,
legacy_projects_dir: None,
removed_projects_dir: None,
policy: RegistrationPolicy::default(),
};
let base = webcodex_runner_config::paths::default_client_config_base_dir().unwrap();
Expand All @@ -1683,25 +1682,27 @@ mod tests {
}

#[test]
fn registration_config_accepts_legacy_projects_dir_alias() {
let legacy = PathBuf::from("/tmp/legacy-projects.d");
fn registration_config_rejects_retired_projects_dir() {
let config = RegistrationRunnerConfig {
project_registry_dir: None,
legacy_projects_dir: Some(legacy.clone()),
removed_projects_dir: Some(toml::Value::String("/tmp/projects.d".to_string())),
policy: RegistrationPolicy::default(),
};
assert_eq!(registration_project_registry_dir(&config).unwrap(), legacy);
let error = registration_project_registry_dir(&config).unwrap_err();
assert!(error.contains("'projects_dir' is retired"), "{error}");
assert!(error.contains("'project_registry_dir'"), "{error}");
}

#[test]
fn registration_config_rejects_both_registry_fields() {
fn registration_config_rejects_retired_projects_dir_with_canonical_field() {
let config = RegistrationRunnerConfig {
project_registry_dir: Some(PathBuf::from("/tmp/project-registry")),
legacy_projects_dir: Some(PathBuf::from("/tmp/projects.d")),
removed_projects_dir: Some(toml::Value::String("/tmp/projects.d".to_string())),
policy: RegistrationPolicy::default(),
};
let error = registration_project_registry_dir(&config).unwrap_err();
assert!(error.contains("cannot both be configured"), "{error}");
assert!(error.contains("'projects_dir' is retired"), "{error}");
assert!(error.contains("'project_registry_dir'"), "{error}");
}

#[test]
Expand Down
Loading
Loading