dohertj2/mxaccessgw
1
0
Fork 0
Code Issues 14 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(IPC-27): close descriptor-freshness blind spots for enums, services, and Galaxy

Browse Source 
ClientProtoInputTests.Descriptor_ContainsEveryContractMessageAndField only
compared messages and fields, and only enumerated the gateway/worker
descriptors, so a new enum value, a new RPC, or any galaxy_repository.proto-only
change would not redden the test even though it is documented as the primary
protoc-free CI gate.

Rename to Descriptor_ContainsEveryContractSymbol and extend the reflection walk
on both sides (published protoset and in-process contract) to also collect
enums/enum values ({enumFullName}, {enumFullName}/{valueName}) and
services/methods ({serviceFullName}, {serviceFullName}/{methodName}), and add
GalaxyRepositoryReflection.Descriptor to the enumerated files. The comparison
stays a flat, order-insensitive string-set diff with no protoc dependency.

Update docs/ClientProtoGeneration.md and docs/Contracts.md prose from
"message or field" to the full symbol coverage.

Red-path proof: pointed the test at the pre-IPC-01 stale protoset and confirmed
it failed naming max_frame_bytes, several MxCommandKind/AlarmProviderMode enum
values, MxAccessGateway/StreamAlarms and GalaxyRepository/BrowseChildren, and
the galaxy_repository.v1.* surface; restored the real path and re-ran green.

