Skip to content

fix(frontend): fall back to a running unit on workflow load - #8935

Merged
kunwp1 merged 4 commits into
apache:mainfrom
eugenegujing:fix/computing-unit-restore-checks-list
Oct 27, 2026
Merged

kunwp1 merged 4 commits into
apache:mainfrom
eugenegujing:fix/computing-unit-restore-checks-list

Conversation

@eugenegujing

@eugenegujing eugenegujing commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Root cause. When a workflow opens, ComputingUnitSelectionComponent.selectFromLastExecution selects the unit the workflow last ran on without checking that the unit is still listed. /api/executions/{wid}/latest also returns terminated units, so ComputingUnitStatusService.selectComputingUnit waits forever for a cuid that never appears, and the workflow opens with no unit selected even while another unit is Running. The "first Running unit" fallback only ran when the lookup failed, and it read the unit list synchronously, so it could select nothing if the list had not loaded yet.

Before:  open workflow -> last execution's unit was terminated -> wait for it forever  -> no unit selected
After:   open workflow -> last execution's unit is not listed   -> first Running unit  -> ready to run

Both screenshots: the workflow last ran on "Analytics Unit", which was then terminated, and the workflow was reloaded. "My Computing Unit" is Running.

Before After
restore-unit-2-reload-after-terminate-nothing-selected restore-unit-5-after-fix-reload-dropdown-open
No unit selected; the run button reads "Computing Unit". "My Computing Unit" is selected; the run button reads "Run".

Change. Both branches of selectFromLastExecution now go through a new selectFromLoadedUnits, which waits for the first non-empty unit list, as selectInitialUnit already does for a remembered unit. It then selects the last execution's unit if it is listed, else the first Running unit the user has WRITE access to, else nothing. The unit list includes units shared with READ access, and the menu disables Run on those, so falling back to one would select a unit the workflow cannot run on; the fallback skips them. The last execution's unit is not access-filtered: the workflow ran on it before, so it is restored as is. The wait for the loaded list is shared between the remembered-unit branch and the new fallback through a small onceUnitsLoaded helper, which also carries the guard that drops a decision once the workflow has changed underneath it. The warehouse preselect is unchanged.

Not in scope:

  • Whether a unit that is still listed but has turned Failed should be restored. It is selected as before.
  • The check uses the unit list the page already holds. In the workspace that list is refreshed on events such as opening the dropdown, renaming or terminating a unit, or a websocket status change, not on a timer, so a unit terminated in another tab since the last refresh can still be selected.

Any related issues, documentation, discussions?

Closes #8933

The last-execution path dates from #3400. The remembered-unit path, which already checks the list, came with #8456.

How was this PR tested?

New tests in frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts:

  • falls back to a Running unit when the last execution's unit is no longer listed
  • skips a Running unit the user cannot run on when falling back
  • selects nothing when the only Running unit is shared with READ access
  • still selects the last execution's unit when it is listed with READ access
  • selects nothing when the last execution's unit is gone and no unit is Running
  • still selects the last execution's unit when it is listed but not Running
  • waits for the unit list before deciding from the latest execution
  • decides from the latest execution only once, on the first loaded list
  • still falls back to a Running unit when the list arrives after the failed lookup
  • drops a latest-execution decision when the list arrives after the workflow changed
  • drops a pending latest-execution decision once the component is destroyed
  • falls back to a Running unit when the latest execution has no unit
  • falls back to a Running unit when both the remembered and the last execution's unit are gone
  • picks the first Running unit in list order when several are Running
  • does not fall back to a unit that is Pending, Terminating, Failed or Unknown, one case per status
  • only preselects the warehouse when the unit was already settled by a remembered choice

makeComputingUnit in the spec now takes an accessPrivilege override, defaulting to WRITE, so the existing fixtures are unchanged.

Existing tests in the same describe that go through the latest-execution path now pass their unit list to bootWithMetaStream(units), since the choice depends on the loaded list. Their expectations are unchanged. "prefers the remembered unit for this workflow over the latest execution" now also puts the latest execution's unit in the list; otherwise the new code would skip that unit anyway, and the test would no longer show that the remembered unit wins. The existing remembered-unit tests, including "drops a pending remembered decision once the workflow has changed underneath it", now go through the shared helper and pass unchanged.

cd frontend
npx ng test --watch=false --include src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts
# 214 passed
npx ng test --watch=false
# 233 files, 6596 passed | 1 skipped
npx tsc --noEmit -p tsconfig.json               # clean
npx eslint <changed files>                      # clean
npx prettier --check <changed files>            # clean

On Node 25 the spec run also needs NODE_OPTIONS="--no-experimental-webstorage", since its built-in localStorage shadows jsdom's. CI runs Node 24 with yarn --cwd frontend run test:ci and needs no flag.

