Skip to content

Commit 8bce1ce

Browse files
authored
Merge pull request #834 from objectstack-ai/copilot/fix-action-metadata-binding
2 parents d01eb0d + e102ca4 commit 8bce1ce

8 files changed

Lines changed: 268 additions & 14 deletions

File tree

examples/app-crm/src/actions/contact.actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const MarkPrimaryContactAction: Action = {
88
label: 'Mark as Primary Contact',
99
icon: 'star',
1010
type: 'script',
11-
execute: 'markAsPrimaryContact',
11+
target: 'markAsPrimaryContact',
1212
locations: ['record_header', 'list_item'],
1313
visible: 'is_primary = false',
1414
confirmText: 'Mark this contact as the primary contact for the account?',

examples/app-crm/src/actions/global.actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const ExportToCsvAction: Action = {
4040
label: 'Export to CSV',
4141
icon: 'download',
4242
type: 'script',
43-
execute: 'exportToCSV',
43+
target: 'exportToCSV',
4444
locations: ['list_toolbar'],
4545
successMessage: 'Export completed!',
4646
refreshAfter: false,

examples/app-crm/src/actions/opportunity.actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const CloneOpportunityAction: Action = {
88
label: 'Clone Opportunity',
99
icon: 'copy',
1010
type: 'script',
11-
execute: 'cloneRecord',
11+
target: 'cloneRecord',
1212
locations: ['record_header', 'record_more'],
1313
successMessage: 'Opportunity cloned successfully!',
1414
refreshAfter: true,

examples/app-todo/src/actions/task.actions.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const CompleteTaskAction: Action = {
88
label: 'Mark Complete',
99
icon: 'check-circle',
1010
type: 'script',
11-
execute: 'completeTask',
11+
target: 'completeTask',
1212
locations: ['record_header', 'list_item'],
1313
successMessage: 'Task marked as complete!',
1414
refreshAfter: true,
@@ -20,7 +20,7 @@ export const StartTaskAction: Action = {
2020
label: 'Start Task',
2121
icon: 'play-circle',
2222
type: 'script',
23-
execute: 'startTask',
23+
target: 'startTask',
2424
locations: ['record_header', 'list_item'],
2525
successMessage: 'Task started!',
2626
refreshAfter: true,
@@ -78,7 +78,7 @@ export const CloneTaskAction: Action = {
7878
label: 'Clone Task',
7979
icon: 'copy',
8080
type: 'script',
81-
execute: 'cloneTask',
81+
target: 'cloneTask',
8282
locations: ['record_header'],
8383
successMessage: 'Task cloned successfully!',
8484
refreshAfter: true,
@@ -90,7 +90,7 @@ export const MassCompleteTasksAction: Action = {
9090
label: 'Complete Selected',
9191
icon: 'check-square',
9292
type: 'script',
93-
execute: 'massCompleteTasks',
93+
target: 'massCompleteTasks',
9494
locations: ['list_toolbar'],
9595
successMessage: 'Selected tasks marked as complete!',
9696
refreshAfter: true,
@@ -102,7 +102,7 @@ export const DeleteCompletedAction: Action = {
102102
label: 'Delete Completed',
103103
icon: 'trash-2',
104104
type: 'script',
105-
execute: 'deleteCompletedTasks',
105+
target: 'deleteCompletedTasks',
106106
locations: ['list_toolbar'],
107107
successMessage: 'Completed tasks deleted!',
108108
refreshAfter: true,
@@ -114,7 +114,7 @@ export const ExportToCsvAction: Action = {
114114
label: 'Export to CSV',
115115
icon: 'download',
116116
type: 'script',
117-
execute: 'exportTasksToCSV',
117+
target: 'exportTasksToCSV',
118118
locations: ['list_toolbar'],
119119
successMessage: 'Export completed!',
120120
refreshAfter: false,

packages/spec/src/stack.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,82 @@ describe('defineStack - Navigation Cross-Reference Validation', () => {
961961
});
962962
});
963963

964+
// ============================================================================
965+
// Action Cross-Reference Validation — ensures action targets resolve
966+
// ============================================================================
967+
968+
describe('defineStack - Action Cross-Reference Validation', () => {
969+
const baseManifest = {
970+
id: 'com.example.test',
971+
name: 'test-project',
972+
version: '1.0.0',
973+
type: 'app' as const,
974+
};
975+
976+
it('should detect action referencing undefined flow', () => {
977+
const config = {
978+
manifest: baseManifest,
979+
objects: [
980+
{ name: 'task', label: 'Task', fields: { title: { type: 'text' as const } } },
981+
],
982+
flows: [
983+
{ name: 'existing_flow', label: 'Existing Flow', type: 'autolaunched' as const, nodes: [], edges: [] },
984+
],
985+
actions: [
986+
{ name: 'run_flow', label: 'Run Flow', type: 'flow' as const, target: 'nonexistent_flow' },
987+
],
988+
};
989+
990+
expect(() => defineStack(config)).toThrow('cross-reference validation failed');
991+
expect(() => defineStack(config)).toThrow('nonexistent_flow');
992+
});
993+
994+
it('should pass when action references a defined flow', () => {
995+
const config = {
996+
manifest: baseManifest,
997+
objects: [
998+
{ name: 'task', label: 'Task', fields: { title: { type: 'text' as const } } },
999+
],
1000+
flows: [
1001+
{ name: 'approval_flow', label: 'Approval Flow', type: 'autolaunched' as const, nodes: [], edges: [] },
1002+
],
1003+
actions: [
1004+
{ name: 'run_approval', label: 'Run Approval', type: 'flow' as const, target: 'approval_flow' },
1005+
],
1006+
};
1007+
1008+
expect(() => defineStack(config)).not.toThrow();
1009+
});
1010+
1011+
it('should skip action flow validation when no flows are defined', () => {
1012+
const config = {
1013+
manifest: baseManifest,
1014+
objects: [
1015+
{ name: 'task', label: 'Task', fields: { title: { type: 'text' as const } } },
1016+
],
1017+
actions: [
1018+
{ name: 'run_flow', label: 'Run Flow', type: 'flow' as const, target: 'some_flow' },
1019+
],
1020+
};
1021+
1022+
expect(() => defineStack(config)).not.toThrow();
1023+
});
1024+
1025+
it('should accept script actions without cross-reference validation', () => {
1026+
const config = {
1027+
manifest: baseManifest,
1028+
objects: [
1029+
{ name: 'task', label: 'Task', fields: { title: { type: 'text' as const } } },
1030+
],
1031+
actions: [
1032+
{ name: 'approve_task', label: 'Approve', type: 'script' as const, target: 'approveTask' },
1033+
],
1034+
};
1035+
1036+
expect(() => defineStack(config)).not.toThrow();
1037+
});
1038+
});
1039+
9641040
// ============================================================================
9651041
// Example-Level Strict Validation — mirrors examples/app-todo & examples/app-crm
9661042
// ============================================================================

packages/spec/src/stack.zod.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,28 @@ function validateCrossReferences(config: ObjectStackDefinition): string[] {
400400
}
401401
}
402402

403+
// Validate action → flow/target cross-references
404+
// Note: When no flows are defined (flowNames.size === 0), flow-type action targets
405+
// are not validated because the referenced flow may be provided by a plugin.
406+
// This is consistent with dashboard/page/report validation in navigation.
407+
if (config.actions) {
408+
const flowNames = new Set<string>();
409+
if (config.flows) {
410+
for (const flow of config.flows) {
411+
flowNames.add(flow.name);
412+
}
413+
}
414+
415+
for (const action of config.actions) {
416+
// Validate flow-type actions reference a defined flow
417+
if (action.type === 'flow' && action.target && flowNames.size > 0 && !flowNames.has(action.target)) {
418+
errors.push(
419+
`Action '${action.name}' references flow '${action.target}' which is not defined in flows.`,
420+
);
421+
}
422+
}
423+
}
424+
403425
return errors;
404426
}
405427

packages/spec/src/ui/action.test.ts

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,39 @@ describe('ActionSchema', () => {
7777
});
7878

7979
describe('Action Types', () => {
80-
it('should accept all action types', () => {
80+
it('should accept all action types with target', () => {
8181
const types = ['script', 'url', 'modal', 'flow', 'api'] as const;
8282

8383
types.forEach(type => {
8484
const action: ActionType = {
8585
name: 'test_action',
8686
label: 'Test',
8787
type,
88+
target: 'test_handler',
8889
};
8990
expect(() => ActionSchema.parse(action)).not.toThrow();
9091
});
9192
});
9293

94+
it('should accept script type without target', () => {
95+
expect(() => ActionSchema.parse({
96+
name: 'test_action',
97+
label: 'Test',
98+
type: 'script',
99+
})).not.toThrow();
100+
});
101+
102+
it('should reject url/flow/modal/api types without target', () => {
103+
const targetRequiredTypes = ['url', 'flow', 'modal', 'api'] as const;
104+
targetRequiredTypes.forEach(type => {
105+
expect(() => ActionSchema.parse({
106+
name: 'test_action',
107+
label: 'Test',
108+
type,
109+
})).toThrow(/target/);
110+
});
111+
});
112+
93113
it('should default to script type', () => {
94114
const action = {
95115
name: 'custom_action',
@@ -294,6 +314,7 @@ describe('ActionSchema', () => {
294314
label: 'Transfer Case',
295315
icon: 'arrow-right',
296316
type: 'modal',
317+
target: 'transfer_case_modal',
297318
locations: ['record_more'],
298319
params: [
299320
{
@@ -561,3 +582,89 @@ describe('ActionSchema - variant', () => {
561582
expect(result.confirmText).toBe('Are you sure?');
562583
});
563584
});
585+
586+
// ============================================================================
587+
// Protocol Improvement Tests: execute → target migration & target validation
588+
// ============================================================================
589+
590+
describe('ActionSchema - execute → target migration', () => {
591+
it('should auto-migrate execute to target when target is not set', () => {
592+
const result = ActionSchema.parse({
593+
name: 'legacy_action',
594+
label: 'Legacy',
595+
type: 'script',
596+
execute: 'legacyHandler',
597+
});
598+
expect(result.target).toBe('legacyHandler');
599+
});
600+
601+
it('should preserve target over execute when both are set', () => {
602+
const result = ActionSchema.parse({
603+
name: 'both_fields',
604+
label: 'Both',
605+
type: 'script',
606+
target: 'preferredHandler',
607+
execute: 'legacyHandler',
608+
});
609+
expect(result.target).toBe('preferredHandler');
610+
});
611+
612+
it('should allow script type without target or execute', () => {
613+
expect(() => ActionSchema.parse({
614+
name: 'inline_script',
615+
label: 'Inline',
616+
type: 'script',
617+
})).not.toThrow();
618+
});
619+
});
620+
621+
describe('ActionSchema - target required for non-script types', () => {
622+
it('should require target for url type', () => {
623+
expect(() => ActionSchema.parse({
624+
name: 'url_action',
625+
label: 'Open URL',
626+
type: 'url',
627+
})).toThrow(/target/);
628+
});
629+
630+
it('should require target for flow type', () => {
631+
expect(() => ActionSchema.parse({
632+
name: 'flow_action',
633+
label: 'Run Flow',
634+
type: 'flow',
635+
})).toThrow(/target/);
636+
});
637+
638+
it('should require target for modal type', () => {
639+
expect(() => ActionSchema.parse({
640+
name: 'modal_action',
641+
label: 'Open Modal',
642+
type: 'modal',
643+
})).toThrow(/target/);
644+
});
645+
646+
it('should require target for api type', () => {
647+
expect(() => ActionSchema.parse({
648+
name: 'api_action',
649+
label: 'Call API',
650+
type: 'api',
651+
})).toThrow(/target/);
652+
});
653+
654+
it('should accept non-script types when target is provided', () => {
655+
expect(() => ActionSchema.parse({ name: 'url_ok', label: 'URL', type: 'url', target: 'https://example.com' })).not.toThrow();
656+
expect(() => ActionSchema.parse({ name: 'flow_ok', label: 'Flow', type: 'flow', target: 'my_flow' })).not.toThrow();
657+
expect(() => ActionSchema.parse({ name: 'modal_ok', label: 'Modal', type: 'modal', target: 'my_modal' })).not.toThrow();
658+
expect(() => ActionSchema.parse({ name: 'api_ok', label: 'API', type: 'api', target: '/api/endpoint' })).not.toThrow();
659+
});
660+
661+
it('should accept non-script types when execute is provided (auto-migrated)', () => {
662+
const result = ActionSchema.parse({
663+
name: 'flow_legacy',
664+
label: 'Flow Legacy',
665+
type: 'flow',
666+
execute: 'my_flow',
667+
});
668+
expect(result.target).toBe('my_flow');
669+
});
670+
});

0 commit comments

Comments
 (0)