Skip to content

Issue #3252: [Backend] Workflow run durable lineage contract - #3334

Open
louis4li wants to merge 2 commits into
feat/2026-08-07_issue-3251from
feat/2026-08-07_issue-3252
Open

Issue #3252: [Backend] Workflow run durable lineage contract#3334
louis4li wants to merge 2 commits into
feat/2026-08-07_issue-3251from
feat/2026-08-07_issue-3252

Conversation

@louis4li

@louis4li louis4li commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Issue

Closes #3252 — [Backend] Workflow run durable lineage contract

Implementation summary

See .implement-loop/runs/implement-issue-3252.md.

Stacked-PR position

🤖 Generated by codex-implement-loop. Reviewer is a Claude subagent (see PR comments for round-N review reports).

Add typed committed lineage facts for retry, fork, and sub-workflow relationships and project them into run activity/read-model contracts.

Closes #3252

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@louis4li
louis4li requested a review from jason-aelf as a code owner August 7, 2026 10:56

@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: 04b7817ac7

ℹ️ 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".

// Implement (issue #3252):
// Behavior: child workflow runs expose their parent/root run lineage as typed bind facts.
// Why this shape: the child actor commits lineage from the call-site handoff instead of deriving it from runtime topology.
InitialLineage = BuildChildInitialLineage(pending, _ownerActorIdAccessor()),

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 Forward child lineage through the bind handler

When a workflow_call binds its child, this field is placed on the incoming BindWorkflowRunDefinitionEvent, but HandleBindWorkflowRunDefinition forwards the request into BindWorkflowRunDefinitionAsync, whose signature and newly constructed committed bind event omit InitialLineage. Consequently every sub-workflow child commits an unavailable lineage instead of its parent/root relationship; pass the typed lineage through the handler into the committed event.

AGENTS.md reference: AGENTS.md:L74-L74

Useful? React with 👍 / 👎.

next.TerminalNotificationAttempt = 0;
next.TerminalNotificationDeliveryStatus = WorkflowRunTerminalNotificationDeliveryStatus.Unspecified;
next.TerminalNotificationRetryCallbackId = string.Empty;
next.Lineage = evt.InitialLineage?.Clone() ?? CreateUnavailableLineage("Run lineage is unavailable for this run.");

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 Preserve lineage during dynamic definition replacement

For a forked or sub-workflow run that executes ReplaceWorkflowDefinitionAndExecuteEvent, ReplaceWorkflowDefinitionBypassingBindingAsync emits a bind event without InitialLineage; this fallback therefore replaces the already committed ancestry with an unavailable lineage, and the following execution-start event retains that loss. Preserve current.Lineage when a rebind omits an explicit replacement so changing the workflow definition cannot erase the run's durable lineage.

AGENTS.md reference: AGENTS.md:L74-L74

Useful? React with 👍 / 👎.

