Skip to content

feat(fix): auto-fix deprecated-tools-field by renaming tools: to allowed-tools: - #45

Merged
protosphinx merged 1 commit into
mainfrom
bot/fix-deprecated-tools-field
Jul 20, 2026
Merged

feat(fix): auto-fix deprecated-tools-field by renaming tools: to allowed-tools:#45
protosphinx merged 1 commit into
mainfrom
bot/fix-deprecated-tools-field

Conversation

@protosphinx

Copy link
Copy Markdown
Member

Why

--fix mode currently skips the deprecated-tools-field warning: when a
skill uses only the legacy tools: key (instead of the spec-supported
allowed-tools:), running skillcheck --fix detects the problem but makes
no change. Authors are left to rename the key by hand.

The rename is unambiguous and safe - it is the only correct resolution when
tools: is the sole tool-list key - so it qualifies for auto-fix.

What

  • src/fix.ts: new deprecated-tools-field branch in applyFixes; new
    helper rewriteFrontmatterRenameToolsToAllowedTools that rewrites the
    tools: key to allowed-tools: in place, preserving all list items,
    surrounding fields, and formatting. Handles both block and inline YAML
    forms.
  • Updated the applyFixes doc comment to list deprecated-tools-field
    alongside the other supported fixes.
  • test/fix.test.ts: 6 new tests covering block form, inline form, field
    preservation, dry run, no-op when the key is absent, and notes content.

Tests

  • npm test - 192 tests pass (186 existing + 6 new)
  • npm run lint - clean
  • npm run build - clean

Self-merge gate

  • all CI checks pass
  • LOC delta < 250 (171 lines added)
  • no public-API surface change (src/index.ts not modified)
  • no runtime-dependency additions
  • no workflow file changes
  • tests added or extended (6 new tests in test/fix.test.ts)

Generated by Claude Code

…wed-tools:

When the deprecated-tools-field warning fires (only the legacy tools: key
is present, with no allowed-tools:), applyFixes now rewrites the key in
place to allowed-tools:, preserving all list items, surrounding fields, and
formatting. Block and inline YAML forms are both handled. Dry-run mode and
notes output follow the same pattern as the other fix rules.

Adds 6 tests covering block form, inline form, field preservation, dry run,
no-op when the key is absent, and notes content.
@protosphinx protosphinx added the automated Opened by the daily bot label Jul 20, 2026 — with Claude
@protosphinx
protosphinx merged commit 8464154 into main Jul 20, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34a03db5de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/fix.ts
// Match only the `tools:` key line itself; leave indented list items alone.
const toolsKeyRe = /^([ \t]*)tools([ \t]*:)/m;
if (!toolsKeyRe.test(block)) return raw;
const newBlock = block.replace(toolsKeyRe, "$1allowed-tools$2");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Serialize tools arrays before renaming

When a legacy tools: value is a YAML array (both block lists and [Read, Write], which LegacyToolsField accepts), this replacement leaves the value as a YAML sequence under allowed-tools:. The schema parses allowed-tools as a string-only field in src/schema.ts, so a file that previously only had a deprecated-tools-field warning is rewritten into one that fails frontmatter-schema on the next lint/run. The fixer should serialize array values to the allowed-tools string form instead of just replacing the key.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Opened by the daily bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant