Repository navigation
feat: pre-commit hook — devin-skill-catalog workspace lint gate - #45
Conversation
Add .pre-commit-hooks.yaml so consumers can lint the repo's Devin items on every commit: SKILL.md frontmatter (name matches dir, non-trivial description) and H1 in rule files. FAIL findings block (exit 1); WARN/PASS print only. --no-user keeps personal skills out of scope; no .devin/ means no findings and a pass. Strictly read-only. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| @@ -0,0 +1,18 @@ | |||
| - id: devin-skill-catalog-gate | |||
There was a problem hiding this comment.
🔍 Hook requires consumer configuration
The manifest publishes a hook but does not activate it for this repository. Contributors need a separate pre-commit configuration to run it before commits.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
No code change — that split is how pre-commit distribution works: .pre-commit-hooks.yaml publishes hook definitions for consumer repos, activation always lives in the consumer’s .pre-commit-config.yaml. Self-activating it here would be actively harmful: it is a language: system hook, and the documented root setup (pip install -e ".[dev]" on a non-package workspace with dependencies = []) does not put devin-skill-catalog on PATH — every contributor commit would fail on a missing binary, the exact failure mode flagged in the other thread. Consumer setup is now documented instead: README gained a "Pre-commit hook" section with the repo:/rev: snippet and the install prerequisite (372dd10).
… layouts Devin Review triage on the hook manifest: - .pre-commit-hooks.yaml: drop always_run in favour of a files filter anchored to .devin/skills|rules and top-level skills|rules, so the system hook only fires when Devin items are staged — a missing devin-skill-catalog binary can no longer block unrelated commits. Description now states the PATH prerequisite and the both-layout inventory. - scan: new resolve_devin_dirs returns every scan root — a top-level skills//rules pair coexisting with .devin/ no longer hides .devin items from lint, gate and diff (cmd_diff now resolves via scan_targets). - tests: test_pre_commit_hook.py pins the manifest shape, the files regex and the entry's exit semantics; test_scan.py gains a mixed-layout regression test (74 passed). - CHANGELOG/README: Unreleased entry plus a consumer .pre-commit-config.yaml snippet. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
github-actions
Bot
added
the
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| a = scan.scan_targets([Path(args.a)], include_user=False) | ||
| b = scan.scan_targets([Path(args.b)], include_user=False) |
There was a problem hiding this comment.
🔴 Top-level changes vanish from inventory diff
When both layouts contain skill:demo, cmd_diff can report IDENTICAL despite changes to the top-level skill. diff_inventories keeps only the later .devin item for each key.
Learn more
A workspace inventory now includes top-level items and .devin items. Both can share a kind:name key, but diff_inventories builds dictionaries on that key. The .devin item comes last, so the top-level item disappears before comparison. The command can return exit code 0 even when one workspace changed.
Example: Workspace A has skills/demo/SKILL.md with content X and .devin/skills/demo/SKILL.md with content Z. Workspace B has top-level content Y and the same .devin content Z. diff A B reports skill:demo as IDENTICAL instead of detecting X versus Y.
Recommended fix: Give each layout a distinct comparison identity in cmd_diff and diff_inventories, or explicitly reject duplicate keys before diffing. Add a mixed-layout diff test with same-name items.
Was this helpful? React with 👍 or 👎 to provide feedback.
| entry: devin-skill-catalog lint --no-user | ||
| language: system | ||
| pass_filenames: false | ||
| files: '^(\.devin/(skills|rules)/|(skills|rules)/)' |
There was a problem hiding this comment.
🟡 Symlinked Devin items skip the lint gate
When a commit adds a symlinked skill or rule, files alone does not trigger the hook. Pre-commit's default file type excludes symlinks, though scan_devin_dir follows them and finds their items.
Learn more
Pre-commit applies its default types: [file] in addition to the path filter. A symlink has the symlink type, even when its target is a Markdown file or skill directory. scan_devin_dir uses is_dir() and is_file(), which follow symlinks; the CLI would lint those items if invoked.
Example: A commit adds .devin/rules/check.md as a symlink to a rule without an H1 title. The path matches files, but its type does not match file, so the invalid rule is committed without lint.
Recommended fix: Include the symlink type in this hook and add a test for a committed symlinked rule or skill.
| files: '^(\.devin/(skills|rules)/|(skills|rules)/)' | |
| files: '^(\.devin/(skills|rules)/|(skills|rules)/)' | |
| types: [file, symlink] |
Was this helpful? React with 👍 or 👎 to provide feedback.
Conflicts resolved: keep main's .pre-commit-hooks.yaml (PR #45 shipped the hardened hook), union the CHANGELOGs, keep main's updater.py imports (PurePath test seam), and combine scan_targets -> resolve_target (PR: lint accepts file targets) with the new resolve_devin_dirs mixed-layout expansion in resolve_target's dir branch (avoids the scan_targets<->resolve_target recursion). Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Summary
.pre-commit-hooks.yamlexposingdevin-skill-catalog-gate: runsdevin-skill-catalog lint --no-user— the catalog's structural lint — before each commit..devin/(or top-levelskills//rules/) and validates each item: SKILL.md frontmatter,namematching its directory, non-trivialdescription, H1 in rule files. Any FAIL exits 1 and blocks; WARN/PASS only print.--no-userkeeps personal skills out of scope; no Devin items means no findings and a pass.Test plan
yaml.safe_loadparses the manifest; fields match the devin-state/devin-judge shapedevin-skill-catalog lint --no-userinvoked locally in a dir with no.devin/: 0 findings, exit 0Generated with Devin