Skip to content

Reject empty agent slugs before cleaning conversion output - #967

Open
rudycelekli wants to merge 2 commits into
msitarzewski:mainfrom
rudycelekli:fix/empty-agent-slug-20261001
Open

rudycelekli wants to merge 2 commits into
msitarzewski:mainfrom
rudycelekli:fix/empty-agent-slug-20261001

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Implements the maintainer-approved item 2 in #917. A nonempty Unicode-only name can normalize to an empty ASCII slug. Reject it in lint and converter preflight before cleaning output; retain the Unicode display name when an explicit ASCII alias is supplied. No transliteration.

Coordination: #917 candidate list. Empty-slug scope is separately approved in the maintainer’s preceding comment.

Agent Information (if adding/modifying an agent)

  • Agent Name: Agent validation and conversion
  • Category: Tooling (approved existing scripts)
  • Specialty: Installer/discovery metadata

Verification

Original accepts an empty slug. Fixed native fixture rejects it before touching a pre-existing sentinel; explicit “Expert 专家” and plain ASCII names convert. Existing regression now checks the sentinel and valid alias. Bash 3.2 syntax, tools/divisions, collision, folded/closing frontmatter and complete three-tool frontmatter regressions passed.

  • Independent review of the complete frozen patch and native controls passed.
  • Scoped lint/originality and git diff --check passed for content; focused script regressions above passed for tooling.
  • On one combined snapshot of the 14 disjoint content patches plus the approved empty-slug fix: full-roster lint passed (282 agents, 0 errors, 58 existing warnings); originality passed; converter round-trip/strict-parse/count checks passed (32 checks, 0 failures, 282 agents × 15 tools). Exactly 14 expected agent checksum drifts are advisory; no checksum/generated files are committed. Full installer suite: 66 passed, 0 failed, 2 documented expected failures for fix(installer): preserve paths with spaces in parallel mode #755. This full suite ran on the combined snapshot, not independently on each PR.

Limits

ASCII aliases are supplied by the author; no locale-dependent normalization or automatic transliteration is introduced.

Checklist

  • Preserves the existing agent template/persona (tooling scope is separately approved).
  • Existing YAML metadata is preserved.
  • Includes a concrete reproduction and focused scenario controls.
  • Reviewed/proofread; no generated integration files or checksum changes.

CI fixture correction

The first Linux run caught an obsolete Codex SKILL-path expectation in the new alias fixture. The current converter correctly emits codex/agents/expert.toml. The fixture now checks that path and explicitly exits on either a missing artifact or a changed preservation sentinel, including on Bash 3.2. The complete frontmatter suite passed on the unchanged final script hash; an independent actual-fixture replay also confirmed healthy success and both forced-failure diagnostics. Production linter/converter code is unchanged by this follow-up. Fresh upstream checks are pending.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@phant0um

phant0um commented Oct 5, 2026

Copy link
Copy Markdown

I tested this PR on macOS 27 with bash 5. Setup: main at 8329468, and main with this PR merged on top (64319d2). The merge is clean.

For the plant, I copied engineering/engineering-ai-data-remediation-engineer.md to engineering/zz-probe.md in a temp copy of the repo. Then I changed only the name: line. Each case ran 2 times with the same result.

Case main main + PR
name: 专家, lint-agents.sh exit 0, no message exit 1, produces an empty agent slug
name: 专家, convert.sh --tool codex exit 0, writes codex/agents/.toml exit 1, empty agent slug for engineering/zz-probe.md, old output kept
name: --- same as 专家 same as 专家
name: Expert 专家 exit 0, writes expert.toml exit 0, writes expert.toml

On main the bug is silent. In one full convert.sh run with name: 专家, the script exits 0 and writes hidden files with an empty stem in 8 places: codex/agents/.toml, cursor/rules/.mdc, gemini-cli/agents/.md, opencode/agents/.md, qwen/agents/.md, vibe/agents/.toml, vibe/prompts/.md and zcode/agents/.md. ls does not show these files. The PR stops this before clean_tool_output runs, so existing output stays in place.

The new block in test-convert-frontmatter.sh passes on the PR. With the scripts from main, it fails on the first new assertion: Expected linter to reject an agent name with an empty install slug. The test catches the bug.

Not tested: the converter with the PR for tools other than codex. I did not run it because check_agent_slug_collisions runs once, before the tool loop. I also did not check hermes/.../agents.json or shellcheck.

Command I used per case:

sed 's/^name:.*/name: 专家/' engineering/engineering-ai-data-remediation-engineer.md > engineering/zz-probe.md
scripts/lint-agents.sh engineering/zz-probe.md; echo "lint=$?"
scripts/convert.sh --tool codex --out /tmp/out; echo "convert=$?"

TheRealVitja pushed a commit to TheRealVitja/agency-agents that referenced this pull request Oct 6, 2026
…nts)

Curated sync of the open upstream pull requests as of 2026-10-06:

- Script and CI fixes: msitarzewski#1055, msitarzewski#1030, msitarzewski#865, msitarzewski#860, msitarzewski#967, msitarzewski#870, msitarzewski#869, msitarzewski#889,
  msitarzewski#1056, msitarzewski#755, msitarzewski#868, msitarzewski#771, msitarzewski#867; ported msitarzewski#523, msitarzewski#512 and the permissions
  part of msitarzewski#790.
- Existing-agent fixes: msitarzewski#1033-msitarzewski#1052, msitarzewski#1053, msitarzewski#1054, msitarzewski#1023, msitarzewski#756-msitarzewski#759, msitarzewski#799,
  msitarzewski#805, msitarzewski#715, msitarzewski#752, msitarzewski#784, msitarzewski#793, msitarzewski#858, msitarzewski#812, msitarzewski#789, msitarzewski#1007.
- New agents: msitarzewski#702, msitarzewski#707, msitarzewski#731, msitarzewski#732, msitarzewski#764, msitarzewski#848, msitarzewski#859, msitarzewski#862, msitarzewski#863, msitarzewski#886,
  msitarzewski#908, msitarzewski#982-msitarzewski#985, msitarzewski#1031, msitarzewski#1032.
- Docs: msitarzewski#577, msitarzewski#743, msitarzewski#762, msitarzewski#785, msitarzewski#786, msitarzewski#815, msitarzewski#816.

Fixes found while integrating: Bash 3.2 guard for the msitarzewski#755 worker argv,
Windsurf re-conversion over a stale .windsurfrules, locale-independent
check-divisions.sh, agency- prefix handling in the outputs eval, and the
India Business Navigator's YAML and headings. Resolves upstream issues
msitarzewski#229, msitarzewski#763, msitarzewski#821 and msitarzewski#1027.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwgfpJ9tGbUh5g84u5VSgv
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