Comment on lines +156 to +158
lineage?.Clone() ?? new WorkflowRunLineage
{
Availability = WorkflowRunLineageAvailability.Unavailable,

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 Classify legacy actor states as legacy-unavailable

When an actor state committed before the new protobuf field is projected after deployment, state.Lineage is null and this fallback writes ordinary Unavailable into the current-state document. Because the query mapper only emits LegacyUnavailable when the document field itself is absent, reprojecting or advancing such an old run permanently misclassifies it as a new run with explicitly unavailable lineage; the projector's null-state fallback should retain the legacy-unavailable distinction.

AGENTS.md reference: AGENTS.md:L85-L85

Useful? React with 👍 / 👎.

@louis4li

louis4li commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Review of PR #3334 - round 1

Verdict: rework
Issue: #3252 - [Backend] Workflow run durable lineage contract
Head: feat/2026-08-07_issue-3252 @ 04b7817
Base: feat/2026-08-07_issue-3251
Reviewed by: Claude subagent (codex-implement-loop)

Verdict rationale

The PR adds a typed WorkflowRunLineage contract, stores it on run state, projects it into current-state read models, exposes it through Observatory models, and keeps retry/fork versus sub-workflow relations as distinct typed sub-messages. However, the child side of sub-workflow lineage is not actually committed by the child run actor: the sub-workflow handoff stamps BindWorkflowRunDefinitionEvent.InitialLineage, but WorkflowRunGAgent.HandleBindWorkflowRunDefinition forwards the request through BindWorkflowRunDefinitionAsync, whose persisted replacement event does not carry InitialLineage. That fails the issue requirement that parent/child sub-workflow lineage be durable and committed by the authoritative actor/event owner, and it also leaves a test gap around the actual child bind path.

Findings

F1 - Sub-workflow child lineage is dropped before the child run commits its bind event

  • Severity: blocking
  • Dimension: issue-intent
  • Location: src/workflow/Aevatar.Workflow.Core/WorkflowRunGAgent.cs:416-462, src/workflow/Aevatar.Workflow.Core/WorkflowRunGAgent.cs:477-491, src/workflow/Aevatar.Workflow.Core/Primitives/SubWorkflowOrchestrator.cs:1509-1515
  • Evidence: PR diff stamps child lineage into the bind envelope:
                ExpectedExecutionMode = state.ExpectedExecutionMode,
  •        // Implement (issue #3252):
    
  •        //   Behavior: child workflow runs expose their parent/root run lineage as typed bind facts.
    
  •        //   Why this shape: the child actor commits lineage from the call-site handoff instead of deriving it from runtime topology.
    
  •        InitialLineage = BuildChildInitialLineage(pending, _ownerActorIdAccessor()),
        };
    
    PR diff then relies on the persisted bind event to apply that field:
    ```diff
    
  •    next.Lineage = evt.InitialLineage?.Clone() ?? CreateUnavailableLineage("Run lineage is unavailable for this run.");
    
    But the child actor handler currently drops it before persistence:
    ```csharp
    public Task HandleBindWorkflowRunDefinition(BindWorkflowRunDefinitionEvent request) =>
        BindWorkflowRunDefinitionAsync(
            request.DefinitionActorId,
            request.WorkflowYaml,
            request.WorkflowName,
            request.InlineWorkflowYamls,
            request.RunId,
            request.ScopeId,
            request.RunOrigin,
            request.ScheduleId,
            request.WorkflowId,
            request.RevisionId,
            request.DefinitionVersion,
            request.CapabilityAdmissionPlan,
            request.ExpectedExecutionMode);
    
    and BindWorkflowRunDefinitionAsync builds the committed event without assigning InitialLineage:
    var bindDefinitionEvent = new BindWorkflowRunDefinitionEvent
    {
        DefinitionActorId = definitionActorId ?? string.Empty,
        WorkflowName = workflowName ?? string.Empty,
        WorkflowYaml = workflowYaml ?? string.Empty,
        RunId = string.IsNullOrWhiteSpace(runId) ? Id : WorkflowRunIdNormalizer.Normalize(runId),
        ScopeId = scopeId?.Trim() ?? string.Empty,
        RunOrigin = runOrigin?.Trim() ?? string.Empty,
        ScheduleId = scheduleId?.Trim() ?? string.Empty,
        WorkflowId = workflowId?.Trim() ?? string.Empty,
        RevisionId = revisionId?.Trim() ?? string.Empty,
        DefinitionVersion = Math.Max(0, definitionVersion),
        CapabilityAdmissionPlan = capabilityAdmissionPlan?.Clone(),
        ExpectedExecutionMode = expectedExecutionMode,
    };
  • Why it's a problem: Issue [Backend] Workflow run durable lineage contract #3252 requires durable typed lineage for parent and child runs and says lineage must be "committed by the authoritative actor/event owner and projected into read models; no query-time reconstruction from event replay or actor runtime topology." CLAUDE.md also requires "单一权威拥有者" and "projection 只消费 committed 事实". Because the child run actor never commits the stamped InitialLineage, child-to-parent sub-workflow navigation falls back to unavailable lineage even though the parent handoff had the typed facts.
  • What would change your verdict: Preserve request.InitialLineage through the authoritative bind commit, for example by extending BindWorkflowRunDefinitionAsync with an optional WorkflowRunLineage? initialLineage parameter and assigning it to the persisted BindWorkflowRunDefinitionEvent, then add a test that sends a BindWorkflowRunDefinitionEvent with deliberately distinct parent/root/child run IDs through HandleBindWorkflowRunDefinition and verifies the child run state/read-model lineage contains the parent sub-workflow relation.

What's good

  • The proto shape is additive and typed: retry/fork lineage and sub-workflow lineage are separate sub-messages, with explicit Unavailable and LegacyUnavailable availability values.
  • The read side maps lineage through the projection/current-state path into Observatory DTOs instead of adding query-time replay or actor-topology reconstruction.
  • The three SCOPE_EXTEND test call-site edits are mechanical bind-contract adaptations, and the PR diff does not include unrelated stacked commits from feat/2026-08-07_issue-3251.

REVIEW_VERDICT:rework:sub-workflow child lineage is dropped before authoritative commit

