Skip to content

chore: register a.springut and a.pyanzin as skill owners - #31

Open
JavadManashti wants to merge 1 commit into
mainfrom
chore/register-new-skill-owners
Open

chore: register a.springut and a.pyanzin as skill owners#31
JavadManashti wants to merge 1 commit into
mainfrom
chore/register-new-skill-owners

Conversation

@JavadManashti

Copy link
Copy Markdown
Collaborator

Two-line follow-up to #28.

The VALID_OWNERS allowlist merged in #28 (added at Copilot's suggestion — the PR description had claimed validation the code didn't do). It landed after #24 and #29 were opened, so both now fail CI on:

skills/asset-upload/SKILL.md: metadata owner 'a.springut' is not in the known-owners list
skills/game-web-portal/SKILL.md: metadata owner 'a.pyanzin' is not in the known-owners list

Neither is a typo — both are real owners of skills currently in review (@aspringut-xsolla, @apyanzin-xsolla). Registering them here rather than asking each author to patch a validator file unrelated to their change.

Validator is clean on this branch: 0 new errors, 0 known debt, 0 warnings.

Note on the tradeoff

Worth a maintainer's opinion: as written, every new skill author's first PR fails CI until someone adds them here. That's real friction for a check whose only job is catching a typo. It's defensible — a bad owner silently breaks review routing, which is the exact bug class #28 fixed in CODEOWNERS — but if the friction outweighs it, downgrading the unknown-owner case from error to warning is a one-line change and I'm happy to do that instead.

The VALID_OWNERS allowlist added in #28 landed after #24 and #29 were
already open, so both fail validation on a legitimate new owner rather
than a typo. Registering both unblocks them without asking either
author to edit a validator they don't otherwise touch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 19, 2026 13:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Registers two additional internal owner IDs in the skill validator allowlist so existing skill PRs using these metadata.owner values can pass CI.

Changes:

  • Added a.springut to the VALID_OWNERS allowlist.
  • Added a.pyanzin to the VALID_OWNERS allowlist.

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

@JavadManashti

Copy link
Copy Markdown
Collaborator Author

@y-klochikhin @e-chernykh — two-line review if either of you has a minute, and it unblocks two other PRs.

Both #24 and #29 currently fail CI on a single error each, and neither is the author's fault:

skills/asset-upload/SKILL.md:    metadata owner 'a.springut' is not in the known-owners list
skills/game-web-portal/SKILL.md: metadata owner 'a.pyanzin' is not in the known-owners list

The VALID_OWNERS allowlist merged in #28 after both PRs were already open, so two legitimate owners read as typos. This registers them. Validator is clean on this branch — 0 new errors, 0 known debt, 0 warnings.

Worth flagging honestly: this is the second time in a week that allowlist has cost more than it caught. Its only job is catching a typo in metadata.owner, and the failure mode it prevents (a bad owner silently breaking review routing) is real — that's the exact bug #28 fixed in CODEOWNERS. But as written, every new skill author's first PR fails CI until a maintainer patches this file, which is friction on precisely the contributors we least want to trip up.

If you'd rather, downgrading the unknown-owner case from error to warning is a one-line change and I'll do that instead — the typo still gets surfaced, it just doesn't block. Say which you prefer and I'll adjust before merge.

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.

2 participants