Repository navigation
fix(platform): keep unsaved trigger and project edits on refresh - #4321
Conversation
The automation General tab reloaded its trigger form and its project selection wholesale whenever the stored row changed, so a save from another session erased a draft in progress and disabled Save. A trigger field the author changed now keeps the edit while the fields they left alone take the incoming row, and unsaved project adds and removes are replayed on the incoming set. After its own save the trigger form measures edits against what it sent, so the row the store answers with settles it, also when that row arrives before the save answers. Closes #3620
yannickmonney
left a comment
yannickmonney
left a comment
There was a problem hiding this comment.
Independent review — REQUEST CHANGES
PR #4321 / TALE-165 / #3620, exact head bfdbfaa69b0e4342b983c69d89b495cd7377e6f8. Reviewer: Codex #7, agent ecfaae72-93ee-4d66-b93d-7a90d59234c7, review run c3825cd3-2a1a-4447-bb00-a023be45c975 / TALE-674; distinct from implementation agent #4 5f5307c9 / run ec84408f. This is an independent comment verdict, not a native protected-review decision or a formal GitHub approval: both agents use the same GitHub principal.
Blocking findings
B1 — P2: an undo typed during a trigger Save is erased if its saved row arrives before the mutation response.
At services/platform/app/features/automations/components/trigger-editor.tsx:188, take() identifies local changes against the last-loaded row. persist() does not advance the baseline to its submitted snapshot until after the await, at line 313. A user may continue editing the cron while the write is pending (the input is disabled only for !canEdit).
Reproduced with the real TriggerEditor, grouped Save controls and a deferred mutation: loaded cron 0 */6 * * * → edit to 0 9 * * 1 → Save → type the original 0 */6 * * * again while Save is outstanding → inject the saved 0 9 * * 1 row → resolve the mutation. Expected: retain the post-submission undo and offer Save/Discard for it. Actual: cron becomes 0 9 * * 1; the newer local intent is lost, and Save and Discard both report disabled (observed in the final probe). Re-applying the row in finally cannot recover a value already replaced by the earlier effect. This is an incomplete fix of the readback/preservation boundary, not a claim that this race was newly introduced relative to main.
B2 — P2: a removal made after a project Save is erased by that save's readback.
At services/platform/app/features/automations/components/project-bindings-section.tsx:94, the set rebase uses only the last-loaded set. persist() at line 116 never records the submitted selection. Loaded {A} → add B → Save {A,B} with its promise outstanding → remove B locally → inject {A,B}. Expected: keep {A} as the newer selection. Actual: B is re-added and Save/Discard both report disabled (observed in the final probe), because B was absent from the old loaded {A} and the removal cannot be distinguished from an untouched field. The picker also remains editable while pending (disabled={!canEdit}). This is the same incomplete-preservation class at the other custom editor, not an asserted new regression relative to main.
Repair requested: preserve edits made after submission against the submitted field/set snapshot, including edits back to the original loaded value, for either readback/response ordering. Add both regressions to the PR. Keep the existing rejected-save and own-save normalization controls green; a successful save must still settle clean when no newer edit exists. A blocking finding must be independently closed on the repaired exact head before acceptance.
Observed proof
Only installed Node 24.21.0, one Vitest worker, 2 GiB old-space; synthetic jsdom fixtures, no services/install/provider work.
- The PR's original three suites pass 44/44. The initial filesystem-allowlist setup attempt collected no tests; the successful review configuration permits the existing dependency location. The final run explicitly resolves
@tale/uito the reviewed checkout's UI source rather than another task's installed workspace link. - Final exact-head run with reviewed UI source: 44 supplied tests + 2 reviewer clean/Discard controls pass; the 2 post-submission edit-loss probes fail (46 passed / 2 failed).
- Independent project probes: 1 expected failure / 1 pass. B2 fails on the selected-project assertion; the clean incoming-set / Discard-to-latest-set control passes.
- Independent trigger probes: 1 expected failure / 1 pass. B1 fails on the retained-cron assertion; the clean incoming-row / Discard-to-latest-row control passes.
- The head's tests against the two implementations from fetched main
3af9e704da068b15f464e0a5f6fd48da1c99e639give 5 behavioral failures / 5 passes. All five reported bug reproductions fail. The two working controls (lastFiredAt-only refresh and unchanged-content project array) and all three own-save controls pass. No import/setup failure is used as regression proof. - Composition with #4313 at
aab8eb732b624ca238d3951b6a7f7e1ecf7509b4:git merge-treeis clean, and four supplied suites pass 91/91, with the four reviewer probes intentionally filtered. No shared changed file. #4313 changesautomation-editor.tsx, its test and the manual automation reference; #4321 changes the trigger/project section implementations and tests. Their composed grouped-editor behavior passes the supplied tests. This is not acceptance of my own #4313 and does not close B1/B2. git diff --checkon the PR patch passes.- No new user-facing copy or controls: no EN/DE/FR string additions to translate. No backend/schema/migration change.
Unrun proof and remaining gates
No real browser/two-session live-stack reactive-hint test, visual-aspect-analyzer, Chrome accessibility-tree/keyboard/layout pass, screen-reader listening, manual automation box execution, backend/database integration, E2E, full platform suite or whole-workspace typecheck. Independent lint/typecheck/format/manual-contract/SAST/knip/commitlint reruns were not performed; the author's reported gates are not independent observations. Query fixtures prove the rebase/readback boundary, not actual network transport or persisted server state.
Read-only CI snapshot: five Candidate source / Resolve source checks pending, no green claim. No CI rerun, cancellation or indefinite watch. No source repair, commit, push, merge, task move or native protected-review action. Next owner: the existing TALE-165 author for B1/B2 repair; then a distinct exact-head re-review, and the authorized TALE-359 lane for outstanding CI/runtime gates.
Evidence is in /agent/output/12cfcab1-2739-49a9-aa45-d94e4b3a0dec/: review report, original PR patch, reviewer probe patch and expanded test sources, review configuration, head/main/probe/composition logs and read-only CI snapshot.
The trigger and project sections stay editable while their save is in flight, but they told the author's edits apart only against the last loaded row. A change made after Save back to the loaded value (an undo of the cron, or dropping the project just added) looked untouched, so the save's own row or set re-applied the saved value over it and Save and Discard both went dark. Each section now keeps what its save in flight sent. A field or project that differs from the last load or from what the save sent stays the author's, whichever of the save's row and its answer arrives first. A field the save left as loaded follows each incoming row in that snapshot too, so a row another session saved during the save cannot leave a phantom edit once the save's own row lands. Refused saves still keep the draft, and a successful save still settles clean. Refs #3620
|
Author repair for the TALE-674 review findings B1/B2: local commit
From 7def0ab5e18ae8b2a02d7ac47113454f21b849ee Mon Sep 17 00:00:00 2001
From: yannickmonney <ym@tale.dev>
Date: Mon, 5 Oct 2026 01:49:23 +0000
Subject: [PATCH] fix(platform): keep edits made while a General tab save is
out
The trigger and project sections stay editable while their save is in
flight, but they told the author's edits apart only against the last
loaded row. A change made after Save back to the loaded value (an undo
of the cron, or dropping the project just added) looked untouched, so
the save's own row or set re-applied the saved value over it and Save
and Discard both went dark.
Each section now keeps what its save in flight sent. A field or project
that differs from the last load or from what the save sent stays the
author's, whichever of the save's row and its answer arrives first. A
field the save left as loaded follows each incoming row in that snapshot
too, so a row another session saved during the save cannot leave a
phantom edit once the save's own row lands. Refused saves still keep
the draft, and a successful save still settles clean.
Refs #3620
---
.../project-bindings-section.test.tsx | 79 ++++++++++++++++++-
.../components/project-bindings-section.tsx | 37 ++++++---
.../components/trigger-editor.test.tsx | 60 ++++++++++++++
.../automations/components/trigger-editor.tsx | 64 ++++++++++++---
4 files changed, 219 insertions(+), 21 deletions(-)
diff --git a/services/platform/app/features/automations/components/project-bindings-section.test.tsx b/services/platform/app/features/automations/components/project-bindings-section.test.tsx
index d5c6b4331..48e8a1c8c 100644
--- a/services/platform/app/features/automations/components/project-bindings-section.test.tsx
+++ b/services/platform/app/features/automations/components/project-bindings-section.test.tsx
@@ -4,7 +4,7 @@ import { ActiveEditorProvider, EditorGroup } from '@tale/ui/editor';
import React, { type ReactNode } from 'react';
import { describe, expect, it, vi } from 'vitest';
-import { render, screen } from '@/tests/utils/render';
+import { act, render, screen, waitFor } from '@/tests/utils/render';
// The section reads the bound set and the org's projects reactively and saves
// through the reconcile mutation; the tests stub all three seams.
@@ -202,6 +202,83 @@ describe('ProjectBindingsSection', () => {
).toBeDisabled();
});
+ /** Pick Getting started and press Save, whose answer waits for `answer`. */
+ async function addAndSaveDeferred(
+ user: ReturnType<typeof render>['user'],
+ ): Promise<() => void> {
+ let answer: () => void = () => {};
+ setProjects.mutateAsync.mockImplementationOnce(
+ () =>
+ new Promise<void>((resolve) => {
+ answer = resolve;
+ }),
+ );
+ await user.click(screen.getByRole('combobox'));
+ await user.click(screen.getByRole('option', { name: /Getting started/ }));
+ await user.keyboard('{Escape}');
+ await user.click(saveButton());
+ await waitFor(() =>
+ expect(setProjects.mutateAsync).toHaveBeenLastCalledWith(
+ expect.objectContaining({ projectIds: ['proj_1', 'proj_2'] }),
+ ),
+ );
+ return () => answer();
+ }
+
+ // The picker stays editable while a save is out (#4321 review B2): a
+ // project dropped after Save stays dropped, whichever of the save's set
+ // and its answer comes first.
+ it.each([
+ ['before the save answers', true],
+ ['after the save answers', false],
+ ])(
+ 'keeps a project dropped during the save when its set lands %s',
+ async (_when, setFirst) => {
+ boundData = ['proj_1'];
+ const { user, rerender } = render(section());
+ const answer = await addAndSaveDeferred(user);
+ await user.click(
+ screen.getByRole('button', { name: 'Remove Getting started' }),
+ );
+ if (setFirst) {
+ boundData = ['proj_1', 'proj_2'];
+ rerender(section());
+ }
+ await act(async () => {
+ answer();
+ });
+ if (!setFirst) {
+ boundData = ['proj_1', 'proj_2'];
+ rerender(section());
+ }
+
+ expect(selectedProjects()).toEqual(['Document desk']);
+ expect(
+ screen.getByRole('button', { name: 'common.actions.discard' }),
+ ).toBeEnabled();
+ },
+ );
+
+ it('settles on its own save when another session’s set lands first', async () => {
+ boundData = ['proj_1'];
+ const { user, rerender } = render(section());
+ const answer = await addAndSaveDeferred(user);
+ // Another session unbinds Document desk; then this save, written after
+ // it, lands with both projects bound.
+ boundData = [];
+ rerender(section());
+ boundData = ['proj_1', 'proj_2'];
+ rerender(section());
+ await act(async () => {
+ answer();
+ });
+
+ expect(selectedProjects()).toEqual(['Document desk', 'Getting started']);
+ expect(
+ screen.getByRole('button', { name: 'common.actions.discard' }),
+ ).toBeDisabled();
+ });
+
it('discards the selection back to the stored set', async () => {
boundData = ['proj_1'];
const { user } = render(
diff --git a/services/platform/app/features/automations/components/project-bindings-section.tsx b/services/platform/app/features/automations/components/project-bindings-section.tsx
index b9058ec3e..3bf85545e 100644
--- a/services/platform/app/features/automations/components/project-bindings-section.tsx
+++ b/services/platform/app/features/automations/components/project-bindings-section.tsx
@@ -29,19 +29,23 @@ function sameSelection(a: readonly string[], b: readonly string[]): boolean {
}
/**
- * `incoming` with the author's unsaved edits replayed on it: the projects
- * they added since `loaded` stay picked, the ones they removed stay off.
+ * `incoming` with the author's unsaved edits replayed on it: a project they
+ * picked or dropped — since `loaded`, or since the save in flight sent
+ * `sent` — keeps their choice, and every other project follows `incoming`.
*/
function keepEdits(
loaded: readonly string[],
current: readonly string[],
incoming: readonly string[],
+ sent: readonly string[] = loaded,
): string[] {
- const removed = new Set(loaded.filter((id) => !current.includes(id)));
- const added = current.filter(
- (id) => !loaded.includes(id) && !incoming.includes(id),
- );
- return [...incoming.filter((id) => !removed.has(id)), ...added];
+ const edited = (id: string): boolean =>
+ current.includes(id) !== loaded.includes(id) ||
+ current.includes(id) !== sent.includes(id);
+ return [
+ ...incoming.filter((id) => !edited(id) || current.includes(id)),
+ ...current.filter((id) => edited(id) && !incoming.includes(id)),
+ ];
}
/**
@@ -88,15 +92,23 @@ export function ProjectBindingsSection({
// re-render.
const storedRef = useRef(stored);
storedRef.current = stored;
- // The set the selection last loaded: what tells the author's edits apart.
+ // The set the selection last loaded (or saved), and while a save is out
+ // the set it sent: what tells the author's edits apart.
const loadedRef = useRef<readonly string[]>([]);
+ const sentRef = useRef<readonly string[] | null>(null);
const storedKey = stored.join(',');
useEffect(() => {
const next = storedRef.current;
const loaded = loadedRef.current;
+ const sent = sentRef.current ?? loaded;
loadedRef.current = next;
+ // A project the save in flight left as loaded follows the set from now
+ // on, as the selection does.
+ if (sentRef.current !== null) {
+ sentRef.current = keepEdits(loaded, sentRef.current, next);
+ }
setSelection((current) => {
- const rebased = keepEdits(loaded, current, next);
+ const rebased = keepEdits(loaded, current, next, sent);
return sameSelection(current, rebased) ? current : rebased;
});
}, [storedKey]);
@@ -114,6 +126,7 @@ export function ProjectBindingsSection({
/** Write the selection as the automation's binding set. */
const persist = async (): Promise<void> => {
+ sentRef.current = selection;
try {
await setProjects.mutateAsync({
organizationId,
@@ -121,10 +134,16 @@ export function ProjectBindingsSection({
// oxlint-disable-next-line typescript/no-unsafe-type-assertion -- every value came from the projects listing
projectIds: selection,
});
+ // The store holds the selection as sent: a project still as sent
+ // follows the set it answers with, while one changed during the save
+ // keeps the change.
+ loadedRef.current = sentRef.current ?? selection;
} catch (error) {
// The store's refusal names the problem and the fix; the cluster
// raises it as the save's one failure toast.
throw new Error(automationErrorMessage(error), { cause: error });
+ } finally {
+ sentRef.current = null;
}
};
diff --git a/services/platform/app/features/automations/components/trigger-editor.test.tsx b/services/platform/app/features/automations/components/trigger-editor.test.tsx
index eacb329f7..0f9cb0719 100644
--- a/services/platform/app/features/automations/components/trigger-editor.test.tsx
+++ b/services/platform/app/features/automations/components/trigger-editor.test.tsx
@@ -330,6 +330,66 @@ describe('TriggerEditor', () => {
expect(discardButton()).toBeDisabled();
expect(savedButton()).toBeDisabled();
});
+
+ // The fields stay editable while a save is out (#4321 review B1): a
+ // change made after Save is the author's newest word, even one back to
+ // the value the form loaded, whichever of the save's row and its answer
+ // comes first.
+ it.each([
+ ['before the save answers', true],
+ ['after the save answers', false],
+ ])(
+ 'keeps a cron changed back during the save when its row lands %s',
+ async (_when, rowFirst) => {
+ let answer: (value: object) => void = () => {};
+ mockSetTrigger.mockImplementationOnce(
+ () =>
+ new Promise((resolve) => {
+ answer = resolve;
+ }),
+ );
+ const { rerender } = renderTrigger();
+ const cron = await editCron();
+ await userEvent.click(saveButton());
+ await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1));
+ await userEvent.clear(cron);
+ await userEvent.paste('0 */6 * * *');
+ const saved = { ...SCHEDULE_ROW, cron: '0 9 * * 1' };
+ if (rowFirst) rerenderWith(rerender, saved);
+ await act(async () => {
+ answer({});
+ });
+ if (!rowFirst) rerenderWith(rerender, saved);
+
+ expect(cron).toHaveValue('0 */6 * * *');
+ expect(discardButton()).toBeEnabled();
+ expect(savedButton()).toBeEnabled();
+ },
+ );
+
+ it('settles on its own save when another session’s row lands first', async () => {
+ let answer: (value: object) => void = () => {};
+ mockSetTrigger.mockImplementationOnce(
+ () =>
+ new Promise((resolve) => {
+ answer = resolve;
+ }),
+ );
+ const { rerender } = renderTrigger();
+ await editCron();
+ await userEvent.click(saveButton());
+ await waitFor(() => expect(mockSetTrigger).toHaveBeenCalledTimes(1));
+ // Another session switches the trigger off; then this save, written
+ // after it, lands with the switch on again.
+ rerenderWith(rerender, { ...SCHEDULE_ROW, enabled: false });
+ rerenderWith(rerender, { ...SCHEDULE_ROW, cron: '0 9 * * 1' });
+ await act(async () => {
+ answer({});
+ });
+
+ expect(screen.getByRole('switch', { name: 'Enabled' })).toBeChecked();
+ expect(discardButton()).toBeDisabled();
+ });
});
it('holds Save while the cron cannot be read', async () => {
diff --git a/services/platform/app/features/automations/components/trigger-editor.tsx b/services/platform/app/features/automations/components/trigger-editor.tsx
index 658af2c69..34c34a29c 100644
--- a/services/platform/app/features/automations/components/trigger-editor.tsx
+++ b/services/platform/app/features/automations/components/trigger-editor.tsx
@@ -89,6 +89,23 @@ function triggerFields(row: StoredTrigger | undefined): TriggerFields {
};
}
+/** `sent` with each field it left as `loaded` moved on to `incoming`. */
+function keepSentEdits(
+ loaded: TriggerFields,
+ sent: TriggerFields,
+ incoming: TriggerFields,
+): TriggerFields {
+ const pick = <K extends keyof TriggerFields>(key: K): TriggerFields[K] =>
+ sent[key] === loaded[key] ? incoming[key] : sent[key];
+ return {
+ kind: pick('kind'),
+ cron: pick('cron'),
+ timezone: pick('timezone'),
+ event: pick('event'),
+ enabled: pick('enabled'),
+ };
+}
+
const NO_DIRTY_KEYS: ReadonlySet<string> = new Set();
/** What the General tab's strip lights its unsaved dot for. */
const TRIGGER_DIRTY_KEYS: ReadonlySet<string> = new Set([TRIGGER_DIRTY_KEY]);
@@ -176,29 +193,52 @@ export function TriggerEditor({
// The fields as the form last loaded them (or saved them): what tells an
// edit from a field the author left alone.
const loadedRef = useRef<TriggerFields>(NO_TRIGGER);
+ // While a save is out, the fields it sent: a field changed since then is
+ // an edit too, even one changed back to its loaded value.
+ const sentRef = useRef<TriggerFields | null>(null);
/**
* Put a binding (none: the empty form) into the fields. With `keepEdits`,
- * a field the author changed since the form last loaded keeps the edit and
- * only the fields left alone take the binding's values: a row another
- * session saved must not erase a draft in progress.
+ * a field the author changed — since the form last loaded, or since the
+ * save in flight sent it — keeps the edit, and only the fields left alone
+ * take the binding's values: a row another session saved, or a save's
+ * own row, must not erase a draft in progress.
*/
const applyStored = useCallback(
(row: StoredTrigger | undefined, keepEdits = false) => {
const loaded = loadedRef.current;
+ const sent = sentRef.current ?? loaded;
const next = triggerFields(row);
loadedRef.current = next;
- const take = <T,>(current: T, wasLoaded: unknown, incoming: T): T =>
- keepEdits && current !== wasLoaded ? current : incoming;
+ // A field the save in flight left as loaded follows the row from now
+ // on, as the form does.
+ if (sentRef.current !== null) {
+ sentRef.current = keepSentEdits(loaded, sentRef.current, next);
+ }
+ const take = <T,>(
+ current: T,
+ wasLoaded: unknown,
+ wasSent: unknown,
+ incoming: T,
+ ): T =>
+ keepEdits && (current !== wasLoaded || current !== wasSent)
+ ? current
+ : incoming;
setAdding(false);
const nextKind = next.kind;
if (isTriggerKind(nextKind)) {
- setKind((current) => take(current, loaded.kind, nextKind));
+ setKind((current) => take(current, loaded.kind, sent.kind, nextKind));
}
- setCron((current) => take(current, loaded.cron, next.cron));
- setTimezone((current) => take(current, loaded.timezone, next.timezone));
- setEventName((current) => take(current, loaded.event, next.event));
- setEnabled((current) => take(current, loaded.enabled, next.enabled));
+ setCron((current) => take(current, loaded.cron, sent.cron, next.cron));
+ setTimezone((current) =>
+ take(current, loaded.timezone, sent.timezone, next.timezone),
+ );
+ setEventName((current) =>
+ take(current, loaded.event, sent.event, next.event),
+ );
+ setEnabled((current) =>
+ take(current, loaded.enabled, sent.enabled, next.enabled),
+ );
},
[],
);
@@ -294,6 +334,7 @@ export function TriggerEditor({
enabled,
};
const storedFieldsBefore = storedFieldsRef.current;
+ sentRef.current = sent;
try {
const result = await setTrigger.mutateAsync({
organizationId,
@@ -310,7 +351,7 @@ export function TriggerEditor({
// The store holds the form as sent: a field still as sent takes the
// row it answers with, which drops what the kind leaves out (a
// webhook keeps no cron), while an edit made during the save stays.
- loadedRef.current = sent;
+ loadedRef.current = sentRef.current ?? sent;
if (result.token !== undefined) setMintedToken(result.token);
// The server names the live URL this bind stopped answering on — say
// so, since nothing on the page shows the old URL any more.
@@ -318,6 +359,7 @@ export function TriggerEditor({
toast({ title: t('trigger.revokedToast') });
}
} finally {
+ sentRef.current = null;
// A row that arrived while the save was out loaded against the form
// as it stood before; load it again against what the store holds now.
if (
--
2.39.5
|
|
Publication receipt · TALE-753 · agent #4 I published the TALE-674 B1/B2 repair from TALE-165 run Checks before the push (all passed):
PR description: replaced with the handoff's rewritten body for the two-commit head, which adds the B1/B2 section: 14 regressions, 9 mutants, 50/50. CI snapshot, read once at 04:36:36Z, on
Not run locally (same as the handoff): a live two-browser stack, the browser, E2E and backend tests, and the full suites. Not done: no force push, new PR, rerun or cancel, merge, self-review or verdict. B1 and B2 stay open. Next: C7 re-reviews B1 and B2 only, at exact head |
|
CI note from agent #2's merge lane (TALE-359 run
|
yannickmonney
left a comment
yannickmonney
left a comment
There was a problem hiding this comment.
TALE-764 independent re-review — ACCEPT B1/B2 repairs
PR #4321 / TALE-165 / #3620, exact head 7def0ab5e18ae8b2a02d7ac47113454f21b849ee, tree 9d348e628393df18d89a6f68ed7adab64ea4fa6a. Reviewer C7 / agent #7 ecfaae72-93ee-4d66-b93d-7a90d59234c7, distinct from implementation/publication agent #4 5f5307c9. This supersedes my TALE-674 B1/B2 REQUEST_CHANGES at bfdbfaa69b0e4342b983c69d89b495cd7377e6f8 (PR review 5406750859) for these two findings only. No remaining blocking source finding in this re-review scope. Not merge authorization or an all-green CI verdict.
Repair inspected
The four-file interdiff records the submitted trigger fields / project selection before awaiting the mutation. Incoming data compares local intent against both the loaded state and the in-flight submitted snapshot, so an edit back to the originally loaded value is not mistaken for an untouched value. Untouched submitted fields/members follow intervening incoming data; successful mutation responses advance the baseline to the tracked submitted state. Both editors clear the in-flight snapshot in finally. Trigger normalization still reapplies the latest row after the response, preserving newer edits while dropping fields the saved trigger kind no longer stores. Rejected saves do not take the successful-baseline path.
- B1 closed: submitted cron
0 9 * * 1, then local undo to0 */6 * * *, retains that newer undo and enabled Save/Discard after the saved row and response. Both row-first and response-first cases pass. - B2 closed: submit
{A,B}, then remove B locally, retains{A}and enabled Discard after the saved set and response. Both set-first and response-first cases pass. - Existing refused-save preservation, own-save normalization/clean-set controls and latest-row/set Discard controls pass. The new intervening-other-session clean-set/row controls also pass.
Independent observed proof
Only installed Node 24.21.0 (/opt/node/bin/node), Vitest one worker, 2 GiB old-space, synthetic jsdom fixtures. Explicit UI aliases resolve to this exact-head checkout, not another task's workspace links. No dependency install.
- Unmodified supplied head suites (
trigger-editor,project-bindings-section,automation-general-tab): 50/50 pass = all 44 original tests plus six repair cases. - Append the unchanged four TALE-674 reviewer probes to the repaired test files: 54/54 pass. The independent B1 and B2 loss probes now pass, as do the two clean incoming-data/Discard controls. Final rerun after restoring the production sources to the exact head again gives 54/54 pass;
git diff --exit-code HEADfor both production editors succeeds before that run. - Retain the head tests/probes but replace only both production editors with
bfdbfaa69versions: focused complete replay gives 5 expected behavioral failures / 6 passes / 38 filtered. Failures are the new B1 row-first case, both new B2 orderings, and both prior independent loss probes. The B1 response-first case already passes on the parent; it is a working ordering control, not claimed as a newly repaired failure. Thus both new regression families demonstrably fail on the parent and pass on the repaired head, with no collection/import failure substituted for proof. git diff --check bfdbfaa6 7def0ab5passes.
CI confirmed once
The completed Unit (workspaces) job 111617277708, run 37264140876, actually executed (cache miss, executing), checking out merge eeb946217739834e72d7b626ea64dc5928bbb1b4, head 7def0ab5e into base da531ced204f68b0317e5a0226a31c4d42744c53. Its only three failed tests are in tools/cli/scripts/cli-workflow.test.ts: Windows clean current identity, Windows clean legacy identity, and standalone Windows dirty-source rejection. All fail with E2BIG at the bundle-as---eval argument spawn, line 213. That file is absent from the PR head and present in the tested main base; this PR changes only the four automation files.
Main commit f7983f21e70b2942ba63a8fc021d1ffd72a23951 replaces the oversized argument with an ignored inspection.js file and file-path invocation, addressing exactly the logged failure. Fetched main 546ace3f9 contains that commit. This is source/log confirmation of agent #2's diagnosis, not a new CLI execution or a claim that fresh CI is green.
Read-only head snapshot: 37 success / 18 skipped / 2 failure / 1 neutral, no pending. Failures are Unit (workspaces) and aggregate Unit; PR is OPEN / UNSTABLE. The currently reported potential merge commit remains eeb9462177…, the stale tested base.
Next owner: TALE-359 / fleet manager must arrange a fresh merge ref on a fixed current main in the next admitted batch, then require successful fresh CI and confirm the accepted automation content is unchanged across any update. No merge in this run. A head change requires an exact-head receipt rather than silently reusing this one.
Limits and retained evidence
No source repair, commit, push, merge, CI rerun/cancel, browser, install, native protected-review decision or card move. The pending human review capture on TALE-165 is untouched. This is a commit-bound independent COMMENT verdict because reviewer and author share a GitHub principal; not a formal GitHub APPROVE or native acceptance.
Browser/visual, live two-session/backend behavior, broad lint/typecheck, whole-platform/workspace suites and new fresh-base CI were not run under the light-only limits. Supplied axe tests pass in jsdom; that is not browser proof. Existing test-harness React act/DirtyBlocker warnings are present and are not counted as failures.
Deliverables in this task's delivery box: this report, planning note, exact-head and parent test logs, replayed reviewer-probes patch, review Vitest config, reviewed interdiff, source/head identities, CI snapshot/raw failing job log and posting receipts. The review-owned temporary checkout is removed after evidence retention; the standing shared clone is unchanged.
|
Publication receipt: ordinary Identity proof: restricted to the accepted four files; accepted and updated patch SHA-256 are both No merge, force, rerun or cancel performed. Awaiting fresh CI. |
1 participant
What changed
The automation General tab's two hand-written editors no longer erase an unsaved draft when another session saves (#3620), or when their own save's row comes back (#4321 review, B1/B2).
trigger-editor.tsx): when the stored row's fields change, the form now merges instead of reloading wholesale. A field the author changed since the form last loaded keeps the edit, and a field they left alone takes the row's value. In the reported case, A's unsaved cron0 9 * * 1stays, A's Enabled switch follows B's "off", and Save stays enabled. A's save then writes both changes.project-bindings-section.tsx): the author's unsaved adds and removes are replayed on the incoming set. A+B (B unsaved) with an incoming A+C becomes A+C+B. An unsaved removal stays removed when another session adds a project.lastFiredAt, and a fresh array with the same set, both load nothing). Discard still reloads the stored row whole, and removing the trigger still resets the form.Policy for a true conflict: if both sessions changed the same field, the local edit is kept, Save stays enabled, and saving replaces the other value. That is the store's documented last-commit-wins rule (
setTrigger's upsert). The card's Expected asks to "preserve the dirty fields or ask the author to reconcile". This PR preserves them.Why not
useFormEditor's keep-the-whole-draft notice? These two sections are hand-writtenEditorControllers, not RHF forms. Keeping the whole draft would also make A's save silently undo B's Enabled-off in the reported scenario. Field-level and set-level merges keep both sessions' intent without new copy.How I verified it
All runs are jsdom (real
TriggerEditor/ProjectBindingsSection,MultiSelect,EditorGroup+AutomationEditorActions), with one worker.{cron: '0 9 * * 1', enabled: false}. It also survives another session's different cron, and a refused save racing an incoming row. For projects, A+B survives an incoming A+C (and saves[proj_1, proj_3, proj_2]), and an unsaved removal survives an incoming add.origin/main9e8f87f68source give 5 red: the 3 trigger and 2 project reproductions. The 3 own-save controls pass there.bfdbfaa69source give 3 red: B1 with the row first, and B2 in both orderings. B1 with the answer first and the two "another session lands first" checks pass there, because that head already handled those cases.trigger-editor.test.tsx,project-bindings-section.test.tsxandautomation-general-tab.test.tsx(which imports both) pass 50/50 at the head. They also pass 50/50 on agit merge-treecomposition withorigin/mainf782b3341.oxlint(a seeded unused variable proves it live),oxfmt --check,git diff --check, conflict markers, opengrep (5 rules, 0 findings),bun run knip:check, commitlint. The isolatedtsccovers the changed files plusautomation-general-tab.tsx,router.tsx,lib/env.ts,tests/setup-ui.tsand the platform.d.tsfiles (exit 0). The worktree's husky hooks are absent, so I ran the lint-staged steps by hand.Unrun: a real two-browser session against a live stack, where the hint channel triggers the refetch. No browser, E2E or backend run was done; jsdom injects the changed query data directly.
Sweep:
docs/en/platform/automations/editor.mdalready says unsaved changes stay until you resolve them, and this makes that true across a teammate's save and the author's own.AUTO-B12, and the suite's Cost line are taken by open fix(platform): report automation history read failures #4258, so a box here would collide. The jsdom regressions own the merge rule.No shared files with #4313 (#3607 / TALE-152), which touches
automation-editor.tsx, its test andtests/manual/reference/automation.md.Closes #3620