diff --git a/docs/LLM_GUIDE.md b/docs/LLM_GUIDE.md index 95eba42..e0ca22a 100644 --- a/docs/LLM_GUIDE.md +++ b/docs/LLM_GUIDE.md @@ -256,11 +256,28 @@ In agent mode, command output is wrapped in a metadata envelope: "count": 42, "truncated": false, "command": "monitors list", - "warnings": [] + "warnings": [], + "note": "This envelope (status/data/metadata) only appears in agent mode. If you are writing a script the user will run outside this agent session, append --no-agent so the output format matches what they will see." } } ``` +### Authoring scripts the user will run + +**The envelope only exists in agent mode.** If you write a script, alias, or runbook that the user (or CI) will run outside this agent session, those callers will not have an agent env var set — so pup will emit the raw payload, not the `{status, data, metadata}` wrapper. A script of yours that does `pup ... | jq '.data[]'` will break when the user runs it. + +Append `--no-agent` whenever you produce pup commands the user will execute later: + +```bash +# Interactive (agent mode auto-detected — envelope wrapped): +pup monitors list --tag='env:prod' + +# In a script you're handing to the user (raw output, parity with their shell): +pup --no-agent monitors list --tag='env:prod' | jq '.[].name' +``` + +This is also surfaced in the agent schema under the `script_authoring` key (run `pup agent schema | jq '.script_authoring'`) and called out in `anti_patterns`. + Error responses in agent mode: ```json diff --git a/src/formatter.rs b/src/formatter.rs index da3eed6..296b321 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -16,14 +16,15 @@ pub struct Metadata { pub next_action: Option, } -/// Agent mode wrapper: { status, data, metadata } -#[derive(Serialize)] -struct AgentEnvelope<'a, T: Serialize> { - status: &'static str, - data: &'a T, - #[serde(skip_serializing_if = "Option::is_none")] - metadata: Option<&'a Metadata>, -} +/// Note injected into `metadata.note` of every agent-mode JSON envelope so +/// an LLM authoring a script for the user to run later is reminded that +/// this envelope only appears in agent mode — without `--no-agent` the +/// user will get raw JSON and any script depending on `.data` / `.status` +/// will silently break. +pub const AGENT_ENVELOPE_NOTE: &str = "This envelope (status/data/metadata) \ + only appears in agent mode. If you are writing a script the user will \ + run outside this agent session, append --no-agent so the output format \ + matches what they will see."; /// Recursively sort all JSON object keys alphabetically. fn sort_json_value(v: serde_json::Value) -> serde_json::Value { @@ -51,6 +52,41 @@ fn go_html_escape(json: &str) -> String { .replace('>', "\\u003e") } +/// Build the agent-mode envelope as a JSON value. Always sets `status`, +/// `data`, and `metadata` — `metadata.note` is always present so an LLM +/// authoring a script for the user is reminded to pass `--no-agent`. +/// Extracted from `format_and_print` for unit-testability. +pub fn build_agent_envelope( + data: &T, + meta: Option<&Metadata>, +) -> Result { + let sorted_data = sort_json_value(serde_json::to_value(data)?); + // Hoist: when the API wraps its list/object in a nested "data" key, + // use that inner value directly so agents see .data[*] instead of .data.data[*]. + let effective_data = match &sorted_data { + serde_json::Value::Object(obj) if obj.contains_key("data") => obj["data"].clone(), + _ => sorted_data.clone(), + }; + let mut metadata_value = match meta { + Some(m) => serde_json::to_value(m)?, + None => serde_json::Value::Object(serde_json::Map::new()), + }; + // `Metadata` is a struct and serializes to an object; an empty map + // is constructed above when `meta` is None. The branch is defensive + // against future changes that might serialize a non-object type. + if let serde_json::Value::Object(ref mut map) = metadata_value { + map.insert( + "note".to_string(), + serde_json::Value::String(AGENT_ENVELOPE_NOTE.to_string()), + ); + } + Ok(serde_json::json!({ + "status": "success", + "data": effective_data, + "metadata": metadata_value, + })) +} + /// Format and print data to stdout. pub fn format_and_print( data: &T, @@ -59,18 +95,7 @@ pub fn format_and_print( meta: Option<&Metadata>, ) -> Result<()> { if agent_mode && *format == OutputFormat::Json { - let sorted_data = sort_json_value(serde_json::to_value(data)?); - // Hoist: when the API wraps its list/object in a nested "data" key, - // use that inner value directly so agents see .data[*] instead of .data.data[*]. - let effective_data = match &sorted_data { - serde_json::Value::Object(obj) if obj.contains_key("data") => obj["data"].clone(), - _ => sorted_data.clone(), - }; - let envelope = AgentEnvelope { - status: "success", - data: &effective_data, - metadata: meta, - }; + let envelope = build_agent_envelope(data, meta)?; let json = go_html_escape(&serde_json::to_string_pretty(&envelope)?); println!("{json}"); return Ok(()); @@ -820,6 +845,60 @@ mod tests { assert!(result.is_ok()); } + #[test] + fn test_agent_envelope_injects_script_authoring_note_with_meta() { + let data = serde_json::json!({"name": "test"}); + let meta = Metadata { + count: Some(1), + truncated: false, + command: Some("monitors list".into()), + next_action: None, + }; + let envelope = build_agent_envelope(&data, Some(&meta)).unwrap(); + assert_eq!(envelope["status"], "success"); + assert_eq!(envelope["metadata"]["count"], 1); + assert_eq!(envelope["metadata"]["command"], "monitors list"); + assert_eq!(envelope["metadata"]["note"], AGENT_ENVELOPE_NOTE); + assert!( + envelope["metadata"]["note"] + .as_str() + .unwrap() + .contains("--no-agent"), + "note must point agents at --no-agent so the gaslighting case is fixed: {envelope}" + ); + } + + #[test] + fn test_agent_envelope_injects_script_authoring_note_without_meta() { + let data = serde_json::json!({"name": "test"}); + let envelope = build_agent_envelope(&data, None).unwrap(); + // Even when callers pass no Metadata, the note must still appear — + // otherwise the "envelope only in agent mode" warning is invisible + // for the many commands that don't construct a Metadata. + assert_eq!(envelope["metadata"]["note"], AGENT_ENVELOPE_NOTE); + assert_eq!(envelope["status"], "success"); + assert!(envelope["metadata"]["count"].is_null()); + assert!( + envelope["metadata"]["note"] + .as_str() + .unwrap() + .contains("--no-agent"), + "note constant itself must mention --no-agent so the rule survives if the constant is rewritten" + ); + } + + #[test] + fn test_agent_envelope_hoists_inner_data_and_keeps_note() { + // When the caller's payload is `{ "data": [...] }`, the envelope + // hoists the inner array so agents see `.data[*]` instead of + // `.data.data[*]`. Verify that the hoist and the metadata.note + // injection don't interfere with each other — both must happen. + let payload = serde_json::json!({"data": [{"id": 1}, {"id": 2}]}); + let envelope = build_agent_envelope(&payload, None).unwrap(); + assert_eq!(envelope["data"], serde_json::json!([{"id": 1}, {"id": 2}])); + assert_eq!(envelope["metadata"]["note"], AGENT_ENVELOPE_NOTE); + } + #[test] fn test_format_and_print_agent_mode_no_meta() { let data = serde_json::json!({"name": "test"}); diff --git a/src/main.rs b/src/main.rs index cf0188e..3bdef15 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9195,6 +9195,26 @@ fn find_subcommand<'a>(cmd: &'a clap::Command, path: &[&str]) -> Option<&'a clap } } +/// Guidance returned in the agent schema for LLMs that author shell scripts +/// or runbooks the user will execute outside the agent session. Agent mode +/// wraps responses in a `{status, data, metadata}` envelope; outside agent +/// mode, output is raw. Without `--no-agent`, a script tested in-session +/// silently breaks when the user runs it. +fn build_script_authoring_guidance() -> serde_json::Value { + serde_json::json!({ + "summary": "Agent mode wraps JSON responses in a {status, data, metadata} envelope. Outside agent mode, pup emits the raw payload. Scripts written without --no-agent will see different shapes depending on who runs them.", + "rule": "When authoring a script, alias, runbook, or any pup command that the user (or CI) will run outside this agent session, append --no-agent so the output format matches what they will see.", + "examples": [ + "# Agent runs interactively (envelope wrapped):", + "pup monitors list --tag='env:prod'", + "", + "# Agent writes a script for the user (raw output, parity with their shell):", + "pup --no-agent monitors list --tag='env:prod' | jq '.[].name'" + ], + "detection": "Agent mode is on when any of: --agent flag, FORCE_AGENT_MODE=1, or an agent env var (CLAUDECODE, CURSOR_AGENT, CODEX, etc.) is set." + }) +} + /// Build a scoped agent schema for a specific subcommand (e.g. `pup logs --help`). fn build_agent_schema_scoped( _root_cmd: &clap::Command, @@ -9322,9 +9342,12 @@ fn build_agent_schema_scoped( "Don't use --from=30d unless you specifically need a month of data; it's slow", "Don't retry failed requests without checking the error; 401 means re-authenticate, 403 means missing permissions", "Don't use 'pup metrics query' without specifying an aggregation (avg, sum, max, min, count)", - "Don't pipe large JSON responses through multiple jq transforms; use query filters at the API level" + "Don't pipe large JSON responses through multiple jq transforms; use query filters at the API level", + "Don't author scripts for the user without --no-agent; the envelope wrapping in agent mode won't appear when they run it (see script_authoring)" ])); + root.insert("script_authoring".into(), build_script_authoring_guidance()); + serde_json::Value::Object(root) } @@ -9392,9 +9415,12 @@ fn build_agent_schema(cmd: &clap::Command) -> serde_json::Value { "Don't use --from=30d unless you specifically need a month of data; it's slow", "Don't retry failed requests without checking the error; 401 means re-authenticate, 403 means missing permissions", "Don't use 'pup metrics query' without specifying an aggregation (avg, sum, max, min, count)", - "Don't pipe large JSON responses through multiple jq transforms; use query filters at the API level" + "Don't pipe large JSON responses through multiple jq transforms; use query filters at the API level", + "Don't author scripts for the user without --no-agent; the envelope wrapping in agent mode won't appear when they run it (see script_authoring)" ])); + root.insert("script_authoring".into(), build_script_authoring_guidance()); + root.insert("best_practices".into(), serde_json::json!([ "Always specify --from to set a time range; most commands default to 1h but be explicit", "Start with narrow time ranges (1h) then widen if needed; large ranges are slow and expensive", @@ -9899,6 +9925,104 @@ mod test_agent_schema { "scoped schema global_flags must include --no-agent" ); } + + /// Assert that a `script_authoring` JSON block has the full contract: + /// summary + rule + examples + detection, with rule mentioning `--no-agent`. + /// Shared between the top-level and scoped schema tests. + fn assert_script_authoring_contract(block: &serde_json::Value) { + assert!( + block.is_object(), + "script_authoring must be an object: {block}" + ); + let summary = block["summary"] + .as_str() + .expect("script_authoring.summary must be a string"); + assert!( + !summary.is_empty(), + "script_authoring.summary must not be empty" + ); + let rule = block["rule"] + .as_str() + .expect("script_authoring.rule must be a string"); + assert!( + rule.contains("--no-agent"), + "script_authoring.rule must mention --no-agent: {rule}" + ); + assert!( + block["examples"].is_array(), + "script_authoring.examples must be an array" + ); + let detection = block["detection"] + .as_str() + .expect("script_authoring.detection must be a string"); + assert!( + !detection.is_empty(), + "script_authoring.detection must not be empty" + ); + } + + /// Top-level schema must surface the script-authoring guidance so that + /// LLMs reading `pup --help` in agent mode know to pass `--no-agent` + /// when writing scripts the user will run later. Without this, an + /// agent's script gets the envelope wrapping the user won't see. + #[test] + fn schema_includes_script_authoring_guidance() { + let schema = get_schema(); + assert_script_authoring_contract(&schema["script_authoring"]); + } + + /// Scoped (per-domain) schema must also include the guidance so an + /// agent that only ran `pup logs --help` still gets the warning. + #[test] + fn scoped_schema_includes_script_authoring_guidance() { + let cmd = Cli::command(); + let logs_cmd = cmd + .get_subcommands() + .find(|s| s.get_name() == "logs") + .expect("logs subcommand not found"); + let schema = build_agent_schema_scoped(&cmd, logs_cmd, &["logs"]); + assert_script_authoring_contract(&schema["script_authoring"]); + } + + /// The anti-patterns array should include a pointer to the new + /// script_authoring section so LLMs that scan anti_patterns first + /// are still led to the full guidance. Match the actual phrasing + /// (`see script_authoring`) so an unrelated future entry that + /// merely contains the word doesn't accidentally satisfy this test. + #[test] + fn schema_anti_patterns_reference_script_authoring() { + let schema = get_schema(); + let anti = schema["anti_patterns"] + .as_array() + .expect("anti_patterns missing"); + assert!( + anti.iter().any(|v| v + .as_str() + .is_some_and(|s| s.contains("see script_authoring"))), + "anti_patterns must reference script_authoring so LLMs find it" + ); + } + + /// Same as above for the scoped schema — agents that only ever call + /// `pup logs --help` should still get pointed at script_authoring. + #[test] + fn scoped_schema_anti_patterns_reference_script_authoring() { + let cmd = Cli::command(); + let logs_cmd = cmd + .get_subcommands() + .find(|s| s.get_name() == "logs") + .expect("logs subcommand not found"); + let schema = build_agent_schema_scoped(&cmd, logs_cmd, &["logs"]); + let anti = schema["anti_patterns"] + .as_array() + .expect("scoped anti_patterns missing"); + assert!( + anti.iter().any(|v| v + .as_str() + .is_some_and(|s| s.contains("see script_authoring"))), + "scoped anti_patterns must reference script_authoring" + ); + } } // ---- Main ----