Fix declarative engine version validation

[pelikhan]

Overview

Fixes validation errors when a shared/included workflow file defines a declarative engine with a default version and uses pre-agent-steps. Two independent gaps in schema/validation were closed.

Root cause

• The engine JSON schema had no version property, so declarative engine definitions specifying a default CLI version failed schema validation.
• The included-file frontmatter allow-list only recognized steps, not pre-steps, pre-agent-steps, or post-steps, so shared engine files using these fields were rejected as having "unexpected fields".

Changes

• pkg/parser/schemas/main_workflow_schema.json: Added version (string or number) to the engine schema object — "Default CLI version applied when a workflow references this engine without specifying engine.version".
• pkg/parser/include_processor.go: Added pre-steps, pre-agent-steps, and post-steps to collectUnexpectedIncludedFrontmatterFields's valid-fields list (plus gofmt realignment).
• pkg/parser/schema_location_test.go: Added TestValidateIncludedFileFrontmatterWithSchemaAndLocation_DeclarativeEngineWithVersionAndPreAgentSteps, asserting that a declarative engine (id, version, display-name, description, experimental, provider, behaviors) combined with pre-agent-steps and runtimes passes included-frontmatter validation.

Impact

Non-breaking. Enables shared engine definition files (e.g. a custom aider engine) to declare a default version and pre-agent install steps without failing validation — previously blocked scenario now supported.

Testing

New unit test added and passing; covers the previously-failing combination directly.> Generated by PR Description Updater for #50915 · auto · 37.4 AIC · ⊞ 6.8K · ◷

[github-actions]

⚠️ Security scanning failed for Test Quality Sentinel. Review the logs for details.

[github-actions]

⚠️ Security scanning failed for Design Decision Gate 🏗️. Review the logs for details.

No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (61 additions detected).

[github-actions]

🧪 Test Quality Sentinel Report

⚠️ Test Quality Score: 62/100 — Acceptable

Analyzed 19 test(s): 9 design, 10 implementation, 0 violation(s).

📊 Metrics (19 tests)

Metric	Value
Analyzed	19 (Go: 19, JS: 0)
✅ Design	9 (47%)
⚠️ Implementation	10 (53%)
Edge/error coverage	8 (42%)
Duplicate clusters	0
Inflation	No (35 test lines : 22 prod lines = 1.59:1, well under 2:1)
🚨 Violations	0

Per-Test Summary:

Test	Classification	Issues
TestValidateWithSchemaAndLocation	design_test	None — Table-driven with 3 scenarios validating error message location & hygiene. ✅ Error coverage.
TestSchemaURLDomainChange	design_test	None — Verifies domain migration (contoso.com validation). Critical regression signal. ✅ Error coverage.
TestValidateMainWorkflowFrontmatterWithSchemaAndLocation	design_test	None — Core workflow frontmatter validation with file paths. ✅ Error coverage.
TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_AdditionalProperties	design_test	🎯 KEY TEST — 8-row table-driven test covering typo detection (requird→required) in workflow_call, dispatch_repository, permissions, tools. This is the validation that tests the PR's core intent: catching schema violations that prevent accidental config errors. ✅ Error coverage on all 8 rows.
Job/runs-on form tests (Tests 5–11)	implementation_test	None — Happy-path acceptance tests for runs-on syntax variants (string, array, object). ✅ High value for regression coverage. ⚠️ No error cases.
Safe-outputs tests (Tests 12–14)	implementation_test	None — Happy-path acceptance tests for safe-outputs features. ✅ High value for regression. ⚠️ No error cases.
TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_RejectsTopLevelCommand	design_test	None — Negative validation case: rejects unknown 'command' field. ✅ Error coverage.
TestValidateIncludedFileFrontmatterWithSchemaAndLocation_SkipsCustomAgentFiles	design_test	None — Validates custom agent files bypass schema validation. Loops through 3 paths. ✅ Error coverage (expects no error).
TestValidateIncludedFileFrontmatterWithSchemaAndLocation_DeclarativeEngineWithVersionAndPreAgentSteps (NEW)	design_test	🆕 ADDED — Tests declarative engine configuration with version="0.86.2" and pre-agent-steps array. This directly tests the production code changes in include_processor.go which added "pre-agent-steps" to the valid frontmatter fields. Validates that the engine version and pre-agent-steps are accepted by validation. ✅ High value. ⚠️ No error assertions (happy-path only).
TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_MaxStack	design_test	None — Table-driven with 6 scenarios testing max-stack bounds (valid: 1,2,5,-1; invalid: 0,-2). ✅ Error coverage on invalid cases.
TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_PullRequestReviewMaxStack	design_test	None — Same max-stack validation on pull_request_review trigger. ✅ Error coverage.

Analysis

Strengths:

• ✅ Zero hard violations (no Go mock libraries, proper //go:build !integration tag)
• ✅ Core test covers PR intent: Test 4 (AdditionalProperties) is a comprehensive 8-row validation test that confirms schema detection of typos like "requird" — exactly what this PR enforces
• ✅ New test validates production change: The new test covers declarative engine version and pre-agent-steps, directly validating the added valid fields in collectUnexpectedIncludedFrontmatterFields
• ✅ Good error coverage: 8 of 9 design tests include explicit error assertions (strings.Contains checks, t.Errorf on error, t.Fatalf)
• ✅ No inflation: 1.59:1 test:prod ratio is well under the 2:1 threshold

Design vs. Implementation Split:

• 47% design tests (invariants, validation logic, error handling)
• 53% implementation tests (happy-path syntax acceptance)
  • Implementation tests are necessary regression coverage for runs-on variants, safe-outputs features, and schema acceptance scenarios. No mock libraries used.

Why This Scores 62 (Not Higher):

• Implementation test count: 10 of 19 tests are happy-path only (no error scenarios). While each is important for regression, they don't test edge cases or error handling.
• New test lacks error coverage: The new DeclarativeEngineWithVersionAndPreAgentSteps test is a single happy-path scenario (no negative case for invalid version or malformed steps).

Verdict

✅ Passed. 53% implementation tests (threshold: 30%). No violations. PR quality acceptable.

Recommendation: This PR correctly validates the schema changes and adds appropriate test coverage for the new declarative engine fields. The score of 62/100 reflects good core behavioral validation (design tests) balanced with necessary regression coverage (implementation tests). The absence of mock libraries and comprehensive typo-detection testing (Test 4) demonstrates solid schema validation practices.

---

Analysis: 19 Go tests, line counts (test: +35, prod: +22). Build tags verified. No test infrastructure changes.

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 34.4 AIC · ⊞ 7.7K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 62/100. 53% implementation tests (threshold: 30%). Zero violations. Schema validation correctly enforces typo detection and supports new declarative engine fields.

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.86.0