feat(actions-catalog): add context-safe truncation for list/describe output - #182
Open
mguerrero3-godaddy wants to merge 4 commits into
Open
feat(actions-catalog): add context-safe truncation for list/describe output#182mguerrero3-godaddy wants to merge 4 commits into
mguerrero3-godaddy wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a shared, context-safe truncation utility to prevent agent-facing CLI commands from emitting oversized JSON, and wires it into platform actions list/describe output.
Changes:
- Introduces
rust/src/truncation.rswith helpers to cap list sizes, truncate long strings, and optionally dump full output to$TMPDIR. - Updates
rust/src/actions_catalog/mod.rsto apply list/payload truncation and surface truncation metadata in JSON output, plus adds integration tests. - Registers the new
truncationmodule inrust/src/main.rs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| rust/src/truncation.rs | New truncation utilities (list cap + payload protection) and unit tests; writes full output to temp files. |
| rust/src/actions_catalog/mod.rs | Uses truncation utilities for actions list and actions describe, and adds CLI-level tests for JSON output. |
| rust/src/main.rs | Adds the new truncation module to the crate. |
Suppressed comments (1)
rust/src/truncation.rs:223
- This test currently asserts the truncated string length is
MAX_STRING_LENGTH + suffix, which matches the current behavior but means truncation exceeds the configured cap. If truncation is fixed to keep the final string withinMAX_STRING_LENGTH, update the assertion accordingly.
assert!(nested.ends_with("...(truncated)"));
assert_eq!(nested.len(), MAX_STRING_LENGTH + "...(truncated)".len());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+84
to
+87
| Value::String(s) if s.chars().count() > MAX_STRING_LENGTH => { | ||
| let truncated: String = s.chars().take(MAX_STRING_LENGTH).collect(); | ||
| Value::String(format!("{truncated}...(truncated)")) | ||
| } |
Comment on lines
+65
to
+71
| let millis = SystemTime::now() | ||
| .duration_since(UNIX_EPOCH) | ||
| .ok()? | ||
| .as_millis(); | ||
| let path = dir.join(format!("{millis}-{}.json", slugify(command_id))); | ||
| let contents = serde_json::to_string_pretty(payload).ok()?; | ||
| std::fs::write(&path, contents).ok()?; |
Comment on lines
+18
to
+23
| pub(crate) struct TruncationMetadata { | ||
| pub(crate) truncated: bool, | ||
| pub(crate) total: usize, | ||
| pub(crate) shown: usize, | ||
| pub(crate) full_output: Option<String>, | ||
| } |
Comment on lines
+48
to
+50
| fn estimate_bytes(value: &Value) -> usize { | ||
| serde_json::to_string(value).map(|s| s.len()).unwrap_or(0) | ||
| } |
Comment on lines
+142
to
+158
| let result = protect_payload(schema, &format!("actions-describe-{name}")); | ||
| let mut payload = result.value; | ||
| if let Value::Object(ref mut map) = payload { | ||
| map.insert("truncated".to_string(), json!(result.metadata.is_some())); | ||
| if let Some(metadata) = result.metadata { | ||
| map.insert("total".to_string(), json!(metadata.total)); | ||
| map.insert("shown".to_string(), json!(metadata.shown)); | ||
|
|
||
| // Truncation should always produce a full_output file, but the write | ||
| // is best-effort and can fail (e.g. disk full, permission denied), so | ||
| // this stays an `if let` rather than an unconditional insert. | ||
| if let Some(full_output) = metadata.full_output { | ||
| map.insert("full_output".to_string(), json!(full_output)); | ||
| } | ||
| } | ||
| } | ||
| Ok(CommandResult::new(payload)) |
Comment on lines
+211
to
+213
| assert_eq!(mode & 0o777, 0o600); | ||
| } | ||
| } |
| assert_eq!(output.exit_code, 0, "{}", output.rendered); | ||
| let rendered: serde_json::Value = | ||
| serde_json::from_str(&output.rendered).expect("stdout should contain json"); | ||
| assert_eq!(rendered["data"]["truncated"], serde_json::json!(false)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Truncation is wired into exactly two calls, both in
rust/src/actions_catalog/mod.rs:gddy platform actions list -> the full ACTIONS catalog (currently only 7 entries, so truncated will always be false today) goes through truncate_list, capping at 50 items and dumping the full list to $TMPDIR/godaddy-cli/ if it ever grows past that.
gddy platform actions describe {{action}} -> the loaded actions JSON schema goes through
protect_payload, which truncates any string field over 1000 chars and falls back to a {truncated, summary} format if the serialized schema is above 16KB.NOTE This implements previous approach of using a cahe directory to write the
full_output, and is not in sync with the api explorer proposal, but It closes the gap while that's implemented and synced with this.