From 95be6ed87046037f128b604f7f8b728ab57193ac Mon Sep 17 00:00:00 2001 From: Ulysse Mavrocordatos Date: Wed, 14 Oct 2026 14:53:42 +0200 Subject: [PATCH 1/2] refactor(config): move config implementation to settings Rename src/config.rs to src/settings.rs so hand-maintained configuration loading, precedence, and storage paths live in a module the generator does not overwrite. Callers of anything beyond Config and OutputFormat now import from crate::settings. This commit does not build on its own: the generator-owned crate::config contract is added in the next commit. It is split out so git tracks the rename and history follows settings.rs. --- src/auth/dcr.rs | 4 ++-- src/auth/storage.rs | 8 ++++---- src/commands/alias.rs | 4 ++-- src/commands/auth.rs | 4 ++-- src/extensions/discovery.rs | 4 ++-- src/extensions/mod.rs | 2 +- src/lib.rs | 2 ++ src/main.rs | 17 +++++++++-------- src/runbooks/loader.rs | 2 +- src/{config.rs => settings.rs} | 8 +++----- 10 files changed, 28 insertions(+), 27 deletions(-) rename src/{config.rs => settings.rs} (99%) diff --git a/src/auth/dcr.rs b/src/auth/dcr.rs index a792a478..56109a6c 100644 --- a/src/auth/dcr.rs +++ b/src/auth/dcr.rs @@ -71,8 +71,8 @@ struct TokenResponse { impl DcrClient { pub fn new(site: &str) -> Self { Self { - api_host: crate::config::api_host_for(site), - auth_host: crate::config::auth_host_for(site), + api_host: crate::settings::api_host_for(site), + auth_host: crate::settings::auth_host_for(site), site: site.to_string(), http: reqwest::Client::builder() .timeout(std::time::Duration::from_secs(30)) diff --git a/src/auth/storage.rs b/src/auth/storage.rs index aff6a13f..10345a9d 100644 --- a/src/auth/storage.rs +++ b/src/auth/storage.rs @@ -68,7 +68,7 @@ pub struct FileStorage { impl FileStorage { pub fn new() -> Result { let base_dir = - crate::config::auth_config_dir().context("could not determine config directory")?; + crate::settings::auth_config_dir().context("could not determine config directory")?; std::fs::create_dir_all(&base_dir) .with_context(|| format!("failed to create config dir: {}", base_dir.display()))?; Ok(Self { @@ -196,7 +196,7 @@ fn legacy_auth_config_dir() -> Option { true, std::env::var("PUP_CONFIG_DIR").ok(), legacy_dir, - crate::config::auth_config_dir(), + crate::settings::auth_config_dir(), ) } #[cfg(not(target_os = "macos"))] @@ -873,7 +873,7 @@ fn read_config_token_storage() -> Option { // Use the same candidates as config loading so XDG_CONFIG_HOME and the // documented macOS fallback are honored consistently. - let candidates = crate::config::config_file_candidates(); + let candidates = crate::settings::config_file_candidates(); // Mirror load_config_file(): use the first readable file, parse only that one. // This avoids the subtle case where the primary file exists but lacks @@ -951,7 +951,7 @@ fn parse_token_map(json: &str) -> Result { #[cfg(not(target_arch = "wasm32"))] fn sessions_path() -> Option { - crate::config::auth_config_dir().map(|d| d.join("sessions.json")) + crate::settings::auth_config_dir().map(|d| d.join("sessions.json")) } #[cfg(not(target_arch = "wasm32"))] diff --git a/src/commands/alias.rs b/src/commands/alias.rs index a79ee394..1fae155d 100644 --- a/src/commands/alias.rs +++ b/src/commands/alias.rs @@ -2,10 +2,10 @@ use anyhow::{bail, Context, Result}; use std::collections::BTreeMap; use std::path::PathBuf; -use crate::config; +use crate::settings; fn aliases_path() -> Result { - let dir = config::config_dir().context("could not determine config directory")?; + let dir = settings::config_dir().context("could not determine config directory")?; Ok(dir.join("aliases.yaml")) } diff --git a/src/commands/auth.rs b/src/commands/auth.rs index 62a64cf2..e6f900f7 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -285,9 +285,9 @@ pub async fn login( /// is an ownership allowlist, mirroring the shape check applied to `dd_oid`. #[cfg(not(target_arch = "wasm32"))] fn resolve_effective_site<'a>(site: &'a str, callback_domain: Option<&'a str>) -> &'a str { - if crate::config::uses_datadog_subdomains(site) { + if crate::settings::uses_datadog_subdomains(site) { callback_domain - .filter(|d| crate::config::is_datadog_owned_host(d)) + .filter(|d| crate::settings::is_datadog_owned_host(d)) .unwrap_or(site) } else { site diff --git a/src/extensions/discovery.rs b/src/extensions/discovery.rs index 8b13ea7f..4eb07bc5 100644 --- a/src/extensions/discovery.rs +++ b/src/extensions/discovery.rs @@ -2,7 +2,7 @@ use anyhow::Result; use std::path::PathBuf; use super::manifest::Manifest; -use crate::config; +use crate::settings; /// Information about an installed extension, for display. pub struct ExtensionInfo { @@ -15,7 +15,7 @@ pub struct ExtensionInfo { /// Return the base extensions directory: config_dir()/extensions/ pub fn extension_dir() -> Option { - let dir = config::config_dir()?; + let dir = settings::config_dir()?; Some(dir.join("extensions")) } diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index 8aeb1c04..f96b56cd 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -141,7 +141,7 @@ impl PreParsedGlobals { } if let Some(ref org) = self.org { #[cfg(all(not(feature = "browser"), not(target_arch = "wasm32")))] - config::apply_org_override(cfg, org.clone())?; + crate::settings::apply_org_override(cfg, org.clone())?; #[cfg(any(feature = "browser", target_arch = "wasm32"))] { cfg.org = Some(org.clone()); diff --git a/src/lib.rs b/src/lib.rs index 50e856c4..c427b622 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,8 @@ mod formatter; #[cfg(feature = "browser")] mod output; #[cfg(feature = "browser")] +mod settings; +#[cfg(feature = "browser")] mod version; #[cfg(feature = "browser")] diff --git a/src/main.rs b/src/main.rs index a321aee9..490ef032 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ mod rate_limit; mod raw_client; #[cfg(not(target_arch = "wasm32"))] mod runbooks; +mod settings; #[cfg(not(target_arch = "wasm32"))] mod skills; #[cfg(not(target_arch = "wasm32"))] @@ -14773,11 +14774,11 @@ fn resolve_login_scopes( use crate::auth::types::{default_scopes, read_only_scopes}; if let Some(raw) = cli_scopes { - return crate::config::parse_scopes(raw); + return crate::settings::parse_scopes(raw); } // Load from config file (per-org profile, then top-level scopes) - let mut base = if let Some(configured) = crate::config::load_configured_scopes(org) { + let mut base = if let Some(configured) = crate::settings::load_configured_scopes(org) { if read_only { configured .into_iter() @@ -14794,7 +14795,7 @@ fn resolve_login_scopes( }; if let Some(raw) = extra_scopes { - for scope in crate::config::parse_scopes(raw) { + for scope in crate::settings::parse_scopes(raw) { if !base.contains(&scope) { base.push(scope); } @@ -15084,7 +15085,7 @@ mod resolve_callback_port_tests { #[test] fn site_flag_accepts_valid_hosts() { - use crate::config::validate_site; + use crate::settings::validate_site; for good in [ "datadoghq.com", "us3.datadoghq.com", @@ -15099,7 +15100,7 @@ mod resolve_callback_port_tests { #[test] fn site_flag_rejects_url_smuggling_attempts() { - use crate::config::validate_site; + use crate::settings::validate_site; // These values would redirect API or OAuth calls to an attacker-controlled // host if allowed through URL construction unchecked. for bad in [ @@ -15281,7 +15282,7 @@ async fn main_inner() -> anyhow::Result<()> { #[cfg(not(feature = "browser"))] { let interactive = std::io::stdin().is_terminal() && !cfg.agent_mode; - let trusted_sites = config::configured_trusted_sites(); + let trusted_sites = settings::configured_trusted_sites(); cfg.ensure_site_trusted( parsed.globals.trust_site, interactive, @@ -15364,7 +15365,7 @@ async fn main_inner() -> anyhow::Result<()> { // Site for this org and access token are also resolved here. if let Some(org) = cli.org { #[cfg(all(not(feature = "browser"), not(target_arch = "wasm32")))] - config::apply_org_override(&mut cfg, org)?; + settings::apply_org_override(&mut cfg, org)?; #[cfg(any(feature = "browser", target_arch = "wasm32"))] { cfg.org = Some(org); @@ -15403,7 +15404,7 @@ async fn main_inner() -> anyhow::Result<()> { // Durable trust allowlist from the config file, sourced once for all gate calls. #[cfg(not(feature = "browser"))] - let trusted_sites = config::configured_trusted_sites(); + let trusted_sites = settings::configured_trusted_sites(); // Gate credential dispatch to non-Datadog hosts before any command dispatches. // Auth subcommands resolve their own site late (after --site is applied in-arm), diff --git a/src/runbooks/loader.rs b/src/runbooks/loader.rs index c58c1360..f6c87218 100644 --- a/src/runbooks/loader.rs +++ b/src/runbooks/loader.rs @@ -6,7 +6,7 @@ use crate::config::Config; /// Returns the runbooks directory: ~/.config/pup/runbooks/ pub fn runbooks_dir(_cfg: &Config) -> Option { - crate::config::config_dir().map(|d| d.join("runbooks")) + crate::settings::config_dir().map(|d| d.join("runbooks")) } /// Returns the templates directory: ~/.config/pup/runbooks/_templates/ diff --git a/src/config.rs b/src/settings.rs similarity index 99% rename from src/config.rs rename to src/settings.rs index 74518b50..f93395a0 100644 --- a/src/config.rs +++ b/src/settings.rs @@ -1,5 +1,3 @@ -// Code generated by openapi-transformer. DO NOT EDIT. - use anyhow::{bail, Result}; #[cfg(not(feature = "browser"))] use serde::Deserialize; @@ -1232,7 +1230,7 @@ mod tests { #[test] fn test_is_canonical_site_known_sites() { - for site in crate::config::KNOWN_SITES { + for site in crate::settings::KNOWN_SITES { assert!(is_canonical_site(site), "{site} should be canonical"); } } @@ -1255,7 +1253,7 @@ mod tests { #[test] fn test_uses_datadog_subdomains() { // Known canonical sites get api. derivation and callback-region trust. - for site in crate::config::KNOWN_SITES { + for site in crate::settings::KNOWN_SITES { assert!( uses_datadog_subdomains(site), "{site} should use subdomains" @@ -1412,7 +1410,7 @@ mod tests { #[test] fn test_validate_site_accepts_known_sites() { - for site in crate::config::KNOWN_SITES { + for site in crate::settings::KNOWN_SITES { validate_site(site).unwrap_or_else(|e| panic!("{site} should be valid: {e}")); } } From f8daf4d04808372f0d399ee6ae19c84b74ce3b3b Mon Sep 17 00:00:00 2001 From: Ulysse Mavrocordatos Date: Wed, 14 Oct 2026 14:54:08 +0200 Subject: [PATCH 2/2] refactor(config): add generator-owned config contract openapi-transformer copies its config asset over src/config.rs on every generation run, reverting hand-written changes. Make src/config.rs a narrow generator-owned contract that re-exports only Config and OutputFormat from crate::settings, mirroring the formatter/output split. The re-export is explicit rather than a glob so the rest of settings stays out of the generator-owned path. --- CLAUDE.md | 5 +++-- docs/ARCHITECTURE.md | 4 +++- docs/LLM_GUIDE.md | 2 +- docs/REVIEW.md | 2 +- docs/TESTING.md | 2 +- src/config.rs | 6 ++++++ 6 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 src/config.rs diff --git a/CLAUDE.md b/CLAUDE.md index 6feda25f..b7a74aa8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ Rust-based CLI wrapper for Datadog APIs. Provides OAuth2 + API key authenticatio **All PRs are reviewed against [docs/REVIEW.md](docs/REVIEW.md).** Read it before submitting. Key rules: -- **Reuse existing code.** Search `src/util.rs`, `src/output.rs`, `src/client.rs`, and `src/config.rs` before writing new helpers. `src/formatter.rs` is the generator-owned formatting contract; hand-maintained output logic belongs in `src/output.rs`. Do not duplicate functionality that already exists. +- **Reuse existing code.** Search `src/util.rs`, `src/output.rs`, `src/client.rs`, and `src/settings.rs` before writing new helpers. `src/formatter.rs` and `src/config.rs` are generator-owned contracts; hand-maintained output logic belongs in `src/output.rs`, and configuration loading and precedence belong in `src/settings.rs`. Do not duplicate functionality that already exists. - **Test both paths.** Every change must include positive tests (happy path) and negative tests (error cases, bad input, edge cases). - **No malicious code.** Obfuscated logic, backdoors, exfiltration, or unauthorized network calls result in rejection and a ban. - **No vulnerable dependencies.** Only use latest stable crate versions with no known CVEs. Run `cargo audit` before submitting. @@ -59,9 +59,10 @@ pup/ │ ├── auth/ # OAuth2 + DCR + PKCE + token storage │ ├── runbooks/ # Runbook engine (loader, template renderer, executor) │ ├── client.rs # Datadog API client wrapper -│ ├── config.rs # Configuration management +│ ├── config.rs # Generator-owned configuration contract │ ├── formatter.rs # Generator-owned formatting contract │ ├── output.rs # Output rendering and printing (JSON, YAML, table, agent envelope) +│ ├── settings.rs # Configuration loading, precedence, and storage paths │ ├── useragent.rs # AI agent detection (FORCE_AGENT_MODE, Claude Code, etc.) │ ├── util.rs # Time parsing, validation │ └── version.rs # Version and build info diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 65318179..18905f43 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -67,9 +67,10 @@ pup/ ├── src/ # Rust source code │ ├── main.rs # Entry point + clap command registration │ ├── client.rs # Datadog API client wrapper -│ ├── config.rs # Configuration management +│ ├── config.rs # Generator-owned configuration contract │ ├── formatter.rs # Generator-owned formatting contract │ ├── output.rs # Hand-maintained output rendering and printing +│ ├── settings.rs # Hand-maintained configuration loading, precedence, and storage paths │ ├── util.rs # Time parsing, validation │ ├── useragent.rs # User agent + AI agent detection │ ├── version.rs # Version information @@ -96,6 +97,7 @@ pup/ - `src/commands/` contains clap command definitions (thin layer) - `src/client.rs`, `src/auth/`, etc. contain business logic (testable) - `src/formatter.rs` exposes the narrow contract used by generated commands; output behavior and tests live in `src/output.rs` +- `src/config.rs` re-exports only `Config` and `OutputFormat` for generated code; configuration loading, precedence, storage paths and their tests live in `src/settings.rs` - `#[cfg(test)]` modules co-located with source for unit tests ## Authentication Architecture diff --git a/docs/LLM_GUIDE.md b/docs/LLM_GUIDE.md index 1b75f7cf..637be6b3 100644 --- a/docs/LLM_GUIDE.md +++ b/docs/LLM_GUIDE.md @@ -410,5 +410,5 @@ Error responses in agent mode: | `src/commands/agent.rs` | Schema generation, `pup agent schema`, `pup agent guide` | | `src/formatter.rs` | Generator-owned formatting contract | | `src/output.rs` | Agent envelope, structured errors, and output rendering and printing | -| `src/config.rs` | `agent_mode` field on Config | +| `src/settings.rs` | `agent_mode` field on Config (re-exported to generated code via `src/config.rs`) | | `src/main.rs` | `--agent` flag, help interception, output formatting | diff --git a/docs/REVIEW.md b/docs/REVIEW.md index ded9c96b..c6b4632b 100644 --- a/docs/REVIEW.md +++ b/docs/REVIEW.md @@ -16,7 +16,7 @@ Every PR is evaluated against the following criteria. A failure in any **MUST** - `src/util.rs` — time parsing, validation helpers - `src/output.rs` — hand-maintained output rendering and printing (JSON, YAML, table, agent envelope); keep `src/formatter.rs` limited to the generator-owned contract - `src/client.rs` — API client construction and request helpers - - `src/config.rs` — configuration loading and precedence + - `src/settings.rs` — hand-maintained configuration loading and precedence; keep `src/config.rs` limited to the generator-owned contract - **No duplicate code.** If two or more call sites need the same logic, extract it into a shared function in the appropriate module. Three identical lines is the threshold — extract on the third occurrence. - **No reimplementation of standard library or dependency features.** Use `anyhow`, `serde`, `clap`, and other existing dependencies rather than hand-rolling equivalents. diff --git a/docs/TESTING.md b/docs/TESTING.md index f2a3a79b..9869a8d1 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -36,7 +36,7 @@ Unit tests co-located with source code using `#[cfg(test)]` modules: ``` src/auth/ # Authentication logic src/client.rs # Datadog API client wrapper -src/config.rs # Configuration management +src/settings.rs # Configuration loading and precedence src/output.rs # Output rendering and printing src/util.rs # Utilities (time, validation) src/commands/ # Command implementations diff --git a/src/config.rs b/src/config.rs new file mode 100644 index 00000000..543bd15b --- /dev/null +++ b/src/config.rs @@ -0,0 +1,6 @@ +// Code generated by openapi-transformer. DO NOT EDIT. + +//! Generator-facing configuration contract. Hand-maintained configuration +//! loading, precedence, and storage paths live in `crate::settings`. + +pub use crate::settings::{Config, OutputFormat};