feature: grpc: Implement GetLastError

[frhuelsz]

🔍 Description

Implements GetLastError RPC.

[Copilot]

Pull request overview

Implements the GetLastError gRPC RPC by wiring it to the Trident datastore and exposing structured error information over the Harpoon API, and introduces a categorization layer for Trident errors that is reused in both serialization and RPC mapping.

Changes:

• Adjusts TridentErrorKind in the protobuf schema to add an explicit UNSPECIFIED value and renumber existing error kinds.
• Refactors trident_api::error serialization to use an ErrorCategory enum and explicit string constants, and adds roundtrip tests for category ↔ string mapping.
• Implements get_last_error on the Trident gRPC server, adds a new datastore module to decode serialized TridentError YAML into HarpoonTridentError, and tests the conversion logic.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
proto/harpoon.proto	Adds a default TRIDENT_ERROR_KIND_UNSPECIFIED to TridentErrorKind and renumbers existing enum values to support an explicit unspecified kind in the wire protocol.
crates/trident_api/src/error.rs	Introduces ErrorCategory as an intermediate mapping for ErrorKind to strings and Harpoon enums, centralizes serialization field names, and adds tests for category roundtrips.
crates/trident/src/server/tridentserver/mod.rs	Registers the new datastore submodule used to implement GetLastError.
crates/trident/src/server/tridentserver/harpoon_impl.rs	Implements the get_last_error RPC by opening the datastore, reading the last error, and mapping it to the Harpoon TridentError type (or returning None if no datastore exists).
crates/trident/src/server/tridentserver/datastore.rs	Adds logic to extract and adapt the last serialized TridentError from the datastore YAML into a HarpoonTridentError, along with a focused unit test verifying the conversion behavior.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[Copilot]

Issue (API compatibility): Adding TRIDENT_ERROR_KIND_UNSPECIFIED = 0 and shifting all existing enum variants by +1 changes the on-the-wire numeric values for TridentErrorKind, which is a breaking change for any existing Harpoon/Trident clients or stored data that interpret these codes. Evidence: previously EXECUTION_ENVIRONMENT_MISCONFIGURATION was 0 (and so on up to UNSUPPORTED_CONFIGURATION = 6), and after this change all of those values move, while TridentError is already used in several existing RPCs (e.g., FinalStatus.error, GetLastErrorResponse.error, ValidateHostConfigurationResponse.error). Suggestion: preserve the original numeric assignments for existing variants and introduce the UNSPECIFIED value either as an alias for an existing value or at the end of the enum (or add documentation that 0 is reserved but do not change the established wire numbers).

[Copilot]

Issue (subkind extraction): harpoon_trident_error_from_value attempts to derive the subkind from a YAML Value::Tagged, but TridentError serialization currently writes the error field as a plain string (see test_error_serialize in trident_api/src/error.rs), so this logic will usually fall through to UNKNOWN_VALUE even when a specific subkind is available. Evidence: for structured errors stored via serde_yaml::to_value(&TridentError), the error field is serialized as Value::String("load-host-status"), not a tagged value, so subkind here will always be set to "unknown". Suggestion: read subkind from the string value of the error field (e.g., as_str() on that value) and only fall back to UNKNOWN_VALUE when the field is missing or not a string, instead of relying on YAML tags.

Suggested change

	// Extract the subkind, which is encoded in the tag of the error value.
	let subkind = root
	.get(s(TridentError::SERIALIZE_FIELD_ERROR))
	.and_then(|v| {
	if let Value::Tagged(t) = v {
	Some(&t.tag)
	} else {
	None
	}
	})
	.map(|t| {
	let raw = t.to_string();
	// Remove leading "!" if present.
	match raw.strip_prefix("!") {
	Some(s) => s.to_string(),
	None => raw,
	}
	})
	.unwrap_or_else(|| UNKNOWN_VALUE.to_string())
	.to_string();
	// Extract the subkind from the error field. Prefer the string value, and fall back
	// to older tagged encodings if present.
	let subkind = root
	.get(s(TridentError::SERIALIZE_FIELD_ERROR))
	.and_then(|v| {
	// Current serialization stores the error as a plain string.
	if let Some(s) = v.as_str() {
	return Some(s.to_string());
	}
	// Backwards compatibility: handle tagged values if they appear.
	if let Value::Tagged(t) = v {
	let raw = t.tag.to_string();
	// Remove leading "!" if present.
	let cleaned = match raw.strip_prefix('!') {
	Some(s) => s.to_string(),
	None => raw,
	};
	return Some(cleaned);
	}
	None
	})
	.unwrap_or_else(|| UNKNOWN_VALUE.to_string());

[fintelia]

Would we be able to define a struct and deserialize into it, then convert to a HarpoonTridentError? If possible, that seems preferable to rolling our own deserialization.

[frhuelsz]

I took a quick stab at that approach, but it lead to some duplication and also there is no great way of deserializing the yaml !tag that gets produced for the subkind.

A possibly better approach would be to standardize and lockdown error serialization using a structure that is more similar to the grpc error definition.

[fintelia]

I'd feel better about doing one of those approaches. If we have a custom deserializer here then we probably need to test it much more extensively than simply checking a single error round-trips properly (which would be a lot of work).