Skip to content

Commit 87e50a1

Browse files
committed
fix(spec): a page action row declares ActionSchema's operation and refuses onSuccess beside operation: 'update' with the action's text
Claude-Session: https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz Co-authored-by: Claude <noreply@anthropic.com>
1 parent e4ec6ec commit 87e50a1

5 files changed

Lines changed: 180 additions & 15 deletions

File tree

‎packages/spec/src/ui/action-on-success-executor.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@
3434
* app-shell `utils/flowResponse.ts`), so the hop fires while that screen or
3535
* the refusal notice is still open.
3636
*
37+
* `operation: 'update'` is read BEFORE any executor (`ActionRunner.execute`,
38+
* `:1384-1388` at the pin): the runner performs the declarative write through
39+
* the platform action route, runs no handler and no executor of its own, and
40+
* then hands the action to `handlePostExecution` like every other branch. So a
41+
* hop beside it follows the write, whose success carries neither a handler's
42+
* return nor a call's response for `${result.*}` to read.
43+
* `ActionSchema` refuses `onSuccess` beside `operation: 'update'` on every
44+
* `type`, and the page action rows refuse it the same way, with the one text
45+
* {@link onSuccessBesideUpdateOperationRefusal} prints.
46+
*
3747
* A page action row adds one case `ActionSchema` cannot have, because the
3848
* action's `type` defaults to `script`: a row that names no executor. The
3949
* runner dispatches it on nothing, so no executor's success is there for the
@@ -113,3 +123,19 @@ export function onSuccessBesideExecutorRefusal(
113123
+ `success carries none, and ${HOPS_AFTER_EVERY_EXECUTOR}. ${DECLARED_BESIDE} ${MOVE_THE_HOP}`;
114124
}
115125
}
126+
127+
/**
128+
* The refusal of an `onSuccess` beside `operation: 'update'`, on `ActionSchema`
129+
* and on a page action row alike: the runner reads `operation` before any
130+
* executor, so the hop would follow the declarative write, and whatever
131+
* executor the action or row names is never run. `subject` opens it ("An
132+
* `operation: 'update'` action's `onSuccess`").
133+
*/
134+
export function onSuccessBesideUpdateOperationRefusal(subject: string): string {
135+
return `${subject} would follow a result the contract does not declare: the console's action runner reads `
136+
+ "`operation` before any executor, so an `operation: 'update'` action runs no handler and no executor of its "
137+
+ 'own (the platform action route writes `patch` to the current record as the caller), and the runner then '
138+
+ 'performs the post-success hop after that write succeeds. `${result.*}` reads a handler\'s return or a '
139+
+ "call's response, and the declarative write declares neither. Drop `onSuccess`; post-success navigation "
140+
+ 'after the declarative write is a spec proposal, not a silent key.';
141+
}

‎packages/spec/src/ui/action-row-update.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe('mixing rule: one refusal per contradiction, at the contradicting key',
215215
['bodyShape', { wrap: 'data' }, /^`bodyShape` shapes a `type: 'api'` request body — an `operation: 'update'` action sends no request body of its own/],
216216
['recordIdParam', 'id', /^`recordIdParam` injects the row id into a request body — an `operation: 'update'` action binds the current record from the invocation context/],
217217
['recordIdField', 'token', /^`recordIdField` seeds `recordIdParam` — an `operation: 'update'` action binds the current record from the invocation context/],
218-
['onSuccess', { navigate: '/tasks' }, /^`onSuccess` is not defined for an `operation: 'update'` action yet/],
218+
['onSuccess', { navigate: '/tasks' }, /^An `operation: 'update'` action's `onSuccess` would follow a result the contract does not declare: the console's action runner reads `operation` before any executor, so an `operation: 'update'` action runs no handler and no executor of its own/],
219219
['opensInNewTab', true, /^`opensInNewTab` pre-opens a tab for a handler-returned `\{ redirectUrl \}` — an `operation: 'update'` action has no handler/],
220220
['newTabUrl', '/open/{recordId}', /^`newTabUrl` is the zero-roundtrip target of the pre-opened-tab flow — an `operation: 'update'` action has no such flow/],
221221
];
@@ -232,6 +232,17 @@ describe('mixing rule: one refusal per contradiction, at the contradicting key',
232232
});
233233
}
234234

