Skip to content

🪜 chore: Plumb allowedTools through resolveManualSkills#12744

Merged
danny-avila merged 2 commits intofeat/agent-skillsfrom
feat/skills-resolved-allowed-tools-precursor
Apr 19, 2026
Merged

🪜 chore: Plumb allowedTools through resolveManualSkills#12744
danny-avila merged 2 commits intofeat/agent-skillsfrom
feat/skills-resolved-allowed-tools-precursor

Conversation

@danny-avila
Copy link
Copy Markdown
Owner

Summary

Tiny shape-only precursor shared by Phase 5 (always-apply frontmatter) and Phase 6 (frontmatter runtime enforcement). Lands first to eliminate the type-shape race between the two phases so they can proceed in parallel.

  • Adds allowedTools?: string[] to ResolvedManualSkill.
  • Widens the getSkillByName return type in ResolveManualSkillsParams and InitializeAgentDbMethods to carry the same field.
  • resolveManualSkills forwards skill.allowedTools verbatim when present; absent otherwise.

No runtime behavior change. The field is populated from the skill doc but not yet consumed by any downstream code path. Phase 6 will union the per-skill allowlist into the agent's effective tool set for the turn; Phase 5's ResolvedAlwaysApplySkill will mirror this exact shape.

Scoped intentionally small:

  • Three files, 65 additions, one deletion.
  • No schema change — Phase 6 owns the structured column (skill.allowedTools) and the frontmatter importer mapping.
  • No changes to injectManualSkillPrimes, call sites in initialize.ts, or JS controllers — downstream consumers only read name and body.

Why a precursor

From the umbrella rollout plan: Phase 5 and Phase 6 both need ResolvedManualSkill.allowedTools in their resolver pipeline. Landing the shape here means neither phase has to own the interface addition, and merge conflicts between their branches stay textual.

Test plan

  • resolveManualSkills forwards allowedTools when the skill doc declares it.
  • resolveManualSkills omits allowedTools when the field is absent (no spurious key on the resolved object).
  • resolveManualSkills preserves an empty allowedTools: [] array (distinguishes "explicitly declared none" from "undeclared" for Phase 6 semantics).
  • All 78 existing skills.test.ts tests pass unchanged.
  • All 14 existing initialize.test.ts tests pass unchanged.
  • tsc --noEmit clean.
  • ESLint clean on the three changed files.

Base branch

Targets feat/agent-skills (umbrella PR #12625), not dev.

Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6
(frontmatter runtime enforcement). Adds `allowedTools?: string[]` to
`ResolvedManualSkill` and widens the `getSkillByName` return type in
`ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the
same field. The resolver forwards `skill.allowedTools` verbatim when
present.

No runtime behavior change — the field is populated but not yet consumed.
Phase 6 will union the per-skill allowlist into the agent's effective
tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror
this shape. Landing this first eliminates the type-shape race between
the two phases.
@danny-avila danny-avila changed the title 🪜 chore: Plumb allowedTools through resolveManualSkills (Phase 5/6 Precursor) 🪜 chore: Plumb allowedTools through resolveManualSkills Apr 19, 2026
@danny-avila danny-avila mentioned this pull request Apr 19, 2026
JSDoc that cites "Phase N will X" goes stale the moment that phase ships.
Swap for "future runtime enforcement" so the docs age with the code.
@danny-avila danny-avila merged commit 4d74cd3 into feat/agent-skills Apr 19, 2026
2 checks passed
@danny-avila danny-avila deleted the feat/skills-resolved-allowed-tools-precursor branch April 19, 2026 17:03
danny-avila added a commit that referenced this pull request Apr 21, 2026
* 🪜 chore: Plumb `allowedTools` through `resolveManualSkills`

Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6
(frontmatter runtime enforcement). Adds `allowedTools?: string[]` to
`ResolvedManualSkill` and widens the `getSkillByName` return type in
`ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the
same field. The resolver forwards `skill.allowedTools` verbatim when
present.

No runtime behavior change — the field is populated but not yet consumed.
Phase 6 will union the per-skill allowlist into the agent's effective
tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror
this shape. Landing this first eliminates the type-shape race between
the two phases.

* 🪜 style: Drop phase-name refs from `allowedTools` JSDoc for longevity

JSDoc that cites "Phase N will X" goes stale the moment that phase ships.
Swap for "future runtime enforcement" so the docs age with the code.
danny-avila added a commit that referenced this pull request Apr 22, 2026
* 🪜 chore: Plumb `allowedTools` through `resolveManualSkills`

Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6
(frontmatter runtime enforcement). Adds `allowedTools?: string[]` to
`ResolvedManualSkill` and widens the `getSkillByName` return type in
`ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the
same field. The resolver forwards `skill.allowedTools` verbatim when
present.

No runtime behavior change — the field is populated but not yet consumed.
Phase 6 will union the per-skill allowlist into the agent's effective
tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror
this shape. Landing this first eliminates the type-shape race between
the two phases.

* 🪜 style: Drop phase-name refs from `allowedTools` JSDoc for longevity

JSDoc that cites "Phase N will X" goes stale the moment that phase ships.
Swap for "future runtime enforcement" so the docs age with the code.
danny-avila added a commit that referenced this pull request Apr 22, 2026
* 🪜 chore: Plumb `allowedTools` through `resolveManualSkills`

Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6
(frontmatter runtime enforcement). Adds `allowedTools?: string[]` to
`ResolvedManualSkill` and widens the `getSkillByName` return type in
`ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the
same field. The resolver forwards `skill.allowedTools` verbatim when
present.

No runtime behavior change — the field is populated but not yet consumed.
Phase 6 will union the per-skill allowlist into the agent's effective
tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror
this shape. Landing this first eliminates the type-shape race between
the two phases.

* 🪜 style: Drop phase-name refs from `allowedTools` JSDoc for longevity

JSDoc that cites "Phase N will X" goes stale the moment that phase ships.
Swap for "future runtime enforcement" so the docs age with the code.
danny-avila added a commit that referenced this pull request Apr 25, 2026
* 🪜 chore: Plumb `allowedTools` through `resolveManualSkills`

Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6
(frontmatter runtime enforcement). Adds `allowedTools?: string[]` to
`ResolvedManualSkill` and widens the `getSkillByName` return type in
`ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the
same field. The resolver forwards `skill.allowedTools` verbatim when
present.

No runtime behavior change — the field is populated but not yet consumed.
Phase 6 will union the per-skill allowlist into the agent's effective
tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror
this shape. Landing this first eliminates the type-shape race between
the two phases.

* 🪜 style: Drop phase-name refs from `allowedTools` JSDoc for longevity

JSDoc that cites "Phase N will X" goes stale the moment that phase ships.
Swap for "future runtime enforcement" so the docs age with the code.
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.

1 participant