feat(clients): CLI-04 typed single-item command parity (+CLI-30 unregister) — 4/5 clients

Every parity-critical single-item MXAccess command now has a typed session helper instead of only a raw-Invoke escape hatch. Added per client: - Phase 1: AdviseSupervisory, WriteSecured, WriteSecured2, AuthenticateUser, ArchestrAUserToId - Phase 2: AddBufferedItem, SetBufferedUpdateInterval, Suspend, Activate - CLI-30: Unregister (Rust + .NET; Go/Python already had it) Each wraps the existing raw-command machinery (no new wire surface) and runs the same MXAccess-level reply validation (hresult < 0 + MxStatusProxy). MXAccess parity preserved: WriteSecured before AuthenticateUser+AdviseSupervisory surfaces the native failure unchanged (not pre-validated/reordered). Credentials (AuthenticateUser password, WriteSecured payloads) route through each client's secret-redaction seam and never reach logs/exceptions/ToString/Debug/Display; each suite asserts a distinctive credential is absent from surfaced errors. New CLI subcommands source credentials via flag/env, never echoed. - .NET: 21 helpers (validated + Raw), CLI subcommands, multi-secret CLI redactor. Build clean (0 warn), 102 passed. - Go: 9 helpers + *Raw variants, redactSecrets seam, promoted CLI advise-supervisory to typed. gofmt/vet/build/test clean. - Rust: 10 helpers incl. unregister; verified ensure_mxaccess_success runs on secured paths; error.rs credential scrub. fmt/check/test/clippy clean. - Python: 9 async helpers, redact_secret seam + _invoke_redacted, CLI commands. 145 passed. - Shared doc: ClientLibrariesDesign "Typed Command Parity" section. Java client typed parity is batched to windev (no local JRE); CLI-04 + CLI-30 stay open until it lands. Claude-Session: https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW
2026-07-09 16:41:43 -04:00
parent 4090a478c8
commit bde042b4d4
21 changed files with 3496 additions and 97 deletions
@@ -0,0 +1,224 @@
+package mxgateway
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+
+	pb "gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated"
+)
+
+// TestAdviseSupervisoryBuildsCommandAndExposesRawReply pins that the promoted
+// typed helper emits an ADVISE_SUPERVISORY command carrying the server/item
+// handles and returns the raw reply.
+func TestAdviseSupervisoryBuildsCommandAndExposesRawReply(t *testing.T) {
+	fake := &fakeGatewayServer{
+		invokeReply: &pb.MxCommandReply{
+			SessionId:      "session-1",
+			Kind:           pb.MxCommandKind_MX_COMMAND_KIND_ADVISE_SUPERVISORY,
+			ProtocolStatus: &pb.ProtocolStatus{Code: pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_OK},
+		},
+	}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	reply, err := session.AdviseSupervisoryRaw(context.Background(), 12, 34)
+	if err != nil {
+		t.Fatalf("AdviseSupervisoryRaw() error = %v", err)
+	}
+	if reply.GetKind() != pb.MxCommandKind_MX_COMMAND_KIND_ADVISE_SUPERVISORY {
+		t.Fatalf("reply kind = %s", reply.GetKind())
+	}
+	cmd := fake.invokeRequest.GetCommand()
+	if cmd.GetKind() != pb.MxCommandKind_MX_COMMAND_KIND_ADVISE_SUPERVISORY {
+		t.Fatalf("command kind = %s", cmd.GetKind())
+	}
+	if cmd.GetAdviseSupervisory().GetServerHandle() != 12 || cmd.GetAdviseSupervisory().GetItemHandle() != 34 {
+		t.Fatalf("advise-supervisory handles = (%d, %d), want (12, 34)",
+			cmd.GetAdviseSupervisory().GetServerHandle(), cmd.GetAdviseSupervisory().GetItemHandle())
+	}
+
+	// The error-returning wrapper drops the reply but must not error on success.
+	if err := session.AdviseSupervisory(context.Background(), 12, 34); err != nil {
+		t.Fatalf("AdviseSupervisory() error = %v", err)
+	}
+}
+
+// TestWriteSecuredSurfacesNativeFailureWithoutPriorAuthenticate pins MXAccess
+// parity: WriteSecured issued without a preceding AuthenticateUser is rejected
+// natively, and the client surfaces that failure as a typed MxAccessError rather
+// than pre-validating or reordering. The write value is also kept out of the
+// surfaced error text.
+func TestWriteSecuredSurfacesNativeFailureWithoutPriorAuthenticate(t *testing.T) {
+	hresult := int32(-2147024891) // E_ACCESSDENIED
+	fake := &fakeGatewayServer{
+		invokeReply: &pb.MxCommandReply{
+			SessionId:         "session-1",
+			Kind:              pb.MxCommandKind_MX_COMMAND_KIND_WRITE_SECURED,
+			Hresult:           &hresult,
+			DiagnosticMessage: "WriteSecured requires a prior AuthenticateUser",
+			ProtocolStatus: &pb.ProtocolStatus{
+				Code:    pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_MXACCESS_FAILURE,
+				Message: "MXAccess failed",
+			},
+		},
+	}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	securedValue := "supersecret-payload"
+	err := session.WriteSecured(context.Background(), 12, 34, 0, 0, StringValue(securedValue))
+
+	var mxErr *MxAccessError
+	if !errors.As(err, &mxErr) {
+		t.Fatalf("error %T does not support errors.As(*MxAccessError); err = %v", err, err)
+	}
+	if strings.Contains(err.Error(), securedValue) {
+		t.Fatalf("surfaced error leaked the secured payload: %q", err.Error())
+	}
+
+	// The command must carry the secured fields verbatim (parity: unaltered).
+	cmd := fake.invokeRequest.GetCommand()
+	if cmd.GetKind() != pb.MxCommandKind_MX_COMMAND_KIND_WRITE_SECURED {
+		t.Fatalf("command kind = %s", cmd.GetKind())
+	}
+	if cmd.GetWriteSecured().GetValue().GetStringValue() != securedValue {
+		t.Fatalf("wire value = %q, want %q", cmd.GetWriteSecured().GetValue().GetStringValue(), securedValue)
+	}
+}
+
+// TestWriteSecuredRejectsNilValueWithoutRoundTrip pins the client-side required
+// guard, which never echoes the (absent) value.
+func TestWriteSecuredRejectsNilValue(t *testing.T) {
+	fake := &fakeGatewayServer{}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	if err := session.WriteSecured(context.Background(), 1, 2, 0, 0, nil); err == nil ||
+		!strings.Contains(err.Error(), "write-secured value is required") {
+		t.Fatalf("WriteSecured(nil value) error = %v", err)
+	}
+}
+
+// TestAuthenticateUserReturnsUserIDOnHappyPath pins the typed helper unpacking of
+// AuthenticateUserReply.user_id and that the credential is carried on the wire
+// but never surfaced.
+func TestAuthenticateUserReturnsUserIDOnHappyPath(t *testing.T) {
+	fake := &fakeGatewayServer{
+		invokeReply: &pb.MxCommandReply{
+			SessionId:      "session-1",
+			Kind:           pb.MxCommandKind_MX_COMMAND_KIND_AUTHENTICATE_USER,
+			ProtocolStatus: &pb.ProtocolStatus{Code: pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_OK},
+			Payload: &pb.MxCommandReply_AuthenticateUser{
+				AuthenticateUser: &pb.AuthenticateUserReply{UserId: 4242},
+			},
+		},
+	}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	userID, err := session.AuthenticateUser(context.Background(), 12, "operator", "hunter2-password")
+	if err != nil {
+		t.Fatalf("AuthenticateUser() error = %v", err)
+	}
+	if userID != 4242 {
+		t.Fatalf("user id = %d, want 4242", userID)
+	}
+
+	cmd := fake.invokeRequest.GetCommand()
+	if cmd.GetKind() != pb.MxCommandKind_MX_COMMAND_KIND_AUTHENTICATE_USER {
+		t.Fatalf("command kind = %s", cmd.GetKind())
+	}
+	if cmd.GetAuthenticateUser().GetVerifyUser() != "operator" {
+		t.Fatalf("verify user = %q, want operator", cmd.GetAuthenticateUser().GetVerifyUser())
+	}
+	if cmd.GetAuthenticateUser().GetVerifyUserPassword() != "hunter2-password" {
+		t.Fatalf("password not carried to the wire verbatim")
+	}
+}
+
+// TestAuthenticateUserScrubsCredentialFromSurfacedError proves the redaction
+// seam: even when the gateway diagnostic message echoes the credential back, the
+// surfaced error redacts it while the typed MxAccessError remains reachable via
+// errors.As.
+func TestAuthenticateUserScrubsCredentialFromSurfacedError(t *testing.T) {
+	password := "hunter2-password"
+	fake := &fakeGatewayServer{
+		invokeReply: &pb.MxCommandReply{
+			SessionId:         "session-1",
+			Kind:              pb.MxCommandKind_MX_COMMAND_KIND_AUTHENTICATE_USER,
+			DiagnosticMessage: "authentication failed for password " + password,
+			// Message is intentionally left empty so MxAccessError.Error() falls
+			// through to the diagnostic message — the free-text field that could
+			// otherwise echo the credential back to a caller's log.
+			ProtocolStatus: &pb.ProtocolStatus{
+				Code: pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_MXACCESS_FAILURE,
+			},
+		},
+	}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	_, err := session.AuthenticateUser(context.Background(), 12, "operator", password)
+	if err == nil {
+		t.Fatal("AuthenticateUser() returned no error on native failure")
+	}
+	if strings.Contains(err.Error(), password) {
+		t.Fatalf("surfaced error leaked the credential: %q", err.Error())
+	}
+	if !strings.Contains(err.Error(), redactedSecretMarker) {
+		t.Fatalf("surfaced error missing redaction marker: %q", err.Error())
+	}
+	var mxErr *MxAccessError
+	if !errors.As(err, &mxErr) {
+		t.Fatalf("redaction wrapper broke errors.As(*MxAccessError); err = %v", err)
+	}
+}
+
+// TestAuthenticateUserRequiresVerifyUser pins the client-side required guard.
+func TestAuthenticateUserRequiresVerifyUser(t *testing.T) {
+	fake := &fakeGatewayServer{}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	if _, err := session.AuthenticateUser(context.Background(), 12, "", "pw"); err == nil ||
+		!strings.Contains(err.Error(), "verify user is required") {
+		t.Fatalf("AuthenticateUser(empty user) error = %v", err)
+	}
+}
+
+// TestSuspendActivateReturnStatus covers two Phase 2 helpers that unpack an
+// MxStatusProxy from their dedicated reply arms.
+func TestSuspendActivateReturnStatus(t *testing.T) {
+	fake := &fakeGatewayServer{
+		invokeReply: &pb.MxCommandReply{
+			SessionId:      "session-1",
+			Kind:           pb.MxCommandKind_MX_COMMAND_KIND_SUSPEND,
+			ProtocolStatus: &pb.ProtocolStatus{Code: pb.ProtocolStatusCode_PROTOCOL_STATUS_CODE_OK},
+			Payload: &pb.MxCommandReply_Suspend{
+				Suspend: &pb.SuspendReply{Status: &pb.MxStatusProxy{Success: 1, DiagnosticText: "suspended"}},
+			},
+		},
+	}
+	client, cleanup := newBufconnClient(t, fake)
+	defer cleanup()
+	session := NewSessionForID(client, "session-1")
+
+	status, err := session.Suspend(context.Background(), 12, 34)
+	if err != nil {
+		t.Fatalf("Suspend() error = %v", err)
+	}
+	if status.GetDiagnosticText() != "suspended" {
+		t.Fatalf("status diagnostic = %q, want suspended", status.GetDiagnosticText())
+	}
+	if fake.invokeRequest.GetCommand().GetSuspend().GetItemHandle() != 34 {
+		t.Fatalf("suspend item handle = %d, want 34", fake.invokeRequest.GetCommand().GetSuspend().GetItemHandle())
+	}
+}