235+
it('refuses `onSuccess` beside `operation: \'update\'` on every `type` it parses with, ONE issue at `onSuccess` (negative control: the same action without it parses)', () => {
236+
for (const typed of [{}, { type: 'script' as const }]) {
237+
const refused = ActionSchema.safeParse({ ...duly, ...typed, onSuccess: { navigate: '/tasks' } });
238+
expect(paths(refused), JSON.stringify(typed)).toEqual(['onSuccess']);
239+
expect(issueAt(refused, 'onSuccess')?.code).toBe('custom');
240+
expect(issueAt(refused, 'onSuccess')?.message).toContain('Drop `onSuccess`');
241+
const control = ActionSchema.safeParse({ ...duly, ...typed });
242+
expect(control.success, JSON.stringify(typed)).toBe(true);
243+
}
244+
});
245+
235246
it('refuses `patch` on an action WITHOUT `operation: \'update\'` — it would be silently dropped', () => {
236247
for (const base of [
237248
{ name: 'x', label: 'X', target: 'doThing' },

‎packages/spec/src/ui/action.zod.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod';
3535
import { lazySchema } from '../shared/lazy-schema';
3636
import { aiJsonSchemaSlot } from '../shared/ai-json-schema-slot';
3737
import { ACTION_TARGET_ALIASES } from './action-target-aliases';
38-
import { ON_SUCCESS_EXECUTORS, onSuccessBesideExecutorRefusal } from './action-on-success-executor';
38+
import {
39+
ON_SUCCESS_EXECUTORS,
40+
onSuccessBesideExecutorRefusal,
41+
onSuccessBesideUpdateOperationRefusal,
42+
} from './action-on-success-executor';
3943
import { navigationTargetRefusal } from './navigation-target';
4044

4145
/**
@@ -1885,10 +1889,9 @@ const DECLARATIVE_UPDATE_REFUSED_KEYS: ReadonlyArray<readonly [key: string, why:
18851889
['recordIdField',
18861890
"`recordIdField` seeds `recordIdParam` — an `operation: 'update'` action binds the current record from "
18871891
+ 'the invocation context and injects nothing. Drop `recordIdField`.'],
1888-
['onSuccess',
1889-
"`onSuccess` is not defined for an `operation: 'update'` action yet — its `${result.*}` scope is a "
1890-
+ "handler's return value, and a declarative update has no handler. Drop it; post-success navigation "
1891-
+ 'for the declarative write is a spec proposal, not a silent key.'],
1892+
// One text with the page action rows' refusal of the same pairing
1893+
// (`action-on-success-executor.ts`).
1894+
['onSuccess', onSuccessBesideUpdateOperationRefusal("An `operation: 'update'` action's `onSuccess`")],
18921895
['outcomeMessages',
18931896
"`outcomeMessages` picks success copy by the `outcome` a handler returns — an `operation: 'update'` "
18941897
+ 'action has no handler: the platform performs the write and reports no outcome, so no entry could '

‎packages/spec/src/ui/component-action-row-on-success-executor.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,90 @@ describe("a page action row's `onSuccess` stays open beside `api`, `script` and
337337
expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', [...row.at, 'onSuccess']]]);
338338
});
339339
});
340+
341+
// ===========================================================================
342+
// 4. operation — ActionSchema's own enum, and its `onSuccess` rule
343+
// ===========================================================================
344+
345+
/** A declarative update row: `operation: 'update'` with a static `patch`. */
346+
const UPDATE = { operation: 'update', patch: { status: 'done' } } as const;
347+
348+
/** `ActionSchema`'s refusal of `onSuccess` beside `operation: 'update'`, and the subject it opens with. */
349+
const ACTION_UPDATE_SUBJECT = "An `operation: 'update'` action's `onSuccess`";
350+
const actionUpdateOnSuccessMessage = (): string => {
351+
const issues = issuesOf(ActionSchema.safeParse({
352+
name: 'close_task', label: 'Close', ...UPDATE, onSuccess: HOP,
353+
}));
354+
expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['onSuccess']]]);
355+
expect(issues[0].message.startsWith(ACTION_UPDATE_SUBJECT)).toBe(true);
356+
return issues[0].message;
357+
};
358+
359+
describe("a page action row's `operation` is `ActionSchema`'s own enum", () => {
360+
it('NO SECOND VOCABULARY: every row holds the very enum instance `ActionSchema.operation` declares', () => {
361+
const operation = ActionSchema.in.shape.operation.unwrap();
362+
type Shape = { shape: Record<string, z.ZodOptional<z.ZodType>> };
363+
const memberOf = (props: unknown) =>
364+
((props as Shape).shape.actions.unwrap() as unknown as z.ZodArray<z.ZodType>).element as unknown as Shape;
365+
expect((ActionButtonPropsSchema as unknown as Shape).shape.operation.unwrap()).toBe(operation);
366+
expect((ActionIconPropsSchema as unknown as Shape).shape.operation.unwrap()).toBe(operation);
367+
expect(memberOf(GROUP).shape.operation.unwrap()).toBe(operation);
368+
expect(memberOf(MENU).shape.operation.unwrap()).toBe(operation);
369+
});
370+
371+
it.each(ROWS)("REFUSAL: $name with `operation: 'delete'` is refused at `operation` with the action's own text", (row) => {
372+
const issues = issuesOf(row.parse({ operation: 'delete', patch: { status: 'done' } }));
373+
expect(issues.map((i) => i.path)).toEqual([[...row.at, 'operation']]);
374+
const action = issuesOf(ActionSchema.safeParse({ name: 'x', label: 'X', operation: 'delete', patch: { status: 'done' } }))
375+
.find((i) => i.path.join('.') === 'operation');
376+
expect(issues[0].message).toBe(action?.message);
377+
expect(issues[0].message.startsWith("`operation: 'delete'` is a bulk-def operation with no row-level form")).toBe(true);
378+
});
379+
380+
it.each(ROWS)("REFUSAL: $name with `operation: 'create'` (no member) is refused at `operation`", (row) => {
381+
const issues = issuesOf(row.parse({ operation: 'create', patch: { status: 'done' } }));
382+
expect(issues.map((i) => [i.code, i.path])).toEqual([['invalid_value', [...row.at, 'operation']]]);
383+
});
384+
385+
it.each(ROWS)("ACCEPTED CONTROL: $name with `operation: 'update'` and a `patch` parses unchanged", (row) => {
386+
const r = row.parse({ ...UPDATE });
387+
expect(r.success, JSON.stringify(issuesOf(r))).toBe(true);
388+
expect(row.parsed(r.data).operation).toBe('update');
389+
});
390+
});
391+
392+
describe("a page action row's `onSuccess` is refused beside `operation: 'update'`, as `ActionSchema` refuses it", () => {
393+
/** No executor, the two the hop is declared beside, and a host handler: the write runs first on each. */
394+
const BESIDE = [undefined, 'api', 'script', 'run'] as const;
395+
const updateRow = (row: Row, executor: string | undefined, extra: Record<string, unknown> = {}) =>
396+
row.parse({ ...(executor === undefined ? {} : { [row.executorKey]: executor }), ...UPDATE, ...extra });
397+
const updateSubject = (row: Row) => onSuccessSubject(row, "`operation: 'update'`");
398+
399+
it.each(ROWS.flatMap((row) => BESIDE.map((executor) => [row.name, String(executor), row, executor] as const)))(
400+
'REFUSAL: %s with executor %s, `operation: \'update\'` and `onSuccess` is refused at `onSuccess`, ONE issue',
401+
(_name, _label, row, executor) => {
402+
const issues = issuesOf(updateRow(row, executor, { onSuccess: HOP }));
403+
expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', [...row.at, 'onSuccess']]]);
404+
expect(firstSentence(issues[0].message)).toBe(
405+
`${updateSubject(row)} would follow a result the contract does not declare: the console's action runner reads `
406+
+ "`operation` before any executor, so an `operation: 'update'` action runs no handler and no executor of its "
407+
+ 'own (the platform action route writes `patch` to the current record as the caller), and the runner then '
408+
+ 'performs the post-success hop after that write succeeds.',
409+
);
410+
},
411+
);
412+
413+
it.each(ROWS.flatMap((row) => BESIDE.map((executor) => [row.name, String(executor), row, executor] as const)))(
414+
'NEGATIVE CONTROL: %s with executor %s and `operation: \'update\'` and no `onSuccess` parses',
415+
(_name, _label, row, executor) => {
416+
const r = updateRow(row, executor);
417+
expect(r.success, JSON.stringify(issuesOf(r))).toBe(true);
418+
},
419+
);
420+
421+
it.each(ROWS)("ONE TEXT: $name gives `ActionSchema`'s own refusal of the pairing, after the subject", (row) => {
422+
const rowMessage = issuesOf(updateRow(row, 'api', { onSuccess: HOP }))[0].message;
423+
expect(rowMessage.slice(updateSubject(row).length))
424+
.toBe(actionUpdateOnSuccessMessage().slice(ACTION_UPDATE_SUBJECT.length));
425+
});
426+
});

0 commit comments

Comments
 (0)