Type WorkflowSuspendedEvent.SuspensionType as enum#554
Conversation
Replaces magic-string suspension_type on WorkflowSuspendedEvent with a proto enum and adds expected_options to the event itself, eliminating the string-compare switch in WorkflowHumanInteractionProjector. Modules emit the typed enum and seed default options via a single helper. Issue #553 finding 5 (rollout step 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b66b3d995
ℹ️ 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".
| } | ||
| message WorkflowSuspendedEvent | ||
| { | ||
| reserved 3; |
There was a problem hiding this comment.
Keep legacy suspension_type tag readable during migration
This schema change makes WorkflowSuspendedEvent stop reading the previously persisted suspension_type at tag 3 (string) and only read the new enum at tag 10, so replaying any pre-upgrade events will deserialize as WORKFLOW_SUSPENSION_TYPE_UNSPECIFIED. Downstream code now converts that enum via ToWireName() to an empty string, which means historical/in-flight suspended runs can lose their suspension semantics (type/options) when projected or delivered to AGUI/human-interaction flows. Please keep a compatibility read path (e.g., retain old field as deprecated and map it to enum) until old events are migrated/backfilled.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## dev #554 +/- ##
==========================================
+ Coverage 71.75% 71.76% +0.01%
==========================================
Files 1251 1252 +1
Lines 90228 90244 +16
Branches 11801 11802 +1
==========================================
+ Hits 64746 64768 +22
+ Misses 20862 20859 -3
+ Partials 4620 4617 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
将 eanzhao PR #554 合入 feature/integrate。 冲突解决: - workflow_execution_messages.proto: dev 的 secure(10)/redacted_output(11) 与 PR 的 suspension_type/expected_options 字段号冲突;保留 dev 已部署的 10/11,PR 新字段移至 suspension_type=12 / expected_options=13;slot 3 保持 reserved(旧字符串 suspension_type 已移除)。 - EventEnvelopeToWorkflowRunEventMapper.cs: 合并 dev 的 secure-input 处理(Secure/RedactedOutput/ resolved variableName/filtered metadata) 与 PR 的 enum→ToWireName + Options。 - WorkflowHumanInteractionProjector.cs: 保留 dev 的 BuildAnnotations,删除被 enum 取代的死方法 ResolveOptions。 - demos/Aevatar.Demos.Workflow.Web/Program.cs: dev 已删除该 demo,保持删除。 - 三个 Host.Api 测试 + 一个集成测试:WorkflowSuspendedEvent.SuspensionType 字符串字面量改为枚举值。 验证:build 0 error;Host.Api mapper/projector 47 测试通过;SecureInputModule 集成测试通过。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… superseded dev advanced 41 commits since feature/integrate branched, including its own parallel refactors that supersede two pieces of this branch's work: - #1688 (constrain ToLegacyMetadata): dev removed ToLegacyMetadata / CurrentMetadata / LegacyMetadata entirely (issues #1621 FromMetadata-external-only + #1574 metadata-scrub-only). #1688's goal is already achieved by dev; dropped (took dev's versions of AgentToolExecutionContext / AgentToolRequestContext / AIGAgentBase / mapper test / guard). - #554 (suspension enum): dev chose a different design — kept suspension_type as string and added typed WorkflowToolApprovalSuspension tool_approval (issue #1668). Dropped #554's enum (took dev's proto + workflow suspension files; removed dead WorkflowSuspensionTypes.cs). Kept + reconciled onto current dev: PR #1691, and issues #1685 / #1686 / #1687 / #1695. Reconciliation fixes: LocalSkillCatalogTests uses dev's typed AgentToolRequestContext.Current; SkillWorkflowMountAdapter(+test) updated to dev's flat ScopeWorkflowUpsertResult shape. 验证:dotnet build aevatar.slnx 0 error;architecture + test_stability guards 通过; AI.Tests 763 / Ornn.Tests 41 / GAgentService.Tests 690 / CQRS.Core.Tests 51 / Workflow.Host.Api.Tests 417 / Workflow.Application.Tests 206 / Hosting.Tests 241 全通过。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
在 feature/integrate 集成(#1698)同步到当前 dev 时,本 PR 被 dev 超越而弃用、关闭。 dev 在本 PR 之后对 如仍需枚举化方向,可在 dev 当前模型上重新提。 |
Summary
WorkflowSuspensionTypeproto enum (HUMAN_APPROVAL/HUMAN_INPUT/SECURE_INPUT) toworkflow_execution_messages.proto; reserve old field 3, type the new field at 10.repeated string expected_options = 11so the suspension-emitting actor declares its expected response options at emission time.HumanApprovalModule/HumanInputModule/SecureInputModule) now emit the typed enum and seed default options via a singleWorkflowSuspensionTypeshelper.WorkflowHumanInteractionProjectorno longer string-switches on suspension type; it copiesevt.ExpectedOptionsand falls back to the helper for hand-built events.workflow_run_events,agui_events,workflow_projection_transport) keep theirstring suspension_typefor AGUI/frontend wire stability — the AGUI mapper translates enum → conventional snake-case viaToWireName()at the boundary.Implements rollout step 1 of #553 (finding 5: typed
SuspensionTypeenum). Per the issue'sSuggested rollout, this lands first as a mechanical change to prevent regressions before tackling the typedWorkflowSuspendedDeliveryTarget(finding 2) and the rest.Test plan
dotnet build aevatar.slnx— clean (0 errors).dotnet test test/Aevatar.Workflow.Host.Api.Tests— 325 passed.dotnet test test/Aevatar.Integration.Tests— 286 passed (4 pre-existing skipped).dotnet test test/Aevatar.GAgentService.Integration.Tests— 286 passed.bash tools/ci/test_stability_guards.sh— pass.bash tools/ci/workflow_binding_boundary_guard.sh/projection_state_version_guard.sh/projection_route_mapping_guard.sh— pass.ProjectAsync_ShouldUseEventExpectedOptions_OverDefaultsverifies actor-supplied options override the helper default.playground_asset_drift_guard.shskipped (pnpm/tsc not installed in this paseo worktree; baseline also fails — not introduced by this change).🤖 Generated with Claude Code