Mutation check: each of these makes at least one new test fail: dropping the list check, skipping a latest execution that has no unit, picking the last Running unit instead of the first, picking the first unit regardless of status, accepting any unit that is not Pending, dropping the WRITE check from the fallback, access-filtering the last execution's unit as well, dropping untilDestroyed, dropping the workflow-change guard in the shared helper (caught from both the remembered and the latest-execution path), dropping the non-empty-list wait, dropping take(1), reading the list synchronously in the no-history branch, ignoring selectUnit = false, and preferring a Running unit over the last execution's unit.

Manual check in Chrome, 1512x861 viewport, against a build of this branch on the local-dev stack with local computing units:

  1. Create two local units and open a workflow with one Text Input operator.
  2. Select unit A, select a warehouse, click Run, and wait for it to complete.
  3. Terminate unit A from the unit dropdown's trash icon.
  4. Reload the workflow. Before: no unit is selected and the run button reads "Computing Unit". After: the Running unit is selected and the run button reads "Run".

Also checked by hand: when the last execution's unit is still listed, that unit is selected rather than the first Running one; a workflow that has never run selects the first Running unit.

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

Yes. Generated-by: Claude Code (claude-fable-5-1, claude-opus-5-5). Co-authored with Claude; the author reviewed the change before submission.

selectFromLastExecution now waits for the loaded unit list and selects the last execution's unit only while it is still listed, otherwise the first Running unit, and the no-history fallback waits for the same list instead of reading it synchronously.
@github-actions github-actions Bot added fix frontend Changes related to the frontend GUI labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

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.

@Yicong-Huang Yicong-Huang added release/v1.3 back porting to release/v1.3 release/v1.2 back porting to release/v1.2 labels Oct 9, 2026
@github-actions
github-actions Bot requested review from mengw15 and xuang7 October 9, 2026 02:31
@github-actions

github-actions Bot commented Oct 9, 2026 •

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 Already labeled — 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.
✅ release/v1.2 Already labeled — 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.

Auto-label run.

@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.82%. Comparing base (bc4f3c8) to head (5f587bb).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8935      +/-   ##
============================================
- Coverage     91.95%   91.82%   -0.14%     
+ Complexity     5081     5073       -8     
============================================
  Files          1280     1274       -6     
  Lines         55622    55415     -207     
  Branches       7048     7016      -32     
============================================
- Hits          51147    50884     -263     
- Misses         2733     2808      +75     
+ Partials       1742     1723      -19     
Flag Coverage Δ *Carryforward flag
access-control-service 77.44% <ø> (ø) Carriedforward from 99ecb1f
agent-service 99.16% <ø> (ø) Carriedforward from 99ecb1f
amber 85.91% <ø> (-0.46%) ⬇️ Carriedforward from 99ecb1f
computing-unit-managing-service 61.71% <ø> (-0.10%) ⬇️ Carriedforward from 99ecb1f
config-service 87.50% <ø> (ø) Carriedforward from 99ecb1f
file-service 82.29% <ø> (ø) Carriedforward from 99ecb1f
frontend 96.53% <100.00%> (+0.01%) ⬆️
notebook-migration-service 83.77% <ø> (ø) Carriedforward from 99ecb1f
pyamber 98.54% <ø> (ø) Carriedforward from 99ecb1f
workflow-compiling-service 74.25% <ø> (ø) Carriedforward from 99ecb1f

*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.

@eugenegujing

Copy link
Copy Markdown
Contributor Author

/request-review @kunwp1

@github-actions
github-actions Bot requested a review from kunwp1 October 24, 2026 23:15

@kunwp1 kunwp1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, the fix looks right to me.

@kunwp1
kunwp1 enabled auto-merge October 25, 2026 17:06
@kunwp1
kunwp1 disabled auto-merge October 25, 2026 17:06
The fallback in selectFromLoadedUnits now skips units without WRITE access, since Run is disabled on them, and the wait for the first loaded unit list is shared with the remembered-unit branch through a new onceUnitsLoaded helper.
@kunwp1
kunwp1 enabled auto-merge October 27, 2026 02:54
@kunwp1

kunwp1 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

This PR is blocked by the release labels. @mengw15 Should we remove the release labels?

@mengw15 mengw15 removed release/v1.2 back porting to release/v1.2 release/v1.3 back porting to release/v1.3 labels Oct 27, 2026
@kunwp1

kunwp1 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

@eugenegujing Can you address the frontend unit test failure?

@kunwp1
kunwp1 added this pull request to the merge queue Oct 27, 2026
@eugenegujing

Copy link
Copy Markdown
Contributor Author

@kunwp1 PTAL!

Merged via the queue into apache:main with commit a40d1ed Oct 27, 2026
23 checks passed
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.

A workflow whose last computing unit was terminated opens with no unit selected

5 participants