Add Skill Routing Kit plugin#183
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Skill Routing Kit' plugin for Codex, adding a local-first routing guard, capability registry, diagnostic scripts, and installation tools. Feedback on the changes highlights several critical issues, including a missing snippet file referenced by the installer, potential performance and correctness bugs in the pre-push hook when handling new branches, and a lack of cleanup traps in the installation shell script. Additionally, improvements are suggested to ensure idempotency when injecting the routing guard, handle broken symlinks robustly during installation, and prevent false-positive categorizations in the registry builder caused by simple substring matching.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if args.install_agents: | ||
| snippet_path = target / "assets" / "agents-routing-snippet.md" | ||
| action = install_agents_snippet(snippet_path, args.agents.expanduser().resolve()) |
There was a problem hiding this comment.
The file assets/agents-routing-snippet.md is referenced here but is not included in the pull request. This will cause a FileNotFoundError when running the installer with the --install-agents flag (which is recommended in the README). Please add the missing file to the repository.
| if [ "$remote_sha" = "0000000000000000000000000000000000000000" ]; then | ||
| range="$local_sha" | ||
| else | ||
| range="$remote_sha..$local_sha" | ||
| fi | ||
|
|
||
| committed_range_files="$committed_range_files | ||
| $(git diff --name-only "$range")" | ||
| committed_range_text="$committed_range_text | ||
| $(git diff --cached --name-only) | ||
| $(git log --format=%B "$range")" |
There was a problem hiding this comment.
When pushing a new branch (remote_sha is all zeros), setting range to "$local_sha" causes git log to traverse the entire commit history back to the root commit. In non-trivial repositories, this will be extremely slow and will likely trigger false positives on old commits. Additionally, git diff --name-only "$local_sha" compares the commit against the working tree rather than finding the files changed in the commits being pushed.
Instead, use git log with --not --remotes to target only the new commits being pushed.
| if [ "$remote_sha" = "0000000000000000000000000000000000000000" ]; then | |
| range="$local_sha" | |
| else | |
| range="$remote_sha..$local_sha" | |
| fi | |
| committed_range_files="$committed_range_files | |
| $(git diff --name-only "$range")" | |
| committed_range_text="$committed_range_text | |
| $(git diff --cached --name-only) | |
| $(git log --format=%B "$range")" | |
| if [ "$remote_sha" = "0000000000000000000000000000000000000000" ]; then | |
| committed_range_files="$committed_range_files | |
| $(git log --name-only --format= "$local_sha" --not --remotes)" | |
| committed_range_text="$committed_range_text | |
| $(git log --format=%B "$local_sha" --not --remotes)" | |
| else | |
| committed_range_files="$committed_range_files | |
| $(git diff --name-only "$remote_sha..$local_sha")" | |
| committed_range_text="$committed_range_text | |
| $(git log --format=%B "$remote_sha..$local_sha")" | |
| fi |
| def install_agents_snippet(snippet_path: Path, agents_path: Path) -> str: | ||
| snippet = snippet_path.read_text(encoding="utf-8") | ||
| agents_path.parent.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| if agents_path.exists(): | ||
| current = agents_path.read_text(encoding="utf-8") | ||
| else: | ||
| current = "" | ||
|
|
||
| if BEGIN_MARKER in current and END_MARKER in current: | ||
| before = current.split(BEGIN_MARKER, 1)[0].rstrip() | ||
| after = current.split(END_MARKER, 1)[1].lstrip() | ||
| updated = f"{before}\n\n{snippet.rstrip()}\n\n{after}".strip() + "\n" | ||
| action = "Updated" | ||
| else: | ||
| separator = "\n\n" if current.strip() else "" | ||
| updated = f"{current.rstrip()}{separator}{snippet.rstrip()}\n" | ||
| action = "Added" | ||
|
|
There was a problem hiding this comment.
To ensure idempotency and prevent duplicate snippet appends on subsequent installer runs, the installer should guarantee that the snippet is wrapped with the BEGIN_MARKER and END_MARKER before writing to AGENTS.md.
def install_agents_snippet(snippet_path: Path, agents_path: Path) -> str:
snippet = snippet_path.read_text(encoding="utf-8").strip()
if not (snippet.startswith(BEGIN_MARKER) and snippet.endswith(END_MARKER)):
snippet = f"{BEGIN_MARKER}\n{snippet}\n{END_MARKER}"
agents_path.parent.mkdir(parents=True, exist_ok=True)
if agents_path.exists():
current = agents_path.read_text(encoding="utf-8")
else:
current = ""
if BEGIN_MARKER in current and END_MARKER in current:
before = current.split(BEGIN_MARKER, 1)[0].rstrip()
after = current.split(END_MARKER, 1)[1].lstrip()
updated = f"{before}\n\n{snippet}\n\n{after}".strip() + "\n"
action = "Updated"
else:
separator = "\n\n" if current.strip() else ""
updated = f"{current.rstrip()}{separator}{snippet}\n"
action = "Added"
agents_path.write_text(updated, encoding="utf-8")
return action| cleanup_dir="$(mktemp -d)" | ||
| git clone --depth 1 "$REPO_URL" "$cleanup_dir/Skill-Routing-Kit" >/dev/null | ||
| ROOT_DIR="$cleanup_dir/Skill-Routing-Kit" |
There was a problem hiding this comment.
If the script is interrupted (e.g., via Ctrl+C) or fails during the git clone or python3 execution, the temporary directory created by mktemp -d will not be cleaned up. It is best practice to use a trap to ensure cleanup on exit.
| cleanup_dir="$(mktemp -d)" | |
| git clone --depth 1 "$REPO_URL" "$cleanup_dir/Skill-Routing-Kit" >/dev/null | |
| ROOT_DIR="$cleanup_dir/Skill-Routing-Kit" | |
| cleanup_dir="$(mktemp -d)" | |
| trap 'rm -rf "$cleanup_dir"' EXIT INT TERM | |
| git clone --depth 1 "$REPO_URL" "$cleanup_dir/Skill-Routing-Kit" >/dev/null | |
| ROOT_DIR="$cleanup_dir/Skill-Routing-Kit" |
| if target.exists(): | ||
| backup_root = target.parent / ".backups" | ||
| backup_root.mkdir(parents=True, exist_ok=True) | ||
| stamp = datetime.now().strftime("%Y%m%d-%H%M%S") | ||
| backup_path = backup_root / f"{target.name}-{stamp}" | ||
| shutil.move(str(target), str(backup_path)) | ||
| print(f"Backed up existing plugin to: {backup_path}") |
There was a problem hiding this comment.
If target is a broken symlink, target.exists() will return False, but the symlink entry still exists in the filesystem. This will cause target.mkdir to fail with a FileExistsError. Checking target.is_symlink() ensures broken symlinks are also cleaned up or backed up properly.
| if target.exists(): | |
| backup_root = target.parent / ".backups" | |
| backup_root.mkdir(parents=True, exist_ok=True) | |
| stamp = datetime.now().strftime("%Y%m%d-%H%M%S") | |
| backup_path = backup_root / f"{target.name}-{stamp}" | |
| shutil.move(str(target), str(backup_path)) | |
| print(f"Backed up existing plugin to: {backup_path}") | |
| if target.exists() or target.is_symlink(): | |
| backup_root = target.parent / ".backups" | |
| backup_root.mkdir(parents=True, exist_ok=True) | |
| stamp = datetime.now().strftime("%Y%m%d-%H%M%S") | |
| backup_path = backup_root / f"{target.name}-{stamp}" | |
| shutil.move(str(target), str(backup_path)) | |
| print(f"Backed up existing plugin to: {backup_path}") |
| for token, values in mapping.items(): | ||
| if token in haystack: | ||
| categories.update(values) |
There was a problem hiding this comment.
Using simple substring matching (token in haystack) can lead to false positives. For example, any description containing the word 'explanation' will match 'plan' and incorrectly categorize the capability under 'planning'. Consider using word boundaries or token-based matching to ensure accuracy.
|
Two issues:\n\n1. This PR renames the existing entry to in marketplace.json. This is an unrelated change that affects another plugin's entry. Please revert that rename.\n\n2. The diff shows the entry was added, which is good. The bundle and marketplace entries are present. Please fix issue 1 and this should be mergeable. |
|
Before this PR can be merged, your plugin repo needs the HOL AI Plugin Scanner running in CI. This is a mandatory requirement for all submissions. Add this workflow to your plugin repo at name: HOL Plugin Scanner
on:
push:
branches: [main, master]
pull_request:
branches: [main, master]
permissions:
contents: read
security-events: write
jobs:
scan:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: HOL Plugin Scanner
uses: hashgraph-online/ai-plugin-scanner-action@v1
with:
plugin_dir: "."
mode: scan
min_score: 80
fail_on_severity: high
format: sarif
upload_sarif: trueAlso run the scanner locally and include the score in your PR description: pipx install plugin-scanner
plugin-scanner scan . --format textYour plugin needs a score of 80/130 or higher with no critical or high severity findings. Link the CI run or paste the score in this PR description. See the full guide: SCANNER_GUIDE.md Additional issues: |
2 participants
Summary
Add Skill Routing Kit to the curated Codex plugin list.
Skill Routing Kit is a local-first Codex routing guard that improves skill/plugin selection with:
Validation
python3 scripts/check-alphabetical.py README.mdpython3 scripts/validate-plugin-pr.py --base-ref origin/maingit diff --cached --checkThe plugin bundle includes a compact marketplace icon and a
.codexignorefile for cleaner packaging.