commit fb2711c02bec2c399a5a533d19568b759ec68332
parent 11044372b4955bf3f63457700fc4f310d87e925a
Author: triesap <tyson@radroots.org>
Date: Sat, 27 Jun 2026 07:11:25 +0000
cli: structure terminal registry invariant failures
- replace duplicate renderer panics with typed registry violations
- validate terminal renderer coverage against the operation registry
- render registry drift as a structured internal terminal failure
- remove duplicate terminal registry operation count assertion
Diffstat:
| M | src/main.rs | | | 137 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------- |
| M | src/out/terminal/registry.rs | | | 190 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- |
2 files changed, 295 insertions(+), 32 deletions(-)
diff --git a/src/main.rs b/src/main.rs
@@ -13,6 +13,7 @@ use std::sync::atomic::{AtomicU64, Ordering};
use std::time::{SystemTime, UNIX_EPOCH};
use clap::Parser;
+use serde_json::json;
use crate::cli::input::runtime_invocation_args_from_target;
use crate::cli::{TargetCliArgs, TargetOutputFormat};
@@ -26,12 +27,16 @@ use crate::ops::{
OperationRequest, OperationRequestPayload, OperationResultPayload, OperationService,
TargetOperationRequest,
};
-use crate::out::envelope::{CliExitCode, OutputEnvelope, OutputError};
+use crate::out::envelope::{CliExitCode, EnvelopeContext, OutputEnvelope, OutputError};
+use crate::out::terminal::errors::terminal_error_document;
+use crate::out::terminal::registry::TerminalRendererRegistryError;
use crate::out::terminal::registry::terminal_renderer_registry;
use crate::out::terminal::renderer::{
TerminalRenderContext, TerminalVerbosity, render_terminal_document,
};
-use crate::registry::{NetworkRequirement, network_requirement, requires_local_signer_mode};
+use crate::registry::{
+ NetworkRequirement, OPERATION_REGISTRY, network_requirement, requires_local_signer_mode,
+};
use crate::runtime::config::{
OutputFormat as RuntimeOutputFormat, RuntimeConfig, SignerBackend, Verbosity,
};
@@ -59,15 +64,13 @@ fn run() -> Result<ExitCode, runtime::RuntimeError> {
render_config_from_target_args(&args, args.format.unwrap_or(TargetOutputFormat::Terminal));
if let Err(error) = validate_pre_runtime_request_contract(&request) {
let envelope = failure_envelope(&request, error);
- render_envelope(&envelope, &pre_runtime_render_config)?;
- return Ok(envelope_exit_code(&envelope));
+ return render_envelope(&envelope, &pre_runtime_render_config);
}
let config = match RuntimeConfig::from_system(&runtime_invocation_args_from_target(&args)) {
Ok(config) => config,
Err(error) => {
let envelope = runtime_config_failure_envelope(&request, error.into());
- render_envelope(&envelope, &pre_runtime_render_config)?;
- return Ok(envelope_exit_code(&envelope));
+ return render_envelope(&envelope, &pre_runtime_render_config);
}
};
request.set_output_format(OperationOutputFormat::from(config.output.format));
@@ -76,16 +79,14 @@ fn run() -> Result<ExitCode, runtime::RuntimeError> {
Ok(logging) => logging,
Err(error) => {
let envelope = runtime_config_failure_envelope(&request, error.into());
- render_envelope(&envelope, &runtime_render_config)?;
- return Ok(envelope_exit_code(&envelope));
+ return render_envelope(&envelope, &runtime_render_config);
}
};
let envelope = match validate_request_contract(&request, &config) {
Ok(()) => execute_request(request, &config, &logging),
Err(error) => failure_envelope(&request, error),
};
- render_envelope(&envelope, &runtime_render_config)?;
- Ok(envelope_exit_code(&envelope))
+ render_envelope(&envelope, &runtime_render_config)
}
fn execute_request(
@@ -594,7 +595,7 @@ fn terminal_verbosity_from_runtime(verbosity: Verbosity) -> TerminalVerbosity {
fn render_envelope(
envelope: &OutputEnvelope,
config: &EnvelopeRenderConfig,
-) -> Result<(), runtime::RuntimeError> {
+) -> Result<ExitCode, runtime::RuntimeError> {
match config.format {
RenderOutputFormat::Terminal => render_terminal_envelope(envelope, &config.terminal),
RenderOutputFormat::Json => {
@@ -602,7 +603,7 @@ fn render_envelope(
let mut handle = stdout.lock();
serde_json::to_writer_pretty(&mut handle, envelope)?;
writeln!(handle)?;
- Ok(())
+ Ok(envelope_exit_code(envelope))
}
RenderOutputFormat::Ndjson => {
let stdout = std::io::stdout();
@@ -611,7 +612,7 @@ fn render_envelope(
serde_json::to_writer(&mut handle, &frame)?;
writeln!(handle)?;
}
- Ok(())
+ Ok(envelope_exit_code(envelope))
}
}
}
@@ -619,17 +620,22 @@ fn render_envelope(
fn render_terminal_envelope(
envelope: &OutputEnvelope,
cx: &TerminalRenderContext,
-) -> Result<(), runtime::RuntimeError> {
+) -> Result<ExitCode, runtime::RuntimeError> {
let registry = terminal_renderer_registry();
- let document = registry
- .get(envelope.operation_id.as_str())
- .ok_or_else(|| {
- runtime::RuntimeError::Config(format!(
- "missing terminal renderer for {}",
- envelope.operation_id
- ))
- })?
- .render(envelope, cx);
+ if let Err(error) = registry.validate_against_operations(OPERATION_REGISTRY) {
+ let failure = terminal_registry_failure_envelope(envelope, error);
+ render_terminal_failure_document(&failure, cx)?;
+ return Ok(envelope_exit_code(&failure));
+ }
+ let renderer = match registry.renderer_for(envelope.operation_id.as_str()) {
+ Ok(renderer) => renderer,
+ Err(error) => {
+ let failure = terminal_registry_failure_envelope(envelope, error);
+ render_terminal_failure_document(&failure, cx)?;
+ return Ok(envelope_exit_code(&failure));
+ }
+ };
+ let document = renderer.render(envelope, cx);
let rendered = render_terminal_document(&document, cx);
if envelope.errors.is_empty() {
let stdout = std::io::stdout();
@@ -640,9 +646,52 @@ fn render_terminal_envelope(
let mut handle = stderr.lock();
writeln!(handle, "{rendered}")?;
}
+ Ok(envelope_exit_code(envelope))
+}
+
+fn render_terminal_failure_document(
+ envelope: &OutputEnvelope,
+ cx: &TerminalRenderContext,
+) -> Result<(), runtime::RuntimeError> {
+ let document = terminal_error_document(envelope);
+ let rendered = render_terminal_document(&document, cx);
+ let stderr = std::io::stderr();
+ let mut handle = stderr.lock();
+ writeln!(handle, "{rendered}")?;
Ok(())
}
+fn terminal_registry_failure_envelope(
+ envelope: &OutputEnvelope,
+ error: TerminalRendererRegistryError,
+) -> OutputEnvelope {
+ let mut output_error = OutputError::new(
+ "internal_error",
+ format!(
+ "terminal renderer registry invariant failed for {}",
+ envelope.operation_id
+ ),
+ CliExitCode::InternalError,
+ );
+ output_error.detail = Some(json!({
+ "class": "internal",
+ "operation_id": envelope.operation_id,
+ "violations": error.violations().iter().map(|violation| {
+ json!({
+ "kind": violation.kind(),
+ "operation_id": violation.operation_id(),
+ "count": violation.count(),
+ })
+ }).collect::<Vec<_>>(),
+ }));
+ let mut context = EnvelopeContext::new(envelope.request_id.clone(), envelope.dry_run);
+ context.output_format = envelope.output_format;
+ context.correlation_id = envelope.correlation_id.clone();
+ context.idempotency_key = envelope.idempotency_key.clone();
+ context.actor = envelope.actor.clone();
+ OutputEnvelope::failure(envelope.operation_id.clone(), output_error, context)
+}
+
fn envelope_exit_code(envelope: &OutputEnvelope) -> ExitCode {
envelope
.errors
@@ -654,3 +703,45 @@ fn envelope_exit_code(envelope: &OutputEnvelope) -> ExitCode {
fn operation_config_error(error: OperationAdapterError) -> runtime::RuntimeError {
runtime::RuntimeError::Config(error.to_string())
}
+
+#[cfg(test)]
+mod tests {
+ use serde_json::Value;
+
+ use crate::out::envelope::{EnvelopeContext, OutputEnvelope, OutputStatus};
+ use crate::out::terminal::errors::terminal_error_document;
+ use crate::out::terminal::registry::TerminalRendererRegistry;
+ use crate::out::terminal::renderer::{TerminalRenderContext, render_terminal_document};
+
+ use super::*;
+
+ #[test]
+ fn terminal_registry_failure_envelope_is_structured_internal_error() {
+ let original = OutputEnvelope::success(
+ "workspace.get",
+ Value::Null,
+ EnvelopeContext::new("req_test", false),
+ );
+ let error = TerminalRendererRegistry::new()
+ .validate_operation_ids(["workspace.get"])
+ .expect_err("missing renderer should fail");
+ let failure = terminal_registry_failure_envelope(&original, error);
+
+ assert_eq!(failure.status, OutputStatus::Error);
+ assert_eq!(failure.reason_code.as_deref(), Some("internal_error"));
+ assert_eq!(failure.errors[0].code, "internal_error");
+ let detail = failure.errors[0].detail.as_ref().expect("error detail");
+ assert_eq!(detail["violations"][0]["kind"], "missing");
+
+ let rendered = render_terminal_document(
+ &terminal_error_document(&failure),
+ &TerminalRenderContext::default(),
+ );
+ assert!(rendered.starts_with("✕ Command failed\n"));
+ assert!(
+ rendered
+ .contains("Reason terminal renderer registry invariant failed for workspace.get")
+ );
+ assert!(!rendered.contains("RuntimeError::Config"));
+ }
+}
diff --git a/src/out/terminal/registry.rs b/src/out/terminal/registry.rs
@@ -1,4 +1,8 @@
+use std::collections::{BTreeMap, BTreeSet};
+use std::fmt;
+
use crate::out::envelope::OutputEnvelope;
+use crate::registry::OperationSpec;
use super::layout::TerminalDocument;
use super::renderer::TerminalRenderContext;
@@ -10,6 +14,7 @@ pub trait TerminalOperationRenderer {
#[derive(Default)]
pub struct TerminalRendererRegistry {
entries: Vec<TerminalRendererEntry>,
+ violations: Vec<TerminalRendererRegistryViolation>,
}
struct TerminalRendererEntry {
@@ -21,6 +26,7 @@ impl TerminalRendererRegistry {
pub fn new() -> Self {
Self {
entries: Vec::new(),
+ violations: Vec::new(),
}
}
@@ -30,7 +36,8 @@ impl TerminalRendererRegistry {
renderer: &'static dyn TerminalOperationRenderer,
) -> Self {
if self.contains(operation_id) {
- panic!("duplicate terminal renderer registration for {operation_id}");
+ self.violations
+ .push(TerminalRendererRegistryViolation::duplicate(operation_id));
}
self.entries.push(TerminalRendererEntry {
operation_id,
@@ -46,10 +53,12 @@ impl TerminalRendererRegistry {
}
pub fn get(&self, operation_id: &str) -> Option<&'static dyn TerminalOperationRenderer> {
- self.entries
+ let mut matches = self
+ .entries
.iter()
- .find(|entry| entry.operation_id == operation_id)
- .map(|entry| entry.renderer)
+ .filter(|entry| entry.operation_id == operation_id);
+ let renderer = matches.next()?.renderer;
+ matches.next().is_none().then_some(renderer)
}
pub fn len(&self) -> usize {
@@ -59,6 +68,146 @@ impl TerminalRendererRegistry {
pub fn operation_ids(&self) -> impl Iterator<Item = &'static str> + '_ {
self.entries.iter().map(|entry| entry.operation_id)
}
+
+ pub fn renderer_for(
+ &self,
+ operation_id: &str,
+ ) -> Result<&'static dyn TerminalOperationRenderer, TerminalRendererRegistryError> {
+ self.get(operation_id).ok_or_else(|| {
+ TerminalRendererRegistryError::from_violation(
+ TerminalRendererRegistryViolation::missing(operation_id.to_owned()),
+ )
+ })
+ }
+
+ pub fn validate_against_operations(
+ &self,
+ operations: &[OperationSpec],
+ ) -> Result<(), TerminalRendererRegistryError> {
+ self.validate_operation_ids(operations.iter().map(|operation| operation.operation_id))
+ }
+
+ pub fn validate_operation_ids<'a>(
+ &self,
+ expected: impl IntoIterator<Item = &'a str>,
+ ) -> Result<(), TerminalRendererRegistryError> {
+ let expected = expected.into_iter().collect::<BTreeSet<_>>();
+ let mut counts = BTreeMap::<&str, usize>::new();
+ for operation_id in self.operation_ids() {
+ *counts.entry(operation_id).or_default() += 1;
+ }
+ let mut violations = self.violations.clone();
+ for operation_id in expected {
+ match counts.get(operation_id).copied().unwrap_or(0) {
+ 0 => {
+ violations.push(TerminalRendererRegistryViolation::missing(
+ operation_id.to_owned(),
+ ));
+ }
+ 1 => {}
+ count => {
+ violations.push(TerminalRendererRegistryViolation::duplicate_count(
+ operation_id.to_owned(),
+ count,
+ ));
+ }
+ }
+ }
+ violations.sort();
+ violations.dedup();
+ if violations.is_empty() {
+ Ok(())
+ } else {
+ Err(TerminalRendererRegistryError { violations })
+ }
+ }
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum TerminalRendererRegistryViolation {
+ DuplicateOperation { operation_id: String, count: usize },
+ MissingOperation { operation_id: String },
+}
+
+impl TerminalRendererRegistryViolation {
+ fn duplicate(operation_id: impl Into<String>) -> Self {
+ Self::DuplicateOperation {
+ operation_id: operation_id.into(),
+ count: 2,
+ }
+ }
+
+ fn duplicate_count(operation_id: impl Into<String>, count: usize) -> Self {
+ Self::DuplicateOperation {
+ operation_id: operation_id.into(),
+ count,
+ }
+ }
+
+ fn missing(operation_id: impl Into<String>) -> Self {
+ Self::MissingOperation {
+ operation_id: operation_id.into(),
+ }
+ }
+
+ pub fn kind(&self) -> &'static str {
+ match self {
+ Self::DuplicateOperation { .. } => "duplicate",
+ Self::MissingOperation { .. } => "missing",
+ }
+ }
+
+ pub fn operation_id(&self) -> &str {
+ match self {
+ Self::DuplicateOperation { operation_id, .. }
+ | Self::MissingOperation { operation_id } => operation_id,
+ }
+ }
+
+ pub fn count(&self) -> Option<usize> {
+ match self {
+ Self::DuplicateOperation { count, .. } => Some(*count),
+ Self::MissingOperation { .. } => None,
+ }
+ }
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct TerminalRendererRegistryError {
+ violations: Vec<TerminalRendererRegistryViolation>,
+}
+
+impl TerminalRendererRegistryError {
+ pub fn from_violation(violation: TerminalRendererRegistryViolation) -> Self {
+ Self {
+ violations: vec![violation],
+ }
+ }
+
+ pub fn violations(&self) -> &[TerminalRendererRegistryViolation] {
+ &self.violations
+ }
+}
+
+impl fmt::Display for TerminalRendererRegistryError {
+ fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+ formatter.write_str("terminal renderer registry invariant failed")?;
+ for violation in &self.violations {
+ match violation {
+ TerminalRendererRegistryViolation::DuplicateOperation {
+ operation_id,
+ count,
+ } => write!(
+ formatter,
+ "; duplicate renderer for {operation_id} ({count})"
+ )?,
+ TerminalRendererRegistryViolation::MissingOperation { operation_id } => {
+ write!(formatter, "; missing renderer for {operation_id}")?
+ }
+ }
+ }
+ Ok(())
+ }
}
pub fn terminal_renderer_registry() -> TerminalRendererRegistry {
@@ -80,8 +229,6 @@ pub fn terminal_renderer_registry() -> TerminalRendererRegistry {
#[cfg(test)]
mod tests {
- use std::collections::BTreeSet;
-
use crate::out::terminal::layout::{TerminalDocument, TerminalHeader, TerminalSymbol};
use crate::registry::OPERATION_REGISTRY;
@@ -112,11 +259,34 @@ mod tests {
}
#[test]
- #[should_panic(expected = "duplicate terminal renderer registration for workspace.get")]
fn duplicate_operation_renderer_registration_fails() {
- let _registry = TerminalRendererRegistry::new()
+ let registry = TerminalRendererRegistry::new()
.register("workspace.get", &TEST_RENDERER)
.register("workspace.get", &TEST_RENDERER);
+ let error = registry
+ .validate_operation_ids(["workspace.get"])
+ .expect_err("duplicate renderer registration should fail");
+
+ assert_eq!(
+ error.violations(),
+ &[TerminalRendererRegistryViolation::duplicate_count(
+ "workspace.get",
+ 2
+ )]
+ );
+ assert!(registry.get("workspace.get").is_none());
+ }
+
+ #[test]
+ fn missing_operation_renderer_registration_fails() {
+ let error = TerminalRendererRegistry::new()
+ .validate_operation_ids(["workspace.get"])
+ .expect_err("missing renderer registration should fail");
+
+ assert_eq!(
+ error.violations(),
+ &[TerminalRendererRegistryViolation::missing("workspace.get")]
+ );
}
#[test]
@@ -130,7 +300,9 @@ mod tests {
assert_eq!(registry.len(), OPERATION_REGISTRY.len());
assert_eq!(actual, expected);
- assert_eq!(registry.len(), 76);
+ registry
+ .validate_against_operations(OPERATION_REGISTRY)
+ .expect("terminal registry covers operation registry");
for operation_id in expected {
assert!(
registry.contains(operation_id),
