Uh oh!
There was an error while loading. Please reload this page.
Harden UI schema type safety, unify filter format, extend cross-reference validation, add negative tests - #802
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…d data and navigation, add negative validation tests Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
CopilotAI
changed the title
[WIP] Enhance spec schema robustness and consistencyHarden UI schema type safety, extend cross-reference validation, add negative testsFeb 24, 2026
hotlong
commented
Feb 24, 2026
Contributor
@copilot 请仔细确认，已经完成了任务中要求的所有开发 |
…add example-level strict validation tests Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
ContributorAuthor
已逐项核对 issue 中的所有要求，补充提交 1. defineStack() 默认严格校验 ✅
2. SSOT 原则 ✅
3. z.any/z.unknown 替换 ✅
4. Filter 格式统一 ✅
5. 测试体系 ✅ (6,456 tests)
6. CI/Lint 🔴 未完成
以上两项已在 ROADMAP Hardening Status 表中标注为 Planned。 |
CopilotAI
changed the title
Harden UI schema type safety, extend cross-reference validation, add negative testsHarden UI schema type safety, unify filter format, extend cross-reference validation, add negative testsFeb 24, 2026
hotlong
marked this pull request as ready for review
February 24, 2026 08:13
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 significantly improves the type safety and robustness of the ObjectStack spec protocol by eliminating loose typing, unifying filter formats, extending cross-reference validation, and adding comprehensive negative validation tests.
Changes:
- Eliminated all 7 instances of
z.any()in UI schemas (dashboard, page, component) by introducing typed unions for value fields - Introduced
ViewFilterRuleSchemaas the standard typed format for declarative view/tab/page filters, replacing allz.array(z.unknown())filter fields - Extended
defineStack()cross-reference validation to cover seed data object references and app navigation references (objects, dashboards, pages, reports) with recursive group support - Added 44+ negative validation tests covering missing required fields, invalid enums, type violations, and cross-reference errors across dashboard, page, report, view, and stack schemas
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/spec/src/ui/view.zod.ts | Introduces ViewFilterRuleSchema with typed field/operator/value structure; replaces z.array(z.unknown()) in ListViewSchema.filter, ViewTabSchema.filter, and quickFilters value |
| packages/spec/src/ui/view.test.ts | Adds ViewFilterRuleSchema validation tests (positive and negative) and ListView filter type tests |
| packages/spec/src/ui/page.zod.ts | Replaces z.any() with FilterConditionSchema in ElementDataSourceSchema and RecordReviewConfigSchema; uses ViewFilterRuleSchema for InterfacePageConfigSchema.filterBy; types review action values |
| packages/spec/src/ui/page.test.ts | Adds 77 lines of negative validation tests for PageSchema, PageComponentSchema, and RecordReviewConfigSchema |
| packages/spec/src/ui/dashboard.zod.ts | Replaces z.any() with typed unions in GlobalFilterSchema options.value and defaultValue fields |
| packages/spec/src/ui/dashboard.test.ts | Adds 88 lines of negative validation tests for DashboardWidgetSchema, DashboardSchema, and GlobalFilterSchema |
| packages/spec/src/ui/component.zod.ts | Replaces z.array(z.unknown()) with ViewFilterRuleSchema in RecordRelatedListProps.filter; replaces z.any() with FilterConditionSchema in ElementNumberPropsSchema and ElementRecordPickerPropsSchema |
| packages/spec/src/stack.zod.ts | Extends validateCrossReferences() to validate seed data object references and app navigation references (objects/dashboards/pages/reports) with recursive group children support |
| packages/spec/src/stack.test.ts | Adds 286 lines including seed data and navigation cross-reference validation tests, plus example-level strict validation tests mirroring app-todo and app-crm patterns |
| packages/spec/src/ui/report.test.ts | Adds 63 lines of negative validation tests for ReportSchema and ReportColumnSchema |
| ROADMAP.md | Updates test metrics (229 files, 6,456 tests) and adds expanded Spec Protocol Hardening Status table documenting z.any() elimination, filter unification, and SSOT compliance |
Comment on lines
+378
to
+388
| if (nav.type === 'dashboard' && typeof nav.dashboardName === 'string' && dashboardNames.size > 0 && !dashboardNames.has(nav.dashboardName)) { | ||
| errors.push( | ||
| `App '${appName}' navigation references dashboard '${nav.dashboardName}' which is not defined in dashboards.`, | ||
| ); | ||
| } | ||
| if (nav.type === 'page' && typeof nav.pageName === 'string' && pageNames.size > 0 && !pageNames.has(nav.pageName)) { | ||
| errors.push( | ||
| `App '${appName}' navigation references page '${nav.pageName}' which is not defined in pages.`, | ||
| ); | ||
| } | ||
| if (nav.type === 'report' && typeof nav.reportName === 'string' && reportNames.size > 0 && !reportNames.has(nav.reportName)) { |
There was a problem hiding this comment.
The cross-reference validation for navigation items only validates dashboard/page/report references when their respective collections have at least one item (dashboardNames.size > 0, pageNames.size > 0, reportNames.size > 0). This means if an app navigation references a dashboard but no dashboards are defined at all, the validation will not catch this error.
Consider removing the size checks so that any reference to a dashboard/page/report is validated, regardless of whether those collections are empty or not. This would make the validation more consistent with how object references are validated (which don't have this size check on line 373).
Suggested change
| if(nav.type==='dashboard'&&typeofnav.dashboardName==='string'&&dashboardNames.size>0&&!dashboardNames.has(nav.dashboardName)){ | |
| errors.push( | |
| `App '${appName}' navigation references dashboard '${nav.dashboardName}' which is not defined in dashboards.`, | |
| ); | |
| } | |
| if(nav.type==='page'&&typeofnav.pageName==='string'&&pageNames.size>0&&!pageNames.has(nav.pageName)){ | |
| errors.push( | |
| `App '${appName}' navigation references page '${nav.pageName}' which is not defined in pages.`, | |
| ); | |
| } | |
| if(nav.type==='report'&&typeofnav.reportName==='string'&&reportNames.size>0&&!reportNames.has(nav.reportName)){ | |
| if(nav.type==='dashboard'&&typeofnav.dashboardName==='string'&&!dashboardNames.has(nav.dashboardName)){ | |
| errors.push( | |
| `App '${appName}' navigation references dashboard '${nav.dashboardName}' which is not defined in dashboards.`, | |
| ); | |
| } | |
| if(nav.type==='page'&&typeofnav.pageName==='string'&&!pageNames.has(nav.pageName)){ | |
| errors.push( | |
| `App '${appName}' navigation references page '${nav.pageName}' which is not defined in pages.`, | |
| ); | |
| } | |
| if(nav.type==='report'&&typeofnav.reportName==='string'&&!reportNames.has(nav.reportName)){ |
| }; | ||
| expect(() => defineStack(config)).toThrow('ghost_dashboard'); | ||
| }); | ||
There was a problem hiding this comment.
The negative test for dashboard reference validation should include a case where no dashboards are defined at all (dashboards array is empty or undefined), but navigation references a dashboard. This would verify that the validation correctly catches dangling references even when the target collection is empty.
Example test case:
it('should detect navigation referencing dashboard when no dashboards are defined',()=>{constconfig={manifest: baseManifest,objects: [{name: 'task',fields: {title: {type: 'text'}}}],// No dashboards definedapps: [{name: 'my_app',label: 'My App',navigation: [{id: 'nav_ghost',type: 'dashboard'asconst,label: 'Missing',dashboardName: 'ghost_dashboard'},],}],};expect(()=>defineStack(config)).toThrow('ghost_dashboard');});Similar tests should be added for page and report references as well.
Suggested change
| it('should detect navigation referencing dashboard when no dashboards are defined (dashboards undefined)',()=>{ | |
| constconfig={ | |
| manifest: baseManifest, | |
| objects: [ | |
| {name: 'task',fields: {title: {type: 'text'}}}, | |
| ], | |
| // dashboards property intentionally omitted | |
| apps: [ | |
| { | |
| name: 'my_app', | |
| label: 'My App', | |
| navigation: [ | |
| { | |
| id: 'nav_ghost_no_dashboards', | |
| type: 'dashboard'asconst, | |
| label: 'Missing', | |
| dashboardName: 'ghost_dashboard', | |
| }, | |
| ], | |
| }, | |
| ], | |
| }; | |
| expect(()=>defineStack(config)).toThrow('ghost_dashboard'); | |
| }); | |
| it('should detect navigation referencing dashboard when dashboards array is empty',()=>{ | |
| constconfig={ | |
| manifest: baseManifest, | |
| objects: [ | |
| {name: 'task',fields: {title: {type: 'text'}}}, | |
| ], | |
| dashboards: [], | |
| apps: [ | |
| { | |
| name: 'my_app', | |
| label: 'My App', | |
| navigation: [ | |
| { | |
| id: 'nav_ghost_empty_dashboards', | |
| type: 'dashboard'asconst, | |
| label: 'Missing', | |
| dashboardName: 'ghost_dashboard', | |
| }, | |
| ], | |
| }, | |
| ], | |
| }; | |
| expect(()=>defineStack(config)).toThrow('ghost_dashboard'); | |
| }); | |
| it('should detect navigation referencing page when no pages are defined',()=>{ | |
| constconfig={ | |
| manifest: baseManifest, | |
| objects: [ | |
| {name: 'task',fields: {title: {type: 'text'}}}, | |
| ], | |
| // pages property intentionally omitted | |
| apps: [ | |
| { | |
| name: 'my_app', | |
| label: 'My App', | |
| navigation: [ | |
| { | |
| id: 'nav_page_ghost', | |
| type: 'page'asconst, | |
| label: 'Missing', | |
| pageName: 'ghost_page', | |
| }, | |
| ], | |
| }, | |
| ], | |
| }; | |
| expect(()=>defineStack(config)).toThrow('ghost_page'); | |
| }); | |
| it('should detect navigation referencing report when no reports are defined',()=>{ | |
| constconfig={ | |
| manifest: baseManifest, | |
| objects: [ | |
| {name: 'task',fields: {title: {type: 'text'}}}, | |
| ], | |
| // reports property intentionally omitted | |
| apps: [ | |
| { | |
| name: 'my_app', | |
| label: 'My App', | |
| navigation: [ | |
| { | |
| id: 'nav_report_ghost', | |
| type: 'report'asconst, | |
| label: 'Missing', | |
| reportName: 'ghost_report', | |
| }, | |
| ], | |
| }, | |
| ], | |
| }; | |
| expect(()=>defineStack(config)).toThrow('ghost_report'); | |
| }); |
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.
Spec protocol had loose typing (
z.any()/z.array(z.unknown())) in UI schemas and incomplete cross-reference validation indefineStack(), allowing invalid metadata to pass silently.z.any()→ typed schemas (7 instances across 3 files)GlobalFilterSchemaoptionsvalue/defaultValue→z.union([z.string(), z.number(), z.boolean()])ElementDataSourceSchema.filter,RecordReviewConfigSchema.filter→FilterConditionSchema; review actionvalue→ typed unionElementNumberPropsSchema.filter,ElementRecordPickerPropsSchema.filter→FilterConditionSchemaZero
z.any()remaining in all 19 UI protocol files.Filter format unification —
ViewFilterRuleSchemaCreated
ViewFilterRuleSchema({ field, operator, value }) as the standard typed schema for declarative view/tab/page filters. Replaced allz.array(z.unknown())filter fields:ListViewSchema.filter,ViewTabSchema.filter,quickFilters[].valueInterfacePageConfigSchema.filterByRecordRelatedListProps.filterTwo unified filter conventions now exist:
FilterConditionSchema— MongoDB-style object filters for data queriesViewFilterRuleSchema— Declarative array-of-objects filters for UI views/tabsCross-reference validation (
stack.zod.ts)data[].objectvalidated against defined objectsobject,dashboard,page,report) validated against defined metadata, recursive into group childrenNegative validation tests (+44)
Covers missing required fields, invalid enum values, incomplete layout, camelCase name rejection, and cross-reference violations for
DashboardWidgetSchema,DashboardSchema,GlobalFilterSchema,PageSchema,PageComponentSchema,RecordReviewConfigSchema,ReportSchema,ViewFilterRuleSchema,ListViewSchema.filter, anddefineStack.Example-level strict validation tests
Added full-stack config validation tests mirroring
examples/app-todoandexamples/app-crmpatterns — Todo-style and CRM-style configs with objects, seed data, dashboards, reports, and navigation all validated in strict mode.SSOT compliance
All 135 UI types derived via
z.infer<typeof ...>, zero duplicate interfaces in.zod.tsfiles. Remainingz.unknown()instances are justified extensibility points (properties,children,context,body,options).ROADMAP.md
Updated test metrics (6,456 tests) and added expanded Spec Protocol Hardening Status table.
Original 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.