Preserve sub-workflow child lineage through the authoritative bind commit path.

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

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.65605% with 89 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feat/2026-08-07_issue-3251@33e1335). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...orkflow/Aevatar.Workflow.Core/WorkflowRunGAgent.cs 51.92% 66 Missing and 9 partials ⚠️
...Application/RunForks/WorkflowRunForkCoordinator.cs 85.00% 2 Missing and 4 partials ⚠️
....Application.Abstractions/Runs/WorkflowRunPorts.cs 50.00% 2 Missing ⚠️
...orkflow.Core/Primitives/SubWorkflowOrchestrator.cs 94.87% 0 Missing and 2 partials ⚠️
...n/ReadModels/WorkflowRunForkSeedReadModelMapper.cs 75.00% 0 Missing and 2 partials ⚠️
...n/RunForks/WorkflowForkRunCommandTargetResolver.cs 85.71% 0 Missing and 1 partial ⚠️
...ojectors/WorkflowExecutionCurrentStateProjector.cs 92.85% 0 Missing and 1 partial ⚠️
@@                      Coverage Diff                      @@
##             feat/2026-08-07_issue-3251    #3334   +/-   ##
=============================================================
  Coverage                              ?   84.45%           
=============================================================
  Files                                 ?     1526           
  Lines                                 ?   146424           
  Branches                              ?    17855           
=============================================================
  Hits                                  ?   123664           
  Misses                                ?    14570           
  Partials                              ?     8190           
Flag Coverage Δ
ci 84.45% <71.65%> (?)

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

Files with missing lines Coverage Δ
...Observatory/IWorkflowRunObservatoryQueryService.cs 100.00% <100.00%> (ø)
...ion.Abstractions/RunForks/WorkflowForkRunModels.cs 90.41% <100.00%> (ø)
...ication.Abstractions/Runs/WorkflowChatRunModels.cs 96.66% <100.00%> (ø)
.../Observatory/WorkflowRunObservatoryQueryService.cs 90.51% <100.00%> (ø)
.../RunForks/WorkflowForkRunAcceptedReceiptFactory.cs 100.00% <100.00%> (ø)
...plication/RunForks/WorkflowForkRunCommandTarget.cs 87.87% <100.00%> (ø)
...ication/Runs/WorkflowChatRequestEnvelopeFactory.cs 86.00% <100.00%> (ø)
...low.Core/WorkflowRunGAgent.IdentityProvisioning.cs 76.00% <100.00%> (ø)
...ion/ReadModels/WorkflowExecutionReadModelMapper.cs 90.90% <100.00%> (ø)
...n/RunForks/WorkflowForkRunCommandTargetResolver.cs 92.46% <85.71%> (ø)
... and 6 more
🚀 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

louis4li commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Review of PR #3334 — round 2

Verdict: pass
Issue: #3252 — [Backend] Workflow run durable lineage contract
Head: feat/2026-08-07_issue-3252 @ ebacfa6
Base: feat/2026-08-07_issue-3251
Reviewed by: Claude subagent (codex-implement-loop)

Verdict rationale

The PR now satisfies issue #3252's durable lineage requirements against the diff: lineage is modeled as additive typed protobuf fields, committed by WorkflowRunGAgent state transitions, projected into current-state read models, and exposed through Activity/Observatory query DTOs without route, graph, actor-id, workflow-name, or string-prefix inference. Round-1 F1 is resolved because BindWorkflowRunDefinitionAsync now accepts optional WorkflowRunLineage, clones it into the authoritative BindWorkflowRunDefinitionEvent.InitialLineage, and HandleBindWorkflowRunDefinition passes the stamped request lineage through that commit path. Retry/fork lineage remains separate from sub-workflow lineage, legacy/missing read-model lineage maps to explicit LegacyUnavailable, and the touched proto fields use additive field numbers.

Findings

No blocking findings.

What's good

  • Round-1 F1 has a direct regression test covering HandleBindWorkflowRunDefinition with distinct parent/root/child run IDs and assertions on both the committed bind event and child actor state.
  • Retry/fork and sub-workflow lineage are separate typed sub-messages, with public run_id values kept distinct from optional actor addresses.
  • Read-side coverage verifies typed lineage projection plus legacy-unavailable fallback, and the implementation summary records passing build, targeted tests, architecture guard, test stability guard, and diff check.

Round comparison

  • Findings carried over from round 1: F1 is resolved.
  • New findings this round: none.
  • Net direction: improving

REVIEW_VERDICT:pass:durable typed lineage contract is preserved through bind and projection

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