Skip to content

Type WorkflowSuspendedEvent.SuspensionType as enum#554

Closed
eanzhao wants to merge 2 commits into
devfrom
refactor/2026-05-01_workflow-suspension-type-enum
Closed

Type WorkflowSuspendedEvent.SuspensionType as enum#554
eanzhao wants to merge 2 commits into
devfrom
refactor/2026-05-01_workflow-suspension-type-enum

Conversation

@eanzhao

@eanzhao eanzhao commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add WorkflowSuspensionType proto enum (HUMAN_APPROVAL / HUMAN_INPUT / SECURE_INPUT) to workflow_execution_messages.proto; reserve old field 3, type the new field at 10.
  • Add repeated string expected_options = 11 so the suspension-emitting actor declares its expected response options at emission time.
  • Modules (HumanApprovalModule / HumanInputModule / SecureInputModule) now emit the typed enum and seed default options via a single WorkflowSuspensionTypes helper.
  • WorkflowHumanInteractionProjector no longer string-switches on suspension type; it copies evt.ExpectedOptions and falls back to the helper for hand-built events.
  • Downstream protos (workflow_run_events, agui_events, workflow_projection_transport) keep their string suspension_type for AGUI/frontend wire stability — the AGUI mapper translates enum → conventional snake-case via ToWireName() at the boundary.

Implements rollout step 1 of #553 (finding 5: typed SuspensionType enum). Per the issue's Suggested rollout, this lands first as a mechanical change to prevent regressions before tackling the typed WorkflowSuspendedDeliveryTarget (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.
  • New test ProjectAsync_ShouldUseEventExpectedOptions_OverDefaults verifies actor-supplied options override the helper default.
  • playground_asset_drift_guard.sh skipped (pnpm/tsc not installed in this paseo worktree; baseline also fails — not introduced by this change).

🤖 Generated with Claude Code

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>

@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: 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.09524% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.76%. Comparing base (0193bc1) to head (496251c).

Files with missing lines Patch % Lines
...r.Workflow.Abstractions/WorkflowSuspensionTypes.cs 64.28% 3 Missing and 2 partials ⚠️
@@            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     
Flag Coverage Δ
ci 71.76% <88.09%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
...vatar.Workflow.Core/Modules/HumanApprovalModule.cs 88.95% <100.00%> (+0.06%) ⬆️
.../Aevatar.Workflow.Core/Modules/HumanInputModule.cs 86.32% <100.00%> (+0.11%) ⬆️
...Aevatar.Workflow.Core/Modules/SecureInputModule.cs 72.37% <100.00%> (+0.15%) ⬆️
...UIAdapter/EventEnvelopeToWorkflowRunEventMapper.cs 88.68% <100.00%> (+0.03%) ⬆️
...n.AGUIAdapter/WorkflowHumanInteractionProjector.cs 94.59% <100.00%> (+11.66%) ⬆️
...WorkflowExecutionArtifactMaterializationSupport.cs 90.00% <100.00%> (+0.22%) ⬆️
...r.Workflow.Abstractions/WorkflowSuspensionTypes.cs 64.28% <64.28%> (ø)

... and 3 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.

eanzhao added a commit that referenced this pull request Jun 2, 2026
将 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>
eanzhao added a commit that referenced this pull request Jun 2, 2026
… 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>
@eanzhao

eanzhao commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

在 feature/integrate 集成(#1698)同步到当前 dev 时,本 PR 被 dev 超越而弃用、关闭。

dev 在本 PR 之后对 WorkflowSuspendedEvent.suspension_type 采用了不同设计:保留 string suspension_type + 新增强类型 WorkflowToolApprovalSuspension tool_approval 子消息(#1668),而非本 PR 的 WorkflowSuspensionType 枚举。两者在 proto 字段与映射上直接冲突;按 maintainer 决策采用 dev 的 typed-sub-message 方向。

如仍需枚举化方向,可在 dev 当前模型上重新提。

@eanzhao eanzhao closed this Jun 2, 2026
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