feat(skills): preserve runtime ids with display metadata (#9)

[kojiwakayama]

Summary

Ready for human review. Current-head CI is green.

Implements the Code/runtime side of #9:

• separates canonical runtime-safe skill identity from human display labels
• keeps runtime skill name equal to the canonical id/folder identity
• preserves display-style labels as displayName / metadata.display_name
• validates project skill ids and provider-safe ids before runtime registration
• keeps prompt augmentation, discovery, CLI validation, and runtime catalog behavior aligned

This pull request is feature-only. Publish it later through a dedicated release pull request based on the then-current main branch.

Review remediation

Current review-fix commit:

• f976217cc94eb99da4b2b5bce8eb7c36c57b7f28 - keeps CLI validation aligned with runtime identity
• 2e9aa7b2d54e7e41d550b35565b02853e066f097 - clarifies owned skill selector diagnostics

Resolved review findings:

• Default validation path . now resolves before taking the directory basename, so veryfront skills validate validates against the actual current skill directory name.
• Legacy/display-style frontmatter names are preserved as display metadata, matching runtime parser behavior, while canonical names are still checked against directory identity.
• Invalid selectors that look like owned/provider-scoped skill ids now report provider-safe owned-id guidance. Plain invalid selectors keep the existing canonical skill-id guidance.

#9 contract / runtime evidence

The runtime contract is explicit:

• canonical skill ids remain runtime-safe (SKILL_NAME_REGEX for local skills, provider-safe ids where owner/provider scope applies)
• buildRuntimeSkillDefinition returns name as canonical id and optional displayName from metadata.display_name or legacy display-style frontmatter
• discovery preserves directory/folder identity as the attachable skill id instead of using display labels
• prompt augmentation and runtime skill metadata expose display names without changing callable ids
• CLI skill validation rejects canonical frontmatter names that do not match the directory name
• whitespace around directory canonical identity is rejected, not normalized

Current branch state

Base: main at a57e8c41ef8b40b64843adfc4a4861a540c4caaf
Head: issue-9/skill-display-runtime-identity-code-20260729-review-ready at 2e9aa7b2d54e7e41d550b35565b02853e066f097

Local remediation commits:

• 84e0d2dd2d5129c82bd2bf6a6ae3ad950e64d356 - removes stale release ownership and restores both version files to current-main 0.1.1175
• 1938252f66209029c3d2ee35205576f3948b6ea1 - normal two-parent merge from current origin/main
• f976217cc94eb99da4b2b5bce8eb7c36c57b7f28 - fixes the two skills validate review findings
• 2e9aa7b2d54e7e41d550b35565b02853e066f097 - fixes the remaining owned-selector diagnostic review finding

Verification

TDD red regression run:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all cli/commands/skills/validate.test.ts

Result: failed before the implementation with the two expected regressions:

• Skill name "code-review" does not match directory name "."
• Invalid skill name "Process Email": must be lowercase alphanumeric with hyphens, 1-64 characters

Focused CLI validate green run:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all cli/commands/skills/validate.test.ts

Result: 1 passed (7 steps), 0 failed.

Focused skill identity suite:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all \
  cli/commands/skills/validate.test.ts \
  src/agent/runtime/project-skill-catalog.test.ts \
  src/agent/runtime/skill-metadata.test.ts \
  src/agent/runtime/skill-prompt.test.ts \
  src/discovery/agent-scoped-capabilities.test.ts \
  src/discovery/skill-discovery.test.ts \
  src/skill/parser.test.ts \
  src/skill/skill-owner-scope.test.ts \
  tests/e2e/features/skill-capabilities.test.ts

Result: 77 passed (43 steps), 0 failed.

Owned selector diagnostic TDD red run:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all src/skill/skill-owner-scope.test.ts

Result: failed before the implementation because writer--Bad Name received the canonical-only message: must be lowercase alphanumeric with hyphens, 1-64 characters.

Owned selector diagnostic green run:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all src/skill/skill-owner-scope.test.ts

Result: 13 passed, 0 failed.

Focused skill tool/owner/Task 13 suite:

VF_DISABLE_LRU_INTERVAL=1 DENO_TESTING=1 NODE_ENV=production deno test --no-check --allow-all \
  src/skill/tools.test.ts \
  src/skill/skill-owner-scope.test.ts \
  cli/commands/skills/validate.test.ts \
  src/agent/runtime/project-skill-catalog.test.ts \
  src/agent/runtime/skill-metadata.test.ts \
  src/agent/runtime/skill-prompt.test.ts \
  src/discovery/agent-scoped-capabilities.test.ts \
  src/discovery/skill-discovery.test.ts \
  src/skill/parser.test.ts \
  tests/e2e/features/skill-capabilities.test.ts

Result: 79 passed (56 steps), 0 failed.

Required script hook:

./scripts/hooks/pre-push

Result: failed at deno task docs:validate because current origin/main is missing ./release-assets API reference coverage:

• missing @example in src/release-assets/index.ts
• missing docs/api-reference/veryfront/release-assets.md

Baseline reproduction on exact origin/main a57e8c41ef8b40b64843adfc4a4861a540c4caaf reproduced the same release-assets docs failure. scripts/docs/docs-coverage.test.ts also failed on both branch and origin/main with missing release-assets.

Later hidden stages after the docs failure:

• deno run --allow-read scripts/docs/validate-guides.ts passed
• deno run --allow-read scripts/docs/validate-public-docs.ts passed
• deno test --no-check --allow-read tests/docs/guide-contracts.test.ts tests/docs/guide-content.test.ts passed, 2 passed (82 steps), 0 failed
• deno test --no-check --allow-all tests/docs/guide-examples.test.ts tests/docs/guide-code-examples.test.ts passed, 45 passed (87 steps), 0 failed
• deno run -A scripts/lint/check-doc-links.ts passed, All 712 doc links OK
• deno task typecheck passed

Exact hook Playwright stage:

npx playwright test --config=tests/e2e/playwright.config.ts

Result: failed on both branch and exact origin/main because tests/e2e/playwright.config.ts does not exist.

Canonical Playwright substitute:

deno task test:e2e:playwright

Result: failed on both branch and exact origin/main with duplicate local Playwright package loading (Requiring @playwright/test second time). This is a local infrastructure baseline, so current-head CI is the equivalent gate for the Playwright coverage.

Normal guarded push:

• first remote-head guard passed against captured head 1938252f66209029c3d2ee35205576f3948b6ea1
• second remote-head guard passed against captured head f976217cc94eb99da4b2b5bce8eb7c36c57b7f28
• git push origin HEAD:issue-9/skill-display-runtime-identity-code-20260729-review-ready used no force and no bypass
• installed pre-push hook passed: formatting, lint, typecheck, and unit tests
• unit result from push hook: 2736 passed (22144 steps), 0 failed
• second push hook unit result: 2737 passed (22144 steps), 0 failed

Current-head CI:

• gh pr checks 3171 --watch --interval 10 completed on head 2e9aa7b2d54e7e41d550b35565b02853e066f097.
• Passed: CodeQL, format, lint, typecheck, CLA, coverage gate, coverage shards 1-8, unit, integration, binary e2e, npm install smoke, RSC browser e2e, language analysis jobs
• Skipped: release, prerelease, update-homebrew, build-binaries, split-mode/version jobs

Explicit exclusions

• fix: embed framework sources in cross-compiled binaries #278 is intentionally excluded from this PR.
• This PR does not own a package version bump, GitHub release, or tag.
• Studio remains separate and unpushed pending its own verification path.
• Required non-author approval may remain as the external readiness gate.

---

Replacement for #3157. This pull request keeps the exact verified head and preserves Kentaro's review request.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1938252f66

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[Copilot]

Pull request overview

Implements runtime-safe canonical skill identity while preserving human-readable display labels across discovery, prompt augmentation, runtime skill metadata, and tooling. This aligns the code/runtime behavior with the #9 contract by keeping callable ids stable (directory/owned id) and surfacing display labels via displayName and metadata.display_name.

Changes:

• Treats the directory (or owned/provider-safe namespaced id) as the canonical skill id/name, and derives optional displayName from metadata.display_name or legacy frontmatter.name.
• Updates runtime/tooling prompt surfaces to show display labels without changing lookup ids, and adds id validation for unresolved selectors.
• Expands/updates unit + e2e coverage for canonical-id lookup, legacy display-name preservation, and owned/provider-safe ids.

Verification (as reported in PR description):

• Focused suite: 77 passed (41 steps), 0 failed
• Pre-push hook: unit suite 2736 passed (22142 steps), 0 failed (docs coverage failure reproduced on base)

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/e2e/features/skill-capabilities.test.ts	Adds an e2e smoke test ensuring canonical-id lookup works while display labels are preserved and not accepted as ids.
src/skill/types.ts	Introduces SKILL_PROVIDER_SAFE_ID_REGEX and adds optional displayName to SkillMetadata.
src/skill/tools.ts	Improves unresolved-skill errors by validating selector shape separately from not-found, and supports provider-safe ids.
src/skill/skill-owner-scope.test.ts	Adds coverage for resolving owned short names (underscores) prior to plain-id validation.
src/skill/prompt-augmentation.ts	Renders skill manifest entries using displayName plus canonical id when available.
src/skill/parser.ts	Makes directory name the canonical name, preserves legacy/display metadata as displayName, and supports provider-safe canonical ids via an option.
src/skill/parser.test.ts	Adds tests for legacy display-name preservation, metadata precedence, and stricter canonical directory validation.
src/discovery/skill-discovery.test.ts	Ensures discovery keys skills by canonical directory id while preserving display metadata.
src/discovery/handlers/skill-handler.ts	Removes now-obsolete warning path and documents directory identity as canonical.
src/discovery/agent-scoped-capabilities.ts	Validates agent-scoped skill ids as provider-safe when building from directory agents.
src/discovery/agent-scoped-capabilities.test.ts	Adds dotted-agent owned-skill coverage and updates empty DiscoveryResult shape.
src/agent/runtime/skill-prompt.ts	Uses displayName for prompt labels instead of overloading name.
src/agent/runtime/skill-prompt.test.ts	Updates expectations and adds a regression test ensuring canonical ids do not appear as display labels.
src/agent/runtime/skill-metadata.ts	Adds metadata normalization, emits displayName, enforces canonical id validity, and preserves structured metadata.
src/agent/runtime/skill-metadata.test.ts	Adds coverage for metadata parsing, legacy display name recovery, invalid id rejection, and provider-safe owned ids.
src/agent/runtime/project-skill-catalog.test.ts	Validates catalog behavior for provider-safe colocated ids (dotted agent ids).
scripts/lint/test-typecheck-baseline.json	Removes a typecheck baseline exclusion now that the test file matches current types.
cli/commands/skills/validate.ts	Adds canonical-name validation against directory name prior to parsing/validation.
cli/commands/skills/validate.test.ts	Updates CLI tests for directory-name matching validation behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/skill/tools.ts:106

• load_skill treats provider-safe owned short names (for example x_y) as invalid when they are not found, because isUnresolvedSkillSelectorValid() only allows SKILL_NAME_REGEX or provider-safe ids that include --. Owned short names are a supported selector (resolved first via skillRegistry.resolveVisibleSkill()), so the not-found path should consider them valid and return the normal "not found" error. Also, the invalid-id message should mention both canonical ids and provider-safe owned ids/short names.

  if (!isUnresolvedSkillSelectorValid(skillId)) {
    throw toError(
      createError({
        type: "agent",
        message:
          `Invalid skill id "${skillId}": must be lowercase alphanumeric with hyphens, 1-64 characters`,

src/skill/types.ts:50

• The SkillMetadata.name doc comment says the identifier is "lowercase, hyphenated", but this PR introduces provider-safe owned skill ids (for example a_b--x_y) that can include underscores and uppercase. Updating the comment avoids misleading API documentation.

  /** Skill identifier (lowercase, hyphenated) */
  name: string;
  /** Optional human-readable label; never used for skill lookup/reference. */
  displayName?: string;

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/skill/types.ts:47

• SkillMetadata.name is documented as "lowercase, hyphenated", but the PR introduces provider-safe owned skill ids (for example a_b--x_y) that legitimately include underscores. The comment should describe the broader contract to avoid confusing API consumers.

  /** Skill identifier (lowercase, hyphenated) */

src/skill/parser.ts:85

• validateSkillMetadata treats its second argument as the canonical skill id (and validates it directly), but the parameter is still named directoryName, which is misleading (callers can pass computed ids like provider-safe owned ids). Renaming it (and updating the JSDoc) would make the contract clearer and reduce confusion for future callers.

export function validateSkillMetadata(
  frontmatter: Record<string, unknown>,
  directoryName: string,
  options: { providerSafeName?: boolean } = {},
): SkillMetadata {

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/skill/tools.ts:138

• isUnresolvedSkillSelectorValid only treats provider-safe ids as valid when they contain the owned-skill -- separator. That makes provider-safe owned short-name selectors (for example x_y) report Invalid skill id when they are missing, even though underscores are used for owned short names (see skill-owner-scope.test.ts with shortName: "x_y"). Consider treating any provider-safe selector as syntactically valid so missing owned short names fall through to the existing "Skill not found" path.

    return true;
  }

  return skillId.includes("--") && SKILL_PROVIDER_SAFE_ID_REGEX.test(skillId);
}

src/skill/types.ts:48

• SkillMetadata.name is documented as "lowercase, hyphenated", but validateSkillMetadata(..., { providerSafeName: true }) now allows provider-safe owned ids (for example a_b--x_y) which can include underscores and uppercase. Updating the comment to reflect both canonical local ids and provider-safe owned ids will keep the type docs accurate.

  /** Skill identifier (lowercase, hyphenated) */
  name: string;

src/skill/types.ts:18

• The comment for SKILL_PROVIDER_SAFE_ID_REGEX says it is an "owned skill id: sanitized namespace + short name", but the regex itself is a general provider-safe identifier check and does not enforce a namespace separator or segment structure. Updating the comment to match what the regex actually validates will avoid confusion for future callers.

This issue also appears on line 47 of the same file.

/** Provider-safe owned skill id: sanitized namespace + short name, max 64 chars */
export const SKILL_PROVIDER_SAFE_ID_REGEX = /^[A-Za-z0-9_-]{1,64}$/;