Skip to content

fix(factorydroid): follow four documented Droid surfaces - #2572

Merged
dyoshikawa merged 3 commits into
mainfrom
resolve-issue-2500-factorydroid-gaps
Aug 4, 2026
Merged

fix(factorydroid): follow four documented Droid surfaces#2572
dyoshikawa merged 3 commits into
mainfrom
resolve-issue-2500-factorydroid-gaps

Conversation

@dyoshikawa

Copy link
Copy Markdown
Owner

Summary

Resolves the four Factory Droid divergences in #2500. All four were re-verified against the live docs before implementing.

1. disabledSkills permission key

The settings docs list disabledSkills ("Disable discovered skills without deleting their files.", an array of skill names) alongside the already-supported hooksDisabled. It is now in FACTORYDROID_OVERRIDE_KEYS, so it can be authored in the factorydroid override and is lifted back into it on import.

2. Skills frontmatter: enabled and allowed-tools

The skills docs table documents 11 fields; rulesync modeled 4. enabled (default true, "Set to false to keep the skill on disk but disable it") and allowed-tools ("Declares the tools the skill is designed to use") are now in FactorydroidSkillFrontmatterSchema and in the factorydroid section of the rulesync skill schema, threaded through both conversion directions — looseObject alone was not enough, since both directions build explicit literals.

3. Hooks commandRegex

Droid documents commandRegex as an "Additional regex filter for Execute commands" sitting next to matcher on the group: {"matcher": "Execute", "commandRegex": "^git ", "hooks": [...]}. Added to HookDefinitionSchema and declared as a groupPassthroughFields entry for Droid. That mechanism previously carried plain objects only (AugmentCode metadata), so it was generalized to carry scalars too; the existing disagreement warning and the object path are unchanged.

4. Commands: drop allowed-tools

The custom-slash-commands docs list only description and argument-hint, and state verbatim: "Tool scoping is not available for custom commands. Use Skills or Custom Droids for tool policy." allowed-tools is dropped on generate rather than written as frontmatter Droid ignores, matching the #2453 precedent of not emitting inert configuration. The stale schema comment and the commands row in the research reference map are corrected.

Test plan

  • New unit tests for each gap: disabledSkills round-trip, skills enabled/allowed-tools in both directions, commandRegex emitted on the group (and not duplicated onto the hook) plus imported onto every hook in the group, and the commands drop.
  • npx vitest run --config vitest.e2e.config.ts src/e2e/e2e-hooks.spec.ts src/e2e/e2e-skills.spec.ts src/e2e/e2e-permissions.spec.ts src/e2e/e2e-commands.spec.ts — 377 passed.
  • pnpm cicheck (full) passes; pnpm run generate:schema and pnpm run generate:docs-content regenerated.

Closes #2500

🤖 Generated with Claude Code

Add disabledSkills to the permissions override keys, carry the skills-frontmatter enabled and allowed-tools fields in both directions, forward the hooks commandRegex filter at matcher-group level, and stop emitting allowed-tools on custom commands, which Droid documents as unsupported ("Tool scoping is not available for custom commands"). Carrying commandRegex needed the group-passthrough mechanism, previously object-only, to accept scalars as well.
Address review feedback. commandRegex narrows when a hook fires, so sharing the group's first value would silently stop a hook that asked for no filter from firing at all. Group passthrough fields now declare whether they subdivide the group; Droid's commandRegex does, while AugmentCode's additive metadata keeps its existing merge-with-warning behavior.

Group passthrough fields also declare their value shape, so a string field no longer accepts an object (or the reverse) and write a value that fails canonical validation on the next generate; string values are checked for control characters like the array passthrough path already was. Finally, allowed-tools is dropped on import as well as on generate, so a round trip cannot reintroduce a field Droid ignores.
The separator was written as a literal NUL rather than an escape, which made git treat tool-hooks-converter.ts as binary — the central change of this PR was unreadable in git diff, gh pr diff, and the GitHub review UI, while every CI check still passed. Also key the group on validated values only, so a value the tool cannot express does not split a group into entries that come out identical, and pin the value-shape guard with an AugmentCode regression test.
@dyoshikawa
dyoshikawa merged commit f4dbdea into main Aug 4, 2026
9 checks passed
@dyoshikawa
dyoshikawa deleted the resolve-issue-2500-factorydroid-gaps branch August 4, 2026 03:33
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.

Follow up Factory Droid upstream updates: disabledSkills key, enabled skill frontmatter, commandRegex hook filter, inert allowed-tools on commands

2 participants