Skip to content

Add AI skills#19

Merged
Vladyslav-Kuksiuk merged 11 commits into
masterfrom
add-ai-skills
Jul 24, 2026
Merged

Add AI skills#19
Vladyslav-Kuksiuk merged 11 commits into
masterfrom
add-ai-skills

Conversation

@Vladyslav-Kuksiuk

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

This PR provides AI skills.

Resolves this issue.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk self-assigned this Jul 23, 2026
@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk marked this pull request as ready for review July 23, 2026 19:31
@Oleg-Melnik

Copy link
Copy Markdown
Collaborator

Suggestions to raise the AI-agents config quality

These are enhancements beyond the review findings above. None block PR #19; they harden the
framework against the failure mode that matters most for governance docs — silent drift between
what the skills assert and what the repository actually does. Ordered by value.

P1 — Make the framework self-validating in CI

The checks I ran by hand for this review (frontmatter parses, links and anchors resolve, 100-char
rule, no trailing whitespace) are exactly the invariants the config depends on, yet nothing enforces
them. Add a lightweight agents-lint job (or a step in check.yml) that fails the build when:

  • any .agents/skills/*/SKILL.md frontmatter is missing or name: does not equal its directory name;
  • a relative Markdown link or #anchor does not resolve (a small link-checker over .agents/**,
    AGENTS.md, PROJECT.md, CLAUDE.md, .github/copilot-instructions.md);
  • a changed Markdown line exceeds 100 columns or carries trailing whitespace.

This turns "verified once during review" into "true on every commit." Keep it Markdown-only so it
adds seconds, not a toolchain.

P2 — Guard the routing list against orphaned skills

AGENTS.md hand-lists the six skills. A new skill directory (or a rename) can be added without a
routing entry, and nothing catches it. Add a check that the set of .agents/skills/*/SKILL.md
directories is exactly the set listed in AGENTS.md "Skill routing" (and reflected in
.github/copilot-instructions.md). A three-line shell diff in the P1 job is enough. Without this,
an unrouted skill is invisible to every agent even though the file exists.

P2 — Enforce one source of truth to prevent factual drift

The skills correctly avoid hard-coding most versions (they say "read from README.md/BuildSettings"),
but a few contracts are stated in two places and can diverge:

  • KDoc rules in writer/SKILL.md and kotlin-engineer/references/kotlin-policy.md (Nit 2 above).
  • Trust-boundary summary in PROJECT.md and the full policy in
    security-engineer/references/trust-boundaries.md.
  • Test-strategy bullets in PROJECT.md, gradle-practices.md, and test-patterns.md.

Per the repo's own rule ("Link to the owning skill instead of duplicating its guidance"), designate
the owning reference for each contract and reduce the other locations to a one-line pointer. This is
the single biggest lever against stale guidance, because a claim that lives in one place cannot
contradict itself.

P3 — Add .gitattributes coverage for agent config

CI runs on windows-latest. Add *.md text eol=lf (scoped to .agents/** and the root agent docs)
so Windows checkouts do not rewrite line endings under the LF-oriented formatting the writing style
assumes. Pairs naturally with documenting the .claude/skills symlink's Unix assumption (Nit 1).

P3 — Normalize frontmatter scalar style

SKILL.md files mix YAML block-scalar styles: kotlin-engineer and test-engineer use > (folded),
the rest use >- (folded, strip-final-newline). Pick one and apply it to all six so the frontmatter
reads uniformly and any future frontmatter parser sees consistent input.

P3 — Prefer durable source pointers over restated specifics

A few skills embed concrete facts that a code change could silently invalidate — e.g.
trust-boundaries.md spells out the fresh-install step order, and gradle-practices.md "Repository
exclusions" names installEmbedCode/checkEmbedding/embedCode behavior. These are accurate today
(verified above), but they are the lines most likely to rot. Where a durable path suffices, phrase the
rule as "trace the current flow in InstallEmbedCodeTask" rather than restating the steps; where a
worked ordering genuinely aids the reader, add a comment reminding editors to re-verify it against the
task when that file changes.

P4 — Add worked good/bad examples to the two dense references

kotlin-policy.md and test-patterns.md teach effectively because they show code. gradle-practices.md
(task-state annotations: @Input vs @LocalState vs @Internal) and trust-boundaries.md
(a minimal fail-closed vs fail-open snippet) are pure rule lists; one small correct/incorrect example
each would raise their instructional value to match the other two.

P4 — State a review-effort or verdict rubric once, shared

The reviewer skill defines the Must fix / Should fix / Nits taxonomy and verdict ladder. If other
entry points (e.g. a future PR-review automation) need the same rubric, keep it in the reviewer skill
and link to it rather than re-describing severities elsewhere — same single-source-of-truth principle
as P2, applied to process vocabulary.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Findings from a review of the agent framework. Links, anchors, frontmatter names, table alignment, the 100-char rule, and every policy-vs-code claim I checked all hold. One Should-fix (a routing coverage gap) and nits below; broader quality suggestions are in a separate top-level comment.

Comment thread .agents/skills/gradle-engineer/SKILL.md Outdated
Comment thread .agents/skills/writer/SKILL.md
Comment thread .agents/skills/test-engineer/references/test-patterns.md Outdated
Comment thread .claude/skills

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Vladyslav-Kuksiuk Please see my comments.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Follow-up nits on the new agent-config validator and CI job. All non-blocking (the earlier Should-fix and other nits are resolved on this head; verdict remains APPROVE).

Comment thread .github/workflows/check.yml Outdated
run: python -m unittest discover -s scripts/tests -p "test_*.py"

- name: Validate Agent Configuration
run: python scripts/check_agent_config.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit — command/interpreter inconsistency. This step runs python, but the validator's own failure hint says python3 (scripts/check_agent_config.py:671) and AGENTS.md also instructs python3. On some distros python is Python 2 or absent, so the CI command and the documented local command can resolve to different interpreters. Also, no minimum Python version is stated anywhere while CI pins 3.14; the code runs on ~3.9+ (thanks to from __future__ import annotations), so a one-line "requires Python 3.9+" note in AGENTS.md — plus aligning this step to python3 — would make the three invocations agree.

import unittest
from pathlib import Path

from scripts.check_agent_config import validate_repository

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit — discovery is working-directory-dependent. from scripts.check_agent_config import ... only resolves when unittest is run from the repository root (which AGENTS.md and the CI job both do, so this is fine today). Running the suite from another directory fails at import. Passing -t . to unittest discover (or adding a scripts/tests/__init__.py / conftest-style path setup) would make the suite location-independent and less fragile to future CI changes.

from scripts.check_agent_config import validate_repository


class AgentConfigValidatorTest(unittest.TestCase):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit — a few implemented rules are unverified. The validator enforces behaviors that no test exercises: the missing-required-file diagnostics (_validate_required_paths), the unsupported-frontmatter-key rejection (frontmatter key ... is not supported), and the key-ordering rules (frontmatter name must be the first entry / description must immediately follow the name). The existing cases are thorough; adding a subTest or two for these closes the gap so a future refactor can't silently drop them.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Vladyslav-Kuksiuk LGTM with minor comments.

@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk merged commit 624e6be into master Jul 24, 2026
3 checks passed
@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk deleted the add-ai-skills branch July 24, 2026 11:16
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