Skip to content

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

Merged
intel352 merged 1 commit intomainfrom
fix/diag-detail-grpc
May 2, 2026
Merged

fix(grpc): serialize Diagnostic.Detail across plugin boundary (closes #52, restores deploy-log visibility)#53
intel352 merged 1 commit intomainfrom
fix/diag-detail-grpc

Conversation

@intel352
Copy link
Copy Markdown
Contributor

@intel352 intel352 commented May 2, 2026

Summary

Closes the silent-drop bug that nullified PR-E2 (v0.8.3) + PR-E4 (v0.8.4) deploy-log fetch work. Also closes #52 (CHANGELOG duplicate ### Fixed heading).

Bug

`internal/module_instance.go::invokeDriverTroubleshoot` serialized `Diagnostic` to `map[string]any` but omitted the `detail` field. wfctl's `remoteResourceDriver.Troubleshoot` reads `m["detail"]` → empty → `emitDiagnostics` never printed Detail body. All work in `attachDeployLogs` to populate Diagnostic.Detail with deploy-log content + visible failure notes was silently dropped at the gRPC boundary.

Symptom (run 25261030300, post-PR-E5 with v0.8.4):

```
##[group]Troubleshoot: coredump-nats-staging
[deploy.components.coredump-nats-staging.wait] f066ea13 — Your deploy failed because your container exited with a non-zero exit code. (at 2026-05-02T20:25:29Z)
[CANCELED] 0059b12b — app spec updated (at 2026-05-02T20:25:26Z)
[deploy.components.coredump-nats-staging.wait] f42b2596 — Your deploy failed because your container exited with a non-zero exit code. (at 2026-05-02T19:36:50Z)
##[endgroup]
```

Cause line ✓ but no Detail block, no log content, no failure-mode notes — exactly what we'd been working to surface for the past 4 plugin releases.

Fix

One-line addition to the diagnostics list-builder:
```go
diagList[i] = map[string]any{
"id": d.ID,
"phase": d.Phase,
"cause": d.Cause,
"at": d.At.Format(time.RFC3339),
"detail": d.Detail, // NEW
}
```

Plus regression test (`TestGRPCDispatch_ResourceDriver_Troubleshoot_Detail_RoundTripFidelity`) that exercises the full grpcRoundTrip → InvokeMethod → response path with a fakeTroubleshootingDriver producing a Diagnostic with Detail set, and asserts the field survives intact.

CHANGELOG dedup (closes #52)

Removed the duplicate `### Fixed` heading PR-E4 introduced. Now exactly one `### Added` and one `### Fixed` under [Unreleased].

Test plan

  • go test ./internal/... -count=1 (full suite green)
  • New regression test passes
  • Validates against next core-dump staging deploy (will surface actual container stdout/stderr OR a clear visible-failure note in Diagnostic.Detail)

Why this is a structpb-boundary class bug

Per workspace memory `feedback_workflow_plugin_structpb_boundary` — domain types crossing the gRPC plugin boundary need every field enumerated in the dispatch translator. Diagnostic.Detail wasn't a typed slice but the same pattern (translator forgets a field → silently drops at boundary). Adding a similar regression test for every field of every domain type that crosses the boundary would catch this whole class of bug.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

…lus closes the silent-drop bug)

Root cause: invokeDriverTroubleshoot in internal/module_instance.go serialized
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 deploy
log content + visible failure notes was silently dropped at the gRPC
boundary.

Symptom: core-dump staging deploy run 25261030300 (post-PR-E5 with v0.8.4)
emitted Troubleshoot block with the Cause line for each failed deployment
("Your deploy failed because your container exited with a non-zero exit
code") but no log content and no failure-mode notes — exactly the operator-
visible output we'd been working to fix for the past 4 plugin releases.

This is the structpb-boundary class of bug warned about in workspace memory
feedback_workflow_plugin_structpb_boundary: "typed slices ... in dispatch
args/Outputs are BLOCKERS under legacy compat". Detail wasn't a typed slice,
but the same pattern — fields in domain types that aren't enumerated in the
gRPC dispatch translator silently drop at the boundary.

Fix: one-line addition to the diagnostics list-builder in
invokeDriverTroubleshoot:

  diagList[i] = map[string]any{
      "id":     d.ID,
      "phase":  d.Phase,
      "cause":  d.Cause,
      "at":     d.At.Format(time.RFC3339),
      "detail": d.Detail,   // NEW
  }

Plus regression test:
TestGRPCDispatch_ResourceDriver_Troubleshoot_Detail_RoundTripFidelity uses a
new fakeTroubleshootingDriver and fakeProviderReturningDriver to exercise
the full grpcRoundTrip → InvokeMethod("ResourceDriver.Troubleshoot") →
serialized response path, asserting the "detail" field survives intact.

CHANGELOG also consolidated: removed the duplicate ### Fixed heading under
[Unreleased] that PR-E4 introduced (Copilot post-merge review on PR #51
flagged it; tracked as plugin#52). Now there is exactly one ### Added and
one ### Fixed under [Unreleased].

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 2, 2026 20:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a gRPC dispatch bug in the plugin layer where Diagnostic.Detail was not being serialized out of ResourceDriver.Troubleshoot, which prevented deploy/build log details from reaching wfctl. It also adds a regression test around the dispatch path and cleans up the unreleased changelog entry structure.

Changes:

  • Add detail to the serialized troubleshooting diagnostic map in invokeDriverTroubleshoot.
  • Add a regression test covering troubleshooting diagnostic detail through the dispatch boundary.
  • Consolidate the duplicate ### Fixed heading in CHANGELOG.md and document the bug fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/module_instance.go Fixes troubleshooting diagnostic serialization so Detail crosses the plugin boundary.
internal/grpc_dispatch_test.go Adds regression coverage for ResourceDriver.Troubleshoot diagnostic detail dispatch behavior.
CHANGELOG.md Merges duplicate unreleased Fixed sections and records the serialization fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CHANGELOG.md

### 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.
@intel352 intel352 merged commit b0aa27c into main May 2, 2026
7 checks passed
@intel352 intel352 deleted the fix/diag-detail-grpc branch May 2, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CHANGELOG.md [Unreleased] has duplicate ### Fixed headings post-PR-E4

2 participants