fix(featureflags): use proto entity and proto json serializer

[shijiesheng]

What changed?

Use proto entity of FeatureFlags instead so it's consistent across languages. It's backward/forward compatible because the proto json marshaler returns the same result with thrift IDL by cadence-workflow/cadence-idl#245

Why?

Proto has better language consistency than thrift.

How did you test it?

Added Unit Test to make sure the thrift marshaled json string can be deserialized into Proto entity.

Potential risks

Release notes

Documentation Changes

---

Reviewer Validation

PR Description Quality (check these before reviewing code):

• "What changed" provides a clear 1-2 line summary
  • Project Issue is linked
• "Why" explains the full motivation with sufficient context
• Testing is documented:
  • Unit test commands are included (with exact go test invocation)
  • Integration test setup/commands included (if integration tests were run)
  • Canary testing details included (if canary was mentioned)
• Potential risks section is thoughtfully filled out (or legitimately N/A)
• Release notes included if this completes a user-facing feature
• Documentation needs are addressed (or noted if uncertain)

[gitar-bot]

🚨 Bug: Interface change breaks handler_test.go mock expectations

The VersionChecker interface change from shared.FeatureFlags to apiv1.FeatureFlags in SupportsWorkflowAlreadyCompletedError was not propagated to service/frontend/api/handler_test.go. That file (lines 4748 and 4752) still passes shared.FeatureFlags{} to the gomock expectation:

s.mockVersionChecker.EXPECT().SupportsWorkflowAlreadyCompletedError("impl-header", "feature-version", shared.FeatureFlags{}).Return(nil)
s.mockVersionChecker.EXPECT().SupportsWorkflowAlreadyCompletedError("impl-header", "feature-version", shared.FeatureFlags{}).Return(errors.New("error"))

While this compiles (gomock recorder uses any typed parameters), at runtime gomock uses reflect.DeepEqual to match expected vs actual arguments. The actual code path now produces apiv1.FeatureFlags{} (from GetFeatureFlagsFromHeader), which will never match shared.FeatureFlags{} — these are entirely different Go types. This will cause the TestNormalizeVersionedErrors test to panic with an unexpected call error.

Fix: Update service/frontend/api/handler_test.go to use apiv1.FeatureFlags{} at both lines, and update its imports accordingly.

_{Was this helpful? React with 👍 / 👎}

[shijiesheng]

No it doesn't. handler doesn't directly mock on this interface but instead use the versionChecker_mock, which is already changed

[gitar-bot]

You're correct that versionChecker_mock.go was updated with the proto type. However, the test expectations in handler_test.go still use shared.FeatureFlags{}.

Details

At handler_test.go:4748 and 4752:

s.mockVersionChecker.EXPECT().SupportsWorkflowAlreadyCompletedError("impl-header", "feature-version", shared.FeatureFlags{}).Return(nil)

This needs to be updated to:

s.mockVersionChecker.EXPECT().SupportsWorkflowAlreadyCompletedError("impl-header", "feature-version", apiv1.FeatureFlags{}).Return(nil)

The mock interface signature was correctly updated, but the test code that uses the mock wasn't updated to match. This is what's causing the gomock type mismatch error in CI.

[shijiesheng]

not used

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Clean migration from Thrift to Proto FeatureFlags with proper serialization, consistent type updates across all consumers, and good test coverage including round-trip validation. Previous finding about handler_test.go mock mismatch was a false positive — all types are properly updated.

✅ 1 resolved

• ✅ Interface change breaks handler_test.go mock expectations

Rules ❌ No requirements met

Repository Rules

❌ PR Description Quality Standards: Complete [How did you test it?] with exact test commands: `go test -v ./common/client -run TestGetFeatureFlagsFromHeader` and `go test -v ./common/client -run TestGetFeatureFlagsFromHeader_RoundTrip` and `go test -v ./host -run TestSignalWorkflow`. Fill [Potential risks] documenting IDL update compatibility, JSON format compatibility for rolling upgrades, and generated code changes. Mark [Documentation Changes] as N/A if certain no docs affected, or document SDK docs needs.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}