Flips IPC-27 to Done in the 2026-07-12 remediation tracker and register.
This commit is contained in:
Joseph Doherty
2026-08-07 07:47:18 -04:00
parent 0646c73e48
commit 6060d21995
5 changed files with 119 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/2026-07-12/remediation/00-tracking.md
+2 -1
View File
File diff suppressed because one or more lines are too long
archreview/2026-07-12/remediation/30-contracts-ipc.md
+1 -1
View File
@@ -16,7 +16,7 @@ All `path:line` citations were re-verified against the working tree at `4f5371f`
| IPC-24 | Medium | P0 | S | — | Done | CI's unconditional Java churn-revert masks real generated-code drift for message-level proto changes |
| IPC-25 | Medium | P0 | M | — | Done | Committed Go/Python worker bindings are stale at HEAD; no guard covers them |
| IPC-26 | Low | P2 | S¹ | WRK-22 | Not started | Cancelled write leaves a ghost frame that is still written (contract requirement here; fix mechanics in WRK-22) |
| IPC-27 | Low | P2 | S | — | Not started | Descriptor freshness test blind to enums, enum values, services/methods, and the Galaxy contract |
| IPC-27 | Low | P2 | S | — | Done | Descriptor freshness test blind to enums, enum values, services/methods, and the Galaxy contract |
| IPC-28 | Low | — | S | — | Done | `docs/Grpc.md` omits the `CommandTooLarge` → `ResourceExhausted` mapping |
| IPC-29 | Low | — | S | — | Done (discharged by WRK-26) | Worker writer priority scheduling and write-time sequence stamping undocumented in the frame-protocol doc |
| IPC-30 | Low | P0 | M | WRK-21 (same file/batch) | Done | Oversized worker→gateway event frame is session-fatal — make the death deliberate, structured, and diagnosable |
docs/ClientProtoGeneration.md
+6 -4
View File
@@ -74,10 +74,12 @@ protoc-version encoding drift and does not false-fail across protoc releases
(it warns, rather than fails, when protoc is off the pin).
The gateway test project carries an independent, protoc-free freshness guard:
`ClientProtoInputTests.Descriptor_ContainsEveryContractMessageAndField` reflects
over the in-process contract descriptors and fails if any contract message or
field is missing from the committed protoset. This is the primary CI gate for
descriptor staleness; a red test means "regenerate and commit the protoset."
`ClientProtoInputTests.Descriptor_ContainsEveryContractSymbol` reflects over the
in-process contract descriptors — `mxaccess_gateway.proto`, `mxaccess_worker.proto`,
and `galaxy_repository.proto` — and fails if any contract message, field, enum,
enum value, service, or method is missing from the committed protoset. This is
the primary CI gate for descriptor staleness; a red test means "regenerate and
commit the protoset."
### Pinned generator versions
docs/Contracts.md
+5 -4
View File
@@ -156,10 +156,11 @@ pwsh -File scripts/publish-client-proto-inputs.ps1
Freshness is guarded two ways so a skipped regeneration cannot ship silently:
- `ClientProtoInputTests.Descriptor_ContainsEveryContractMessageAndField` (gateway test project)
reflects over the in-process contract descriptors and fails if any message or field is missing
from the committed protoset. It is semantic (symbol presence), needs no protoc, and runs in the
Linux CI. A red test means "regenerate and commit the protoset."
- `ClientProtoInputTests.Descriptor_ContainsEveryContractSymbol` (gateway test project)
reflects over the in-process contract descriptors — including `galaxy_repository.proto` — and
fails if any message, field, enum, enum value, service, or method is missing from the committed
protoset. It is semantic (symbol presence), needs no protoc, and runs in the Linux CI. A red
test means "regenerate and commit the protoset."
- `pwsh -File scripts/publish-client-proto-inputs.ps1 -Check` rebuilds the descriptor and compares
it to the committed one. The comparison normalizes both sides through the same protoc with
`source_code_info` stripped, so it does not false-fail across protoc releases.
src/ZB.MOM.WW.MxGateway.Tests/Contracts/ClientProtoInputTests.cs
+105 -12
View File
@@ -3,22 +3,24 @@ using Google.Protobuf;
using Google.Protobuf.Reflection;
using ZB.MOM.WW.MxGateway.Contracts;
using ZB.MOM.WW.MxGateway.Contracts.Proto;
using ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy;
namespace ZB.MOM.WW.MxGateway.Tests.Contracts;
public sealed class ClientProtoInputTests
{
/// <summary>
/// Guards the published client descriptor set against silent staleness. Every message
/// and field compiled into the in-process contract (which the build regenerates from the current
/// <c>.proto</c> sources) must appear in the committed protoset. A missing symbol means the
/// descriptor was not regenerated after a proto change; run
/// <c>scripts/publish-client-proto-inputs.ps1</c> and commit the refreshed protoset.
/// Guards the published client descriptor set against silent staleness. Every message,
/// field, enum, enum value, service, and method compiled into the in-process contract
/// (which the build regenerates from the current <c>.proto</c> sources) must appear in the
/// committed protoset. A missing symbol means the descriptor was not regenerated after a
/// proto change; run <c>scripts/publish-client-proto-inputs.ps1</c> and commit the
/// refreshed protoset.
/// The check is semantic (symbol presence) rather than byte-wise, so it is independent of protoc
/// version and does not require protoc on the test runner.
/// </summary>
[Fact]
public void Descriptor_ContainsEveryContractMessageAndField()
public void Descriptor_ContainsEveryContractSymbol()
{
DirectoryInfo repositoryRoot = FindRepositoryRoot();
string descriptorPath = Path.Combine(
@@ -34,20 +36,65 @@ public sealed class ClientProtoInputTests
HashSet<string> publishedMessages = new(StringComparer.Ordinal);
HashSet<string> publishedFields = new(StringComparer.Ordinal);
HashSet<string> publishedEnums = new(StringComparer.Ordinal);
HashSet<string> publishedServices = new(StringComparer.Ordinal);
foreach (FileDescriptorProto file in descriptorSet.File)
{
foreach (DescriptorProto message in file.MessageType)
{
CollectPublishedSymbols(file.Package, message, publishedMessages, publishedFields);
CollectPublishedSymbols(file.Package, message, publishedMessages, publishedFields, publishedEnums);
}
foreach (EnumDescriptorProto enumType in file.EnumType)
{
CollectPublishedEnumSymbols(file.Package, enumType, publishedEnums);
}
foreach (ServiceDescriptorProto service in file.Service)
{
string serviceFullName = string.IsNullOrEmpty(file.Package) ? service.Name : file.Package + "." + service.Name;
publishedServices.Add(serviceFullName);
foreach (MethodDescriptorProto method in service.Method)
{
publishedServices.Add(serviceFullName + "/" + method.Name);
}
}
}
List<string> missing = [];
foreach (FileDescriptor file in new[] { MxaccessGatewayReflection.Descriptor, MxaccessWorkerReflection.Descriptor })
FileDescriptor[] contractFiles =
[
MxaccessGatewayReflection.Descriptor,
MxaccessWorkerReflection.Descriptor,
GalaxyRepositoryReflection.Descriptor,
];
foreach (FileDescriptor file in contractFiles)
{
foreach (MessageDescriptor message in file.MessageTypes)
{
CollectMissingContractSymbols(message, publishedMessages, publishedFields, missing);
CollectMissingContractSymbols(message, publishedMessages, publishedFields, publishedEnums, missing);
}
foreach (EnumDescriptor enumType in file.EnumTypes)
{
CollectMissingEnumSymbols(enumType, publishedEnums, missing);
}
foreach (ServiceDescriptor service in file.Services)
{
if (!publishedServices.Contains(service.FullName))
{
missing.Add(service.FullName);
}
foreach (MethodDescriptor method in service.Methods)
{
string key = service.FullName + "/" + method.Name;
if (!publishedServices.Contains(key))
{
missing.Add(key);
}
}
}
}
@@ -62,7 +109,8 @@ public sealed class ClientProtoInputTests
string package,
DescriptorProto message,
HashSet<string> messages,
HashSet<string> fields)
HashSet<string> fields,
HashSet<string> enums)
{
string fullName = string.IsNullOrEmpty(package) ? message.Name : package + "." + message.Name;
messages.Add(fullName);
@@ -74,7 +122,26 @@ public sealed class ClientProtoInputTests
foreach (DescriptorProto nested in message.NestedType)
{
CollectPublishedSymbols(fullName, nested, messages, fields);
CollectPublishedSymbols(fullName, nested, messages, fields, enums);
}
foreach (EnumDescriptorProto enumType in message.EnumType)
{
CollectPublishedEnumSymbols(fullName, enumType, enums);
}
}
private static void CollectPublishedEnumSymbols(
string containingScope,
EnumDescriptorProto enumType,
HashSet<string> enums)
{
string enumFullName = string.IsNullOrEmpty(containingScope) ? enumType.Name : containingScope + "." + enumType.Name;
enums.Add(enumFullName);
foreach (EnumValueDescriptorProto value in enumType.Value)
{
enums.Add(enumFullName + "/" + value.Name);
}
}
@@ -82,6 +149,7 @@ public sealed class ClientProtoInputTests
MessageDescriptor message,
HashSet<string> publishedMessages,
HashSet<string> publishedFields,
HashSet<string> publishedEnums,
List<string> missing)
{
if (!publishedMessages.Contains(message.FullName))
@@ -100,7 +168,32 @@ public sealed class ClientProtoInputTests
foreach (MessageDescriptor nested in message.NestedTypes)
{
CollectMissingContractSymbols(nested, publishedMessages, publishedFields, missing);
CollectMissingContractSymbols(nested, publishedMessages, publishedFields, publishedEnums, missing);
}
foreach (EnumDescriptor enumType in message.EnumTypes)
{
CollectMissingEnumSymbols(enumType, publishedEnums, missing);
}
}
private static void CollectMissingEnumSymbols(
EnumDescriptor enumType,
HashSet<string> publishedEnums,
List<string> missing)
{
if (!publishedEnums.Contains(enumType.FullName))
{
missing.Add(enumType.FullName);
}
foreach (EnumValueDescriptor value in enumType.Values)
{
string key = enumType.FullName + "/" + value.Name;
if (!publishedEnums.Contains(key))
{
missing.Add(key);
}
}
}