Remove included_file_schema.json and consolidate to single main workflow schema#10484
Conversation
- Added compiler-level validation to check for forbidden fields in shared workflows - Updated ValidateIncludedFileFrontmatterWithSchema to use main_workflow_schema - Removed included_file_schema.json and all references to it - Updated test expectations to match new validation behavior - All tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…dation - Shared workflows now validate against main schema with temporary 'on' field - This allows catching unknown fields while still enforcing forbidden field restrictions - All tests pass including TestSharedWorkflowWithInvalidFields Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot jobs should be allowed and imported |
Removed 'jobs' from sharedWorkflowForbiddenFields to allow shared workflows to define custom GitHub Actions jobs that can be imported by main workflows. This enables modular workflow design where job definitions can be reused. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review forbidden field and check if they are supported in code |
|
@copilot For every forbidden field, add a negative test case. For every allowed field, add a test ensuring it is imported. |
Added schema_validation_test.go with: - TestForbiddenFieldsInSharedWorkflows: Verifies all 24 forbidden fields are rejected - TestAllowedFieldsInSharedWorkflows: Verifies all 15 allowed fields work correctly All fields have been verified to be: - Present in main_workflow_schema.json - Actively used in the codebase - Properly restricted or allowed as intended All tests pass. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed all 24 forbidden fields - all are verified and properly supported in code: Verified:
Added comprehensive tests (commit 4e4c0e6):
All tests pass. No changes needed to the forbidden fields list. |
Added forbidden_fields_import_test.go with: - TestForbiddenFieldsImportRejection: Tests that 23 forbidden fields are rejected when imported (23 tests) - TestAllowedFieldsImportSuccess: Tests that 15 allowed fields can be successfully imported (15 tests) - TestImportsFieldForbiddenInSharedWorkflows: Separate test for "imports" field (edge case) Total: 39 new integration tests verifying field restrictions work correctly in import scenarios. All tests pass. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Added comprehensive tests (commit b695aff): Negative test cases for forbidden fields:
Import tests for allowed fields:
Total: 39 new integration tests. Combined with existing parser-level tests, we now have 78 tests covering field validation and import scenarios. All tests pass. |
|
@copilot add support for post-steps, bots to be included (merge bots in set, append post-steps) |
- Removed 'bots' and 'post-steps' from forbidden fields list - Added extractBotsFromContent() to extract and merge bots (union of bot names) - Added extractPostStepsFromContent() to extract and append post-steps - Updated ImportsResult struct with MergedBots and MergedPostSteps fields - Bots are merged as a set (no duplicates) across all imports - Post-steps are appended in import order - Updated all tests to reflect new allowed fields (now 17 allowed fields) - All 80 tests pass (22 forbidden + 17 allowed fields × 2 test levels) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Added support for importing Bots: Merged as a union set across all imports (no duplicates) Implementation:
|
|
@copilot imports and labels should be importable |
- Removed 'imports' and 'labels' from forbidden fields list - Added extractLabelsFromContent() to extract and merge labels (union of label names) - Labels are merged as a set (no duplicates) across all imports - Imports in shared workflows enable nested imports (already supported in code) - Updated ImportsResult struct with MergedLabels field - Updated all tests to reflect new allowed fields (now 19 allowed fields) - Replaced TestImportsFieldForbiddenInSharedWorkflows with TestImportsFieldAllowedInSharedWorkflows - All 78 tests pass (20 forbidden + 19 allowed fields × 2 test levels) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Added support for importing Imports: Enable nested imports - shared workflows can import other shared workflows (already supported in code, now allowed in schema) Implementation:
|
|
@copilot allow "cache", add to list of caches |
- Removed 'cache' from forbidden fields list - Added extractCacheFromContent() to extract cache configurations - Cache configurations are appended to a list across all imports - Updated ImportsResult struct with MergedCaches field - Updated all tests to reflect new allowed fields (now 20 allowed fields) - All 79 tests pass (19 forbidden + 20 allowed fields × 2 test levels) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Added support for importing Cache: Cache configurations are appended to a list across all imports Implementation:
|
|
@copilot maintain list of forbidden field in constants.go so that it can be mined by agents easily. Based on that list, generate missing tests for each field. Run tests and fix failures |
- Moved SharedWorkflowForbiddenFields from schema_validation.go to constants.go - Added comprehensive documentation for the forbidden fields list - Updated schema_validation.go to use constants.SharedWorkflowForbiddenFields - Updated tests to use the constant from constants package - Tests dynamically iterate over the constant to ensure completeness - All 79 tests pass This enables agents and automated tools to easily discover and mine the forbidden fields list. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Moved forbidden fields list to constants.go and updated tests (commit ca35234): Implementation:
Benefits for agents:
Test results:
|
|
@copilot included workflows also support source already |
- Removed 'source' from SharedWorkflowForbiddenFields constant - Added 'source' to allowed fields tests in both parser and workflow tests - Source is a metadata field indicating where workflow was added from - It's rendered as a comment in the lock file and doesn't affect execution - All 80 tests pass (18 forbidden + 21 allowed fields × 2 test levels) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Removed Source field is now allowed in shared workflows:
Implementation:
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
GitHub MCP: ✅ (PRs: Consolidate release artifacts, simplify release_tag usage, fix release creation order, and modernize with JavaScript; Fix daily firewall report caching stale aggregated data)
|
|
Smoke Test Results Last 2 merged PRs:
✅ GitHub MCP (search_pull_requests) Status: PASS
|
Smoke Test ResultsLast 2 Merged PRs:
Tests:
Overall: PARTIAL PASS (4/5 tests)
|
The codebase maintained two JSON schemas:
main_workflow_schema.jsonfor main workflows andincluded_file_schema.jsonfor shared/imported workflows. This caused duplication and consistency issues.Changes
Schema consolidation:
pkg/parser/schemas/included_file_schema.json(1,547 lines)main_workflow_schema.jsonCompiler-level enforcement:
validateSharedWorkflowFields()to check for forbidden fields in shared workflowson,command,concurrency,container,env,environment,features,github-token,if,name,roles,run-name,runs-on,sandbox,strict,timeout-minutes,timeout_minutes,tracker-idpkg/constants/constants.goasSharedWorkflowForbiddenFieldsfor easy mining by agents and automated toolsValidation approach:
This catches both forbidden fields (main-workflow-only) and unknown fields (typos/invalid) while maintaining a single schema source.
Shared workflow capabilities:
jobsthat can be imported and reused across multiple workflowsbotsthat are merged as a union set (no duplicates) across all importspost-stepsthat are appended in import order for sequential executionlabelsthat are merged as a union set (no duplicates) across all imports for workflow categorizationimportsto import other shared workflows, enabling nested/hierarchical imports and modular compositioncacheconfigurations that are appended to a list across all importssourceattribution metadata indicating where the workflow was added fromtools,engine,network,mcp-servers,permissions,runtimes,safe-outputs,safe-inputs,services,steps,secret-masking,jobs,bots,post-steps,labels,imports,cache,source,description,metadata,inputsImport behavior:
extractBotsFromContent(),extractPostStepsFromContent(),extractLabelsFromContent(), andextractCacheFromContent()functionsImportsResultstruct withMergedBots,MergedPostSteps,MergedLabels, andMergedCachesfieldsTest coverage:
pkg/parser/schema_validation_test.gowith parser-level validation tests:TestForbiddenFieldsInSharedWorkflows: Dynamically usesconstants.SharedWorkflowForbiddenFieldsto verify all 18 forbidden fields are properly rejected (18 tests)TestAllowedFieldsInSharedWorkflows: Verifies all 21 allowed fields work correctly (21 tests)pkg/workflow/forbidden_fields_import_test.gowith workflow-level import integration tests:TestForbiddenFieldsImportRejection: Dynamically usesconstants.SharedWorkflowForbiddenFieldsto test that 18 forbidden fields are rejected during import compilation (18 tests)TestAllowedFieldsImportSuccess: Tests that 20 allowed fields can be successfully imported (20 tests)TestImportsFieldAllowedInSharedWorkflows: Tests nested imports work correctly (1 test)Code cleanup:
includedFileSchemavariable, cache, and getter fromschema_compiler.goOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Changeset
included_file_schema.jsonand usingmain_workflow_schema.jsonfor both main and shared workflows. Added compiler-level validation to reject fields that are only allowed in main workflows when used in shared workflows. The forbidden fields list is centralized inpkg/constants/constants.goasSharedWorkflowForbiddenFields.