Skip to content

Address skills by location and unify filesystem and PDO resource resolution - #12

Draft
asterixcapri wants to merge 31 commits into
1.xfrom
feature/unified-skill-storage
Draft

asterixcapri wants to merge 31 commits into
1.xfrom
feature/unified-skill-storage

Conversation

@asterixcapri

@asterixcapri asterixcapri commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Select skills by their exact catalog location so skills with the same declared name can coexist across filesystem and database sources. Locations have no trailing slash; callers pass resource paths separately as literal filenames.

- $repository->get('writing');
+ $repository->get('file:///app/skills/writing');
+ $matches = $repository->findByName('writing'); // list<Skill>, or []
+ new DatabaseSkillStorage($pdo);
  $skill->readResource('references/my guide.md');

Both adapters use ResourceLocator to translate locations under a base URI into skill identifiers and relative resource paths. Location segments are URI encoded; spaces and percent signs in resource paths remain literal. findByName() returns all exact, case-sensitive matches without replacing location-based get().

Filesystem storage accepts an absolute directory or a file:// input, requires an existing directory, and resolves its root with realpath(). Reads resolve each path once and reject resources escaping the selected skill through traversal or symlinks. Invalid skill documents are skipped with diagnostics containing skillLocation and message.

Add PDO storage over application-owned (skill_identifier, path, content) rows, a combined runnable demo with SQLite setup, and updated documentation and glossary. Storage reads return PHP strings, including empty and binary content.

Evidence

  • Before (1.x): discovery returns skill identifiers, selection uses declared names, and later same-name skills are shadowed.
    After: exact catalog locations select their owning storage; same-name skills coexist, findByName() returns every match, duplicate locations fail, and missing resources never fall back to another source.
  • Regression tests cover locations without trailing slashes, rejection of trailing-slash aliases, spaces, Unicode, literal percent sequences, database resource directory segments, missing filesystem roots, sibling-skill traversal, escaping symlinks, and execution from a decoded local skill location.
  • composer check: 241 tests, 718 assertions; PHPStan reports no errors.

Merge Danger

Door: two-way

Code can be reverted; the library performs no schema creation or database migration.

Blast Radius: consumers

The public contract changes relative to 1.x: get() selects by location, storage discovery and reads use locations without trailing slashes, SkillStorageInterface::location() is removed, and tool inputs use location instead of name. The Skill constructor order is name, description, location, storage. Diagnostics identify their source with skillLocation.

Filesystem roots must already exist; published locations use the root resolved by realpath(). Custom adapters and integrations need the migration described in the README. Database collation and exact row identity are the application's schema responsibility.

@asterixcapri
asterixcapri marked this pull request as draft October 7, 2026 06:03
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