Skip to content

fix: writers emit tags as JSON array frontmatter (skill/command/subagent)#202

Merged
tylergraydev merged 1 commit into
tylergraydev:mainfrom
prefrontalsys:fix/writers-emit-tags-as-json-arrays
Apr 22, 2026
Merged

fix: writers emit tags as JSON array frontmatter (skill/command/subagent)#202
tylergraydev merged 1 commit into
tylergraydev:mainfrom
prefrontalsys:fix/writers-emit-tags-as-json-arrays

Conversation

@prefrontalsys

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #200. Closes the same round-trip failure class that PR caught for rule_writer, applied to the three other flat-.md writers.

The DB readers for skills, commands, and sub-agents deserialise the tags column via serde_json::from_str — see db/schema.rs:1727, :1860, :2009. The three matching writers silently dropped tags from frontmatter entirely. No live corruption today because no scanner re-ingests these primitives — but given #200 closed the "every primitive has a disk scanner except rules" asymmetry, it's reasonable to assume scanners for skills/commands/agents will follow. At that point, disk round-trip would either lose tags or (if anyone ships a comma-joined stopgap) silently corrupt them the same way rule_writer did pre-#200.

Changes

File Change
src-tauri/src/services/skill_writer.rs generate_skill_markdown emits tags: <json-array> when non-empty
src-tauri/src/services/command_writer.rs generate_command_markdown emits tags: <json-array> when non-empty
src-tauri/src/services/subagent_writer.rs generate_subagent_markdown emits tags: <json-array> when non-empty

Each writer gains two tests: one pins the JSON-array emission shape, one pins the empty-vec skip path.

No schema change, no new dependency, no public-API change.

Test plan

  • cargo fmt --check passes
  • cargo test --lib — full suite passes locally, 6 new tests added
  • cargo test --lib writer — 299/299 writer tests pass
  • End-to-end: UI-created skill/command/subagent with tags round-trips to disk as JSON-array form (no live scanner to ingest yet, so this is visual verification only)

Not in scope (intentionally)

paths, allowed_tools, tools, skills, disallowed_tools in these same writers still use comma-joined form. Same latent round-trip shape, but expanding this PR would:

  • Bloat the diff from three symmetric 7-line emissions to a dozen heterogeneous changes
  • Require deciding a canonical form before the scanners exist (e.g. allowed_tools space-separated? comma with CSV quoting?), which is better decided with the scanner PR that'll consume the format

Separate PRs when their scanners land.

Relation to #200

This PR branches from main (not from the #200 branch) — zero file overlap with #200's diff. Safe to review and merge independently in either order.

…ent)

Mirrors the round-trip bug tylergraydev#200 caught for rule_writer: the DB reader for
skills, commands, and sub-agents deserialises the `tags` column via
`serde_json::from_str` (see `db/schema.rs:1727`, `:1860`, `:2009`), but
the three corresponding writers silently dropped the `tags` field from
frontmatter entirely. No live bug today — no scanner re-ingests these
primitives — but once scanners do (and given the asymmetry tylergraydev#200 closed,
that direction is likely), disk round-trip would either lose tags or, if
someone shipped a comma-joined stopgap, silently corrupt them.

Fixes:
- skill_writer::generate_skill_markdown emits `tags: <json>` when non-empty
- command_writer::generate_command_markdown emits `tags: <json>` when non-empty
- subagent_writer::generate_subagent_markdown emits `tags: <json>` when non-empty

Each writer gains two tests: one pinning the JSON-array shape, one pinning
the empty-vec skip path. Full `cargo test --lib` passes.

Not touched: `paths`, `allowed_tools`, `tools`, `skills`, `disallowed_tools`
in the same writers still use comma-joined form. They have the same latent
shape but are out of scope for this PR (would expand the diff; can land
separately once their scanners exist and decide on a canonical form).
@tylergraydev tylergraydev merged commit 63671e3 into tylergraydev:main Apr 22, 2026
6 of 10 checks passed
tylergraydev pushed a commit that referenced this pull request May 4, 2026
User-facing additions:
- Model picker now uses Claude Code aliases (opus/sonnet/haiku/opusplan/best)
  with a 1M context toggle and a free-form "Other (custom model ID)" field
  for Bedrock ARNs / Vertex / Foundry IDs (#213).
- Rules dropped into ~/.claude/rules/ or {project}/.claude/rules/ now show
  up in the UI via new global/project scanners (#200).

Fixes:
- Sync Config no longer wipes externally-managed configs (.mcp.json,
  claude.json, OpenCode/Copilot/Cursor/Gemini/Codex) — extends the #191
  guard to all 7 writers and imports .mcp.json on project add (#204).
- Skill/command/sub-agent writers emit `tags` as JSON arrays so they
  round-trip through the DB reader (#202).
- PR builds from forks/Dependabot no longer fail on missing
  TAURI_SIGNING_PRIVATE_KEY (#203).
- delete_rule_inner returns real DB errors instead of swallowing them as
  "row absent" (follow-up to #200).

Internal:
- Cargo clippy from 134 warnings to 0; CI now enforces -D warnings on
  --lib --tests (#205).

All 1565 frontend tests and 2021 Rust tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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