Fix/orchestrator plugins#2518
Conversation
…#2386) * feat(orchestrator): add card height mode config Expose a workflow instance page option to switch between fixed card heights and content-based sizing, with a new hook and changeset entry. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(orchestrator): simplify layout and warn on invalid mode Refactor the workflow instance layout to reuse card components and rename the height mode flag for clarity, and warn when config values are unexpected before falling back to fixed mode. Made-with: Cursor --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…t retrigger (#2279) * fix: show spinner on ActiveText retrigger * fix(orchestrator-form-widgets): keep spinner until ActiveText eval completes * Merge upstream/main Co-authored-by: Cursor <cursoragent@cursor.com> * chore(changeset): mention clearOnRetrigger Document the new fetch:clearOnRetrigger behavior in the existing changeset for the ActiveText retrigger spinner update. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(orchestrator-form-widgets): dedupe clearOnRetrigger Extract shared clear-on-retrigger behavior into a reusable hook and reuse it across ActiveTextInput, ActiveDropdown, and ActiveMultiSelect. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(orchestrator-form-widgets): guard retrigger races Ignore stale fetch responses when retrigger values change and avoid reapplying cached data while a retriggered fetch is loading. Use layout effect for clearOnRetrigger to reduce UI flicker. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
karthikjeeyar
requested review from
batzionb,
gciavarrini and
mareklibra
as code owners
Review Summary by QodoAdd card height mode config and fix retrigger spinner timing in orchestrator
WalkthroughsDescription• Add card height mode config for workflow instance page layout • Show spinner immediately on ActiveText retrigger to prevent stale values • Clear widget values when fetch:retrigger dependencies change • Guard against stale fetch responses with request ID tracking • Extract reusable clearOnRetrigger hook for form widgets Diagramflowchart LR
A["Config: cardHeightMode"] -->|"fixed or content"| B["useWorkflowInstanceCardHeightMode"]
B -->|"applies layout"| C["WorkflowInstancePageContent"]
D["fetch:clearOnRetrigger prop"] -->|"enables"| E["useClearOnRetrigger hook"]
E -->|"clears on change"| F["ActiveTextInput/Dropdown/MultiSelect"]
G["useFetch hook"] -->|"tracks requestId"| H["Prevents stale responses"]
H -->|"guards setData/setError"| I["Consistent widget state"]
File Changes1. workspaces/orchestrator/plugins/orchestrator-common/config.d.ts
|
Code Review by Qodo
1. Stale request clears loading
|
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
| const requestId = ++requestIdRef.current; | ||
| const retriggerSnapshot = retrigger; | ||
| try { | ||
| setError(undefined); | ||
| if (typeof evaluatedFetchUrl !== 'string') { |
There was a problem hiding this comment.
1. Stale request clears loading 🐞 Bug ⛯ Reliability
In useFetch, when fetch:retrigger changes you set loading=true immediately (to show a spinner during the debounce window), but any in-flight request from the previous retrigger can still run its catch/finally and call setError / setLoading(false) because those paths are guarded only by requestId. Since requestIdRef is incremented only when the next debounced fetch starts, the old request remains “current” during the debounce delay and can remove the spinner and/or display an error for stale inputs.
Agent Prompt
## Issue description
`useFetch` intends to show a spinner immediately on `fetch:retrigger` changes (during the debounce window), but an older in-flight request can still call `setError(...)` / `setLoading(false)` while the next request is waiting for the debounce delay.
## Issue Context
`requestIdRef.current` is incremented only when a debounced fetch actually starts. If retrigger changes while a request is in-flight, that request is still considered current until the next fetch begins, so its `catch/finally` updates can override the immediate `setLoading(true)` effect.
## Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useFetch.ts[80-128]
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useFetch.ts[142-197]
## Implementation notes
- Increment/invalidate `requestIdRef.current` (or a separate "generation" ref) inside the retrigger-change effect so any older request becomes non-current immediately.
- Alternatively/additionally, gate `setError` and `setLoading(false)` with the same `(requestId === current && isEqual(retriggerSnapshot, latestRetriggerRef.current))` check used for `setData`.
- Consider aborting the previous request (if `fetchApi.fetch` supports `signal`) to avoid wasted work and simplify state consistency.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2 participants
This is a manual cherry pick of d6dff3b and 309547d