Skip to content

Add workflow chat descriptor set#2383

Open
louis4li wants to merge 5 commits into
feature/integratefrom
fix/2026-06-26_workflow-descriptor-set
Open

Add workflow chat descriptor set#2383
louis4li wants to merge 5 commits into
feature/integratefrom
fix/2026-06-26_workflow-descriptor-set

Conversation

@louis4li

Copy link
Copy Markdown
Contributor

Summary

  • Populate workflow service revision artifacts with chat request/response protobuf descriptors.
  • Include dependency descriptors in build order so payloadJson packing can resolve workflow chat payload types.
  • Add regression coverage for workflow adapter descriptor generation.

Test plan

  • dotnet test test/Aevatar.GAgentService.Tests/Aevatar.GAgentService.Tests.csproj --nologo --no-restore --filter ServiceImplementationAdaptersTests -v quiet
  • bash tools/ci/test_stability_guards.sh
  • Local Mainnet API smoke: created workflow service revision and schedule with payloadJson for type.googleapis.com/aevatar.ai.ChatRequestEvent; schedule creation returned 202 and readback showed scheduleKind: 1.

🤖 Generated with Claude Code

Populate workflow revision artifacts with chat protobuf descriptors so payloadJson requests can be packed for workflow service schedules.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7816184e3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +50 to +52
ProtocolDescriptorSet = BuildProtocolDescriptorSet(
ChatRequestEvent.Descriptor,
ChatResponseEvent.Descriptor),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include descriptors in workflow replay hash

When a workflow revision is assembled, ArtifactHash is computed from the full prepared artifact, so this new ProtocolDescriptorSet changes the stored hash. The existing-revision replay path recomputes the expected workflow hash via ScopeBindingCommandApplicationService.BuildWorkflowArtifact, but that mirror artifact still omits the descriptor set; after creating a workflow revision, a retry/upsert with AllowExistingRevisionReplay for the same revision will compare the stored hash including these bytes with an expected hash without them and reject the identical revision as a different artifact.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 226aebf by moving descriptor-set construction into a shared ServiceProtocolDescriptorSetBuilder and using it from both WorkflowServiceImplementationAdapter and ScopeBindingCommandApplicationService.BuildWorkflowArtifact, so replay hash calculation includes the same ProtocolDescriptorSet bytes as prepared revisions. Also updated scope binding replay tests to compute the descriptor-inclusive workflow artifact hash.

Use a shared descriptor set builder for workflow artifacts so scope binding replay compares the same artifact hash that revision preparation stores.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.94737% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.75%. Comparing base (90ea572) to head (c781618).
⚠️ Report is 356 commits behind head on feature/integrate.

Files with missing lines Patch % Lines
...e/Adapters/WorkflowServiceImplementationAdapter.cs 78.94% 6 Missing and 2 partials ⚠️
@@                  Coverage Diff                  @@
##           feature/integrate    #2383      +/-   ##
=====================================================
- Coverage              84.08%   83.75%   -0.33%     
=====================================================
  Files                   1139     1231      +92     
  Lines                  82320    94468   +12148     
  Branches               10705    12358    +1653     
=====================================================
+ Hits                   69219    79125    +9906     
- Misses                  8414     9857    +1443     
- Partials                4687     5486     +799     
Flag Coverage Δ
ci 83.75% <78.94%> (-0.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...e/Adapters/WorkflowServiceImplementationAdapter.cs 87.77% <78.94%> (-6.46%) ⬇️

... and 256 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

louis4li and others added 3 commits June 26, 2026 14:23
Use a well-known descriptor registry keyed by FileDescriptor name instead of branching on proto dependency strings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Align the Studio test helper with the current ServiceRunSnapshot shape so coverage-quality can build after rebasing on feature/integrate.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant