Skip to content

Add regression test confirming top-level roles: is rejected by schema validation - #56981

Merged
pelikhan merged 3 commits into
mainfrom
copilot/deep-report-fix-top-level-roles-issue
Aug 29, 2026
Merged

Add regression test confirming top-level roles: is rejected by schema validation#56981
pelikhan merged 3 commits into
mainfrom
copilot/deep-report-fix-top-level-roles-issue

Conversation

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Top-level roles: was believed to be silently ignored by extractRoles(), falling back to default roles [admin, maintainer, write] with no warning — a risky gap if an author expected it to restrict access.

Investigation showed this is already handled correctly end-to-end: schema validation (additionalProperties: false at the top level, roles only defined under on) rejects a top-level roles: field before extractRoles() is ever invoked, producing Unknown property: roles ... 'roles' belongs under 'on'. Docs already document roles: exclusively under on:. The only real gap was test coverage at the validation/compile level (existing tests only covered the low-level extractRoles() fallback behavior, not that the field is actually rejected upfront).

  • Test coverage: Added a case to TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_AdditionalProperties in pkg/parser/schema_location_test.go asserting that a top-level roles: field fails schema validation with the message 'roles' belongs under 'on'.

No production code changes were required — the described behavior was already fixed by existing schema validation.

Copilot AI and others added 2 commits August 29, 2026 21:59
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix top-level roles field being ignored in frontmatter Add regression test confirming top-level roles: is rejected by schema validation Aug 29, 2026
Copilot AI requested a review from pelikhan August 29, 2026 22:01
@pelikhan
pelikhan marked this pull request as ready for review August 29, 2026 22:07
Copilot AI balanced review requested due to automatic review settings August 29, 2026 22:07
@pelikhan
pelikhan merged commit 3bb23d0 into main Aug 29, 2026
@pelikhan
pelikhan deleted the copilot/deep-report-fix-top-level-roles-issue branch August 29, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused test accurately covers the reported validation behavior and expected diagnostic.

Pull request overview

Adds regression coverage confirming misplaced top-level roles configuration is rejected before workflow compilation proceeds.

Changes:

  • Tests schema rejection of top-level roles.
  • Verifies guidance points users to on.roles.
File summaries
File Description
pkg/parser/schema_location_test.go Adds the schema-validation regression case.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.87.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[deep-report] Top-level roles: frontmatter field is silently ignored, falls back to defaults with no warning

3 participants