Skip to content

fix(workflow): keep a newer local edit over a save's response - #8540

Draft
yangzhang75 wants to merge 1 commit into
apache:mainfrom
yangzhang75:fix/stale-save-response
Draft

fix(workflow): keep a newer local edit over a save's response#8540
yangzhang75 wants to merge 1 commit into
apache:mainfrom
yangzhang75:fix/stale-save-response

Conversation

@yangzhang75

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #8536, the item deferred from #8456 on review.

Every save's response is fed back as the workflow's metadata (the canvas autosave in workspace.component.ts, the menu's own save for a rename / description / revert, the Form View switch's save). Saves go out one at a time, so a response can land after a newer local edit and put the old name or description back: rename while an autosave is out and the title flips back until the rename's own save answers; rename while the Form View switch's save is out and the rename is lost, since the switch left as soon as its own save had completed and the page load aborted the rename's.

Handled once, in WorkflowPersistService, the one place every save goes through:

  • Each response is relayed with the page's current name and description in place of the ones the save was sent with. Those are the two fields a user edits; everything else in the response (id, timestamps, publish state, default view) is the server's and arrives as before. A response is left alone when another workflow is open by the time it answers, and the local name is kept for a workflow the save has just created (the page still holds the default id). The action service is looked up lazily at response time, so the dashboard, which also uses this service (retrieve, create, duplicate), does not construct the graph-owning service as a side effect. The callers that feed a response back as metadata (the workspace autosave, the menu's own save, the Form View switch, the form's save) are unchanged by this part; the settings panel's save and the workspace's unload save never read the response.
  • whenSavesDrained() emits once every save asked for so far has answered or failed (at once when none is pending). The Form View switch waits for it before leaving, so a save queued behind its own (a rename's, a description's, which save through the menu itself and do not go through workflowChanged) lands before the page unloads. A queued save that fails reports its error through its own caller and does not hold the hand-over (before this change it was simply aborted by the page load).

Verified in a real browser against a dev server, with every persist response held back 1.5 s: before, a rename made while an earlier rename's save was out flipped the title back to the old name when that response landed, and a rename made during the Form View switch's save was lost (the page left after 1.9 s with the old name stored); after, the title stays on the new name, and the switch leaves once the rename's save has landed, with the new name stored.

Any related issues, documentation, discussions?

Closes #8536. Follow-up to #8456 (threads on menu.component.ts:692 and :231); part of the Form View feature (parent issue #8011).

How was this PR tested?

Unit tests (vitest): the persist service relays a response with the page's current name and description and the server's other fields, keeps the local name for a just-created workflow, leaves a response alone when another workflow is open; whenSavesDrained emits at once when idle, only after the last of two queued saves has answered, and after a failed save; the menu's switch leaves only once the queue has drained. Each new guard was deletion-checked (removing it turns the corresponding test red). eslint, prettier and the production (AOT) build pass; every changed line is statement and function covered.

Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY

Every save's response is fed back as the workflow's metadata: the canvas
autosave, the menu's own save (rename, description, revert) and the Form View
switch's save. Saves go out one at a time, so a response can land after a newer
local edit and put the old name or description back. Rename while an autosave is
out and the title flips back until the rename's own save answers; rename while
the Form View switch's save is out and the rename is lost, since the switch left
as soon as its own save had completed and the page load aborted the rename's.

WorkflowPersistService, the one place every save goes through, now relays each
response with the page's current name and description in place of the ones the
save was sent with (left alone when another workflow is open by then; kept for a
workflow the save has just created, which still carries the default id locally),
and exposes whenSavesDrained(), which the Form View switch waits for before
leaving, so a save queued behind its own lands before the page unloads.

Closes apache#8536.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
@Yicong-Huang Yicong-Huang added the release/v1.3 back porting to release/v1.3 label Sep 14, 2026
@github-actions github-actions Bot added fix frontend Changes related to the frontend GUI labels Sep 14, 2026
@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Sep 14, 2026
@github-actions
github-actions Bot requested a review from mengw15 September 14, 2026 02:48
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@github-actions
github-actions Bot requested a review from xuang7 September 14, 2026 02:48
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch Analysis
release/v1.3 Change detected on this branch — label added; this fix is queued to backport here. @mengw15 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other. Review requested.
release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. @xuang7 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other. Review requested.

Auto-label run.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.91%. Comparing base (5042d96) to head (672761d).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8540   +/-   ##
=========================================
  Coverage     95.90%   95.91%           
  Complexity      459      459           
=========================================
  Files           589      589           
  Lines         33953    33969   +16     
  Branches       4246     4249    +3     
=========================================
+ Hits          32564    32581   +17     
  Misses          811      811           
+ Partials        578      577    -1     
Flag Coverage Δ *Carryforward flag
access-control-service 80.18% <ø> (ø) Carriedforward from 5042d96
agent-service 99.32% <ø> (ø) Carriedforward from 5042d96
amber 90.82% <ø> (ø) Carriedforward from 5042d96
computing-unit-managing-service 77.14% <ø> (ø) Carriedforward from 5042d96
config-service 87.12% <ø> (ø) Carriedforward from 5042d96
file-service 83.65% <ø> (ø) Carriedforward from 5042d96
frontend 96.72% <100.00%> (+<0.01%) ⬆️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 5042d96
pyamber 98.47% <ø> (ø) Carriedforward from 5042d96
workflow-compiling-service 74.09% <ø> (ø) Carriedforward from 5042d96

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mengw15 mengw15 removed release/v1.2 back porting to release/v1.2 release/v1.3 back porting to release/v1.3 labels Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow): a save's response must not overwrite a newer local edit (WorkflowPersistService)

4 participants