Uh oh!
There was an error while loading. Please reload this page.
Action target binding enforcement and execute→target migration - #834
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ss-reference checks - ActionSchema: Add .refine() requiring target for url/flow/modal/api types - ActionSchema: Add .transform() auto-migrating deprecated execute→target - defineStack: Add action→flow cross-reference validation - Examples: Migrate all execute usages to target in app-todo and app-crm - Tests: Add 15 new tests covering target validation, migration, and cross-refs Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
… from ActionType, add docs for skip behavior Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
CopilotAI
changed the title
[WIP] Fix consistency of action metadata with handler validationAction target binding enforcement and execute→target migrationFeb 28, 2026
hotlong
commented
Feb 28, 2026
Contributor
@copilot 仔细评估是否完成了任务中的所有要求，并且example能跑通 |
…ld error The Set narrowed type Exclude<ActionType, 'script'> was incompatible with Set.has() receiving the full ActionType union in strict TypeScript DTS emit. Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
ContributorAuthor
Completed a thorough audit. Here's the status against the issue checklist: 1. Schema ✅
2. Cross-reference validation ✅
3. Examples ✅
4. Tests ✅
|
hotlong
marked this pull request as ready for review
March 1, 2026 02:57
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the ActionSchema contract in packages/spec by enforcing canonical target binding (with backward-compatible execute → target migration) and adds strict cross-reference validation for flow-type action targets via defineStack().
Changes:
- Introduces
ActionType, migrates legacyexecutetotarget, and enforcestargetfor non-scriptaction types via schema refinement. - Adds
defineStackcross-reference validation foraction.type === 'flow'to ensureaction.targetpoints to an existing flow (when flows are present). - Updates examples and expands unit tests to cover migration and validation behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/spec/src/ui/action.zod.ts | Adds ActionType, execute → target migration, and target required enforcement for non-script action types. |
| packages/spec/src/ui/action.test.ts | Adds tests for target enforcement and execute→target migration; updates an existing modal example to include target. |
| packages/spec/src/stack.zod.ts | Adds cross-reference validation for flow actions to ensure target resolves to a defined flow. |
| packages/spec/src/stack.test.ts | Adds tests covering action→flow cross-reference validation behavior (including “skip when no flows defined”). |
| examples/app-todo/src/actions/task.actions.ts | Migrates example actions from execute: to target:. |
| examples/app-crm/src/actions/*.actions.ts | Migrates CRM example actions from execute: to target:. |
Comments suppressed due to low confidence (2)
packages/spec/src/ui/action.test.ts:670
ActionSchema - target required for non-script typeslargely duplicates assertions already covered earlier indescribe('Action Types')(script without target, and url/flow/modal/api rejection without target, plus acceptance with target). Consider consolidating into a single suite to avoid redundant coverage and future drift when action type rules change.
describe('ActionSchema - target required for non-script types', () => {
it('should require target for url type', () => {
expect(() => ActionSchema.parse({
name: 'url_action',
label: 'Open URL',
type: 'url',
})).toThrow(/target/);
});
it('should require target for flow type', () => {
expect(() => ActionSchema.parse({
name: 'flow_action',
label: 'Run Flow',
type: 'flow',
})).toThrow(/target/);
});
it('should require target for modal type', () => {
expect(() => ActionSchema.parse({
name: 'modal_action',
label: 'Open Modal',
type: 'modal',
})).toThrow(/target/);
});
it('should require target for api type', () => {
expect(() => ActionSchema.parse({
name: 'api_action',
label: 'Call API',
type: 'api',
})).toThrow(/target/);
});
it('should accept non-script types when target is provided', () => {
expect(() => ActionSchema.parse({ name: 'url_ok', label: 'URL', type: 'url', target: 'https://example.com' })).not.toThrow();
expect(() => ActionSchema.parse({ name: 'flow_ok', label: 'Flow', type: 'flow', target: 'my_flow' })).not.toThrow();
expect(() => ActionSchema.parse({ name: 'modal_ok', label: 'Modal', type: 'modal', target: 'my_modal' })).not.toThrow();
expect(() => ActionSchema.parse({ name: 'api_ok', label: 'API', type: 'api', target: '/api/endpoint' })).not.toThrow();
});
it('should accept non-script types when execute is provided (auto-migrated)', () => {
const result = ActionSchema.parse({
name: 'flow_legacy',
label: 'Flow Legacy',
type: 'flow',
execute: 'my_flow',
});
expect(result.target).toBe('my_flow');
});
});
packages/spec/src/stack.zod.ts:423
- This new action→flow cross-reference validation is unreachable when
objectsis omitted/empty becausevalidateCrossReferences()returns early whenobjectNames.size === 0. If a stack definesflows+actionsbut noobjects, strict mode will skip action target validation entirely. Consider moving the action→flow block before the early return, or changing the early return into conditional guards around only the object-dependent validations.
// Validate action → flow/target cross-references
// Note: When no flows are defined (flowNames.size === 0), flow-type action targets
// are not validated because the referenced flow may be provided by a plugin.
// This is consistent with dashboard/page/report validation in navigation.
if (config.actions) {
const flowNames = new Set<string>();
if (config.flows) {
for (const flow of config.flows) {
flowNames.add(flow.name);
}
}
for (const action of config.actions) {
// Validate flow-type actions reference a defined flow
if (action.type === 'flow' && action.target && flowNames.size > 0 && !flowNames.has(action.target)) {
errors.push(
`Action '${action.name}' references flow '${action.target}' which is not defined in flows.`,
);
}
}
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.refine()requirestargetforurl,flow,modal,apitypes;.transform()auto-migrates deprecatedexecute→targetvalidateCrossReferences()checks action→flow referencesexecute:→target:in app-todo and app-crmpnpm buildpasses (DTS, ESM, CJS) — fixed TS strict type error in TARGET_REQUIRED_TYPESOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.