fix(grpc): serialize Diagnostic.Detail across plugin boundary (closes #52, restores deploy-log visibility)

CHANGELOG.md

@@ -4,12 +4,6 @@ All notable changes to workflow-plugin-digitalocean are documented here.
 
 ## [Unreleased]
 
-### Fixed
-
-- **Troubleshoot deploy/build log fetch** — Fixed two issues that prevented log blocks from appearing in operator-facing Diagnostic output:
-  - `deploymentComponents` now reads component names from `dep.Services / .StaticSites / .Workers / .Jobs / .Functions` (deployment-level arrays populated by both ListDeployments and GetDeployment) before falling back to `dep.Spec.*` and finally `[""]` aggregate. Previously only Spec was inspected, which is nil from ListDeployments, so the empty-aggregate fallback was always hit and DO API returned no logs.
-  - GetLogs API errors, HTTP-fetch errors, empty HistoricURLs, and empty-body responses now append a brief failure note to `Diagnostic.Detail` (in addition to the existing stderr log, which is captured at hashicorp/go-plugin TRACE level and not surfaced to operators). Operators now see the failure mode in the same Troubleshoot block as the rest of the diagnostic output.
-
 ### Added
 
 - **Troubleshoot fetches DO deploy/build logs** (PR-E2) — `AppPlatformDriver.Troubleshoot`
@@ -73,6 +67,12 @@ All notable changes to workflow-plugin-digitalocean are documented here.
 
 ### Fixed
 
+- **gRPC Diagnostic.Detail field omitted from plugin response** — `internal/module_instance.go::invokeDriverTroubleshoot` serialised `Diagnostic` to `map[string]any` but omitted the `detail` field. wfctl's `remoteResourceDriver.Troubleshoot` reads `m["detail"]` → empty string → `emitDiagnostics` never printed the Detail body. All work done in `attachDeployLogs` (v0.8.3 + v0.8.4) to populate `Diagnostic.Detail` with log content / failure notes was silently dropped at the gRPC boundary. Added `"detail": d.Detail` to the serialisation map. This is the structpb-boundary class of bug previously documented in workspace memory.
+
+- **Troubleshoot deploy/build log fetch** — Fixed two issues that prevented log blocks from appearing in operator-facing Diagnostic output:
+  - `deploymentComponents` now reads component names from `dep.Services / .StaticSites / .Workers / .Jobs / .Functions` (deployment-level arrays populated by both ListDeployments and GetDeployment) before falling back to `dep.Spec.*` and finally `[""]` aggregate. Previously only Spec was inspected, which is nil from ListDeployments, so the empty-aggregate fallback was always hit and DO API returned no logs.
+  - GetLogs API errors, HTTP-fetch errors, empty HistoricURLs, and empty-body responses now append a brief failure note to `Diagnostic.Detail` (in addition to the existing stderr log, which is captured at hashicorp/go-plugin TRACE level and not surfaced to operators). Operators now see the failure mode in the same Troubleshoot block as the rest of the diagnostic output.
+
 - **DetectDrift Config-drift detection**: out of scope for this release. The
   IaCProvider interface signature receives only refs, not the parsed declared
   config, so per-driver Diff comparisons cannot be performed safely (VPC reads

internal/grpc_dispatch_test.go

@@ -17,6 +17,7 @@ package internal
 // comment starting "REGRESSION(v0.7.5)".
 
 import (
+	"context"
 	"testing"
 
 	"github.com/GoCodeAlone/workflow/interfaces"
@@ -760,3 +761,84 @@ func TestGRPCDispatch_IaCProvider_Destroy_RoundTripFidelity(t *testing.T) {
 		t.Errorf("lastRefs[1].Name = %q, want %q", fake.lastRefs[1].Name, "my-cache")
 	}
 }
+
+// fakeTroubleshootingDriver implements ResourceDriver + Troubleshooter so we
+// can exercise invokeDriverTroubleshoot's serialization path. The default
+// stubResourceDriver does NOT implement Troubleshooter (so the dispatch
+// returns codes.Unimplemented), which is fine for the panic-check tests but
+// not for the round-trip-fidelity test we need below.
+type fakeTroubleshootingDriver struct {
+	stubResourceDriver
+	diags []interfaces.Diagnostic
+	err   error
+}
+
+func (f *fakeTroubleshootingDriver) Troubleshoot(_ context.Context, _ interfaces.ResourceRef, _ string) ([]interfaces.Diagnostic, error) {
+	return f.diags, f.err
+}
+
+// fakeProviderReturningDriver returns a fixed driver from ResourceDriver
+// regardless of which type is asked for. Used to inject a Troubleshooter-
+// implementing driver into the dispatch path without modifying every test
+// fixture.
+type fakeProviderReturningDriver struct {
+	fakeIaCProvider
+	driver interfaces.ResourceDriver
+}
+
+func (f *fakeProviderReturningDriver) ResourceDriver(_ string) (interfaces.ResourceDriver, error) {
+	return f.driver, nil
+}
+
+// REGRESSION(v0.8.5): invokeDriverTroubleshoot must serialize Diagnostic.Detail
+// across the gRPC boundary. A prior version (v0.8.3 + v0.8.4) populated
+// Diagnostic.Detail with deploy-log content + visible failure notes via
+// attachDeployLogs but the dispatch handler omitted "detail" from the response
+// map[string]any — wfctl's remoteResourceDriver read m["detail"] → empty string
+// → emitDiagnostics never printed the Detail body, silently dropping all log
+// content at the gRPC boundary.
+func TestGRPCDispatch_ResourceDriver_Troubleshoot_Detail_RoundTripFidelity(t *testing.T) {
+	wantDetail := "---\nDeploy logs — component \"svc\" (last 200 lines):\nERROR: container exited with code 1\n---"
+	td := &fakeTroubleshootingDriver{
+		diags: []interfaces.Diagnostic{
+			{
+				ID:     "f42b2596",
+				Phase:  "deploy.components.svc.wait",
+				Cause:  "Your deploy failed because your container exited with a non-zero exit code.",
+				Detail: wantDetail,
+			},
+		},
+	}
+	provider := &fakeProviderReturningDriver{driver: td}
+	mi := &doModuleInstance{provider: provider}
+
+	args := grpcRoundTrip(t, map[string]any{
+		"resource_type":   "infra.container_service",
+		"ref_name":        "test-svc",
+		"ref_provider_id": "app-123",
+		"ref_type":        "infra.container_service",
+		"failure_msg":     "health check timed out",
+	})
+	res, err := mi.InvokeMethod("ResourceDriver.Troubleshoot", args)
+	if err != nil {
+		t.Fatalf("Troubleshoot dispatch: %v", err)
+	}
+	rawDiags, ok := res["diagnostics"].([]any)
+	if !ok || len(rawDiags) != 1 {
+		t.Fatalf("diagnostics shape: got %T len=%d, want []any len=1", res["diagnostics"], len(rawDiags))
+	}
+	m, ok := rawDiags[0].(map[string]any)
+	if !ok {
+		t.Fatalf("diag[0]: got %T, want map[string]any", rawDiags[0])
+	}
+	gotDetail, _ := m["detail"].(string)
+	if gotDetail != wantDetail {
+		t.Errorf("Detail field missing or mismatched after gRPC dispatch:\n got=%q\nwant=%q", gotDetail, wantDetail)
+	}
+	if got := m["id"]; got != "f42b2596" {
+		t.Errorf("ID: got %v, want f42b2596", got)
+	}
+	if got := m["cause"]; got == nil {
+		t.Errorf("Cause should be present")
+	}
+}

internal/module_instance.go

@@ -419,10 +419,11 @@ func (m *doModuleInstance) invokeDriverTroubleshoot(args map[string]any) (map[st
 	diagList := make([]any, len(diags))
 	for i, d := range diags {
 		diagList[i] = map[string]any{
-			"id":    d.ID,
-			"phase": d.Phase,
-			"cause": d.Cause,
-			"at":    d.At.Format(time.RFC3339),
+			"id":     d.ID,
+			"phase":  d.Phase,
+			"cause":  d.Cause,
+			"at":     d.At.Format(time.RFC3339),
+			"detail": d.Detail,
 		}
 	}
 	return map[string]any{"diagnostics": diagList}, nil