Bug fixes batch

[thomtrp]

Fixes #18181

Fixes #16842
Iterators remain running, which prevent the stopping state to eventually become stopped

Fixes #18186

[cubic-dev-ai]

1 issue found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-ui/src/components/chip/Chip.tsx">

<violation number="1" location="packages/twenty-ui/src/components/chip/Chip.tsx:202">
P2: Whitespace-only labels are no longer treated as empty, causing blank-looking chips instead of the empty label fallback.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: Whitespace-only labels are no longer treated as empty, causing blank-looking chips instead of the empty label fallback.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-ui/src/components/chip/Chip.tsx, line 202:

<comment>Whitespace-only labels are no longer treated as empty, causing blank-looking chips instead of the empty label fallback.</comment>

<file context>
@@ -197,7 +199,7 @@ export const Chip = ({
     >
       {leftComponent}
-      {!isLabelHidden && label && label.trim() ? (
+      {!isLabelHidden && isDefined(label) && isNonEmptyString(label) ? (
         <OverflowingTextWithTooltip size={size} text={label} />
       ) : !forceEmptyText && !isLabelHidden ? (
</file context>

Suggested change

	{!isLabelHidden && isDefined(label) && isNonEmptyString(label) ? (
	{!isLabelHidden && isDefined(label) && isNonEmptyString(label.trim()) ? (

[thomtrp]

that's intended

[cubic-dev-ai]

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

[greptile-apps]

Greptile Summary

This PR bundles three independent bug fixes: optional chaining guards for null record[filterKey] values in isRecordMatchingFilter, a Chip label rendering fix to handle null/undefined labels, and a workflow iterator stopping fix.

Key findings:

• isRecordMatchingFilter.ts: Adding ?. to all composite field accesses (fullName, address, links, actor, emails, phones) is a clean, targeted null-safety fix with no regressions.

• get-stopped-iterator-step-infos.util.ts: New utility is well-isolated, correctly typed, and easy to test.

• workflow-runner.workspace-service.ts: The primary workflow fix marks iterator steps as STOPPED before transitioning to STOPPING, which handles the common case (stop requested between loop iterations) correctly. However, a race condition exists where an actively-running iterator job can overwrite the persisted STOPPED status back to RUNNING via updateWorkflowRunStepInfo, leaving the workflow permanently stuck in STOPPING. Consider checking if all running steps remain stopped after marking iterators, and if so, transition directly to STOPPED.

• Chip.tsx: The move from label.trim() to isNonEmptyString(label) resolves the null/undefined crash but changes behavior—whitespace-only labels (e.g., " ") now render as blank content instead of falling back to emptyLabel.

Confidence Score: 3/5

• Safe to merge for most scenarios, but the workflow stopping fix has an unresolved race condition that can leave a run permanently in STOPPING state.
• The frontend fixes (isRecordMatchingFilter, Chip) are straightforward and low-risk. The workflow fix addresses the root cause for the common case but has an inherent race condition when a stop is requested while an iterator job is actively executing. In that scenario, the job's subsequent updateWorkflowRunStepInfo call overwrites the STOPPED status, and no future job will ever correct it, resulting in a workflow permanently stuck in STOPPING state.
• packages/twenty-server/src/modules/workflow/workflow-runner/workspace-services/workflow-runner.workspace-service.ts — the iterator-stopping logic needs attention for the race condition edge case.

_{Last reviewed commit: 41623f2}

[charlesBochet]

naming improve: get means that we are not changing. Here we are mutating.
setAllIteratorsStepInfosAsStopped (is it really what we want btw?)

[thomtrp]

I renamed it but that's not a mutation yet, we are just building the infos.
That's what we want because current stoppage only works when there are no running steps left. But an iterator remain running until it has been called by its last iteration. Which will never happen if the iteration steps have been stopped.

[charlesBochet]

better still not happy with naming :p is the iterator the one stopping? I prefer: setAllIteratorsStepInfosAsStopped

[charlesBochet]

approving though, non blocking