mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
main
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
06e16de066 |
chore(skills): drop Anthropic attribution from SKILL.md spec references
Two related cleanups landed together because they touch the same surface
(skill-spec uplift PRs #569/#570/#571/#572):
1. Wording: replace "Anthropic spec" / "Anthropic Claude Code skill spec"
with "SKILL.md spec" across admin UI tooltips, code comments, test
docstrings, migration 056's module docstring, and the user-facing
`arguments` description in tools/skills.json. Renames a parser test
`test_anthropic_tags` -> `test_nested_metadata_tags` and consolidates
a parse-API test of the same shape; fixture author renamed
`Anthropic` -> `Acme` to keep the fixture neutral. Legitimate
provider/SDK/API references (provider name, api.anthropic.com,
`_anthropic.py`, capability comments) are intentionally untouched.
2. Admin UX: in the Create + Edit Skill modals, six fields per modal
(Compatibility, Paths, Hide-from-skill-picker, Arguments, Argument
hint, Activation) had long uppercase label-hint spans crammed into
the visible label. Migrated each to the existing
`.settings-help-btn` + `.settings-help-popover` pattern already used
in the Settings tab — short label + inline `?` button that opens a
styled popover with proper `<code>` formatting for technical tokens.
Pattern reuse required two small generalisations in admin.js:
* `_toggleSettingsHelp` now looks up the popover via a new
`data-help-target="<id>"` attribute first, falling back to the
settings-tab `.settings-label-col` ancestor lookup.
* `_closeAllSettingsHelp` mirrors the same dual-path lookup when
resetting `aria-expanded`, so modal buttons don't get stuck on
`aria-expanded="true"` after another popover opens.
* Added a document-delegated click handler that fires only for
buttons with `data-help-target`; existing per-button binding
in the settings-tab render path is unchanged.
CSS: `.settings-help-btn` now paints its `?` via `::after` with the
button's own `font-size: 0`, so prettier-introduced whitespace
inside the new HTML buttons can't off-center the glyph. The same
rule applies to existing admin.js-generated buttons (text content
hidden, pseudo identical). Small additions for
`.settings-help-popover code` / `strong` styling so technical
tokens render with the same monospace pill treatment used elsewhere
in skill UI.
Known follow-ups (intentionally NOT in this PR):
* Migrate the settings-tab `_renderSettingRow` button assembly to the
empty-`<button>` + `data-help-target` form so the per-button
addEventListener loop can be dropped in favour of pure document
delegation, and the `font-size: 0` rule stops being a workaround for
two markup styles.
* The 12 new popover blocks are duplicated verbatim between the
Create and Edit modals (same as the rest of the create/edit modal
pair). A small renderer that emits popovers from a shared data
object would eliminate the drift risk but is unrelated cleanup.
|
||
|
Patrick Buckley
|
9309162ac0 |
feat(skills): wire \$ARGUMENTS / \$N / \$<name> / \${CLAUDE_*} substitution (#572)
Implements the Anthropic Claude Code skill spec's placeholder
substitution end to end. The renderer in ``_substitute_skill_args``
handles every spec form except ``\${CLAUDE_SKILL_DIR}`` (deferred):
* ``\$ARGUMENTS`` — full args string as the user/model typed it
* ``\$ARGUMENTS[N]`` / ``\$N`` — Nth positional arg, ``shlex.split``-parsed
* ``\$<name>`` — named arg from the SKILL.md ``arguments:`` list
* ``\${CLAUDE_SESSION_ID}`` / ``\${CLAUDE_EFFORT}`` — session state
Substitution is single-pass (one combined regex, one ``re.sub``).
Append rule: when args are passed but the body has no bare
``\$ARGUMENTS``, append ``ARGUMENTS: …`` at the end.
## Surface
* Parser: ``arguments:`` (list/space-delim) + ``argument-hint:`` (str)
extracted into ``ParsedSkill``.
* Install: persists both to the pre-allocated columns from migration
056 (PR #574). Install path clamps ``argument_hint`` to 128 chars
to match the admin-create cap (untrusted upstream source).
* Admin: ``CreateSkillRequest`` / ``UpdateSkillRequest`` accept both
fields; create + edit modals get inputs; parse-preview echoes.
* Renderer: ``_substitute_skill_args`` runs AFTER ``_render_template``
in ``_load_skills`` so user-supplied args containing ``{{var}}`` can't
be re-expanded by the legacy renderer.
* Session: ``_skill_arguments`` plumbed through ``__init__``,
``set_skill``, and ``_save_config`` so a resumed workstream re-renders
with the original arg payload.
* Model tool: ``skills(action='load')`` accepts an ``arguments`` string.
Approval label includes a SHA-256 digest of the args so a once-
approved skill name can't grant cover for a future payload; preview
surfaces the args inline.
## ``/review`` findings (addressed)
* ``\${CLAUDE_EFFORT}`` referenced ``self._reasoning_effort`` — wrong
attribute; the real one is ``self.reasoning_effort``. Always rendered
empty. Fixed.
* Two-pass layering let user args containing ``{{var}}`` re-expand.
Render order reversed.
* ``_skill_arguments`` wasn't in ``_save_config`` — resumed workstreams
silently lost their payload. Added.
* Approval label omitted ``arguments``. Digest + preview added.
* Install path didn't bound ``argument_hint``. Clamped.
* Added ``_skill_arg_names`` decode tests + "load same skill,
different args → re-render" invariant test.
## Copilot review findings (addressed)
* ``skills.json`` tool description was inaccurate about ``shlex``
stripping quotes and "empty string disables substitution". Rewrote
to match actual behaviour.
* Named-argument regex was stricter than parser/storage contract.
``arguments: [issue-number]`` would partial-match ``\$issue-number``
as ``\$issue``, leaving ``-number`` as stray text. Broadened the
regex to ``[A-Za-z_][A-Za-z0-9_]*`` AND added validation at
``_skill_arg_names`` decode time so names not matching the regex
are dropped with a warning.
## Tests
* ``tests/test_substitute_skill_args.py`` — placeholder forms,
single-pass guarantee, append-at-end rule, shell-quoted input,
unbalanced-quote fallback, uppercase + underscore-prefix names
* ``tests/test_skill_parser.py::TestArgumentsAndHint`` — parser
extraction
* ``tests/test_skill_parse_api.py`` — HTTP parse-preview echoes
both fields
* ``tests/test_skill_discovery_api.py::test_install_seeds_arguments_and_argument_hint``
— install round-trip
* ``tests/test_skills_tool.py::test_load_forwards_arguments_to_set_skill``
+ ``test_load_same_skill_different_args_triggers_resub`` —
wire path through prepare → exec → set_skill
* ``tests/test_skills_tool.py::TestSkillArgNames`` — storage decode
helper including the hyphen/dot/leading-digit filter
|
||
|
Patrick Buckley
|
66400999dd |
fix(skills): address Copilot review on #577
Two findings from Copilot's review of PR #577: * ``_extract_bool`` int branch: Copilot flagged that ``bool(raw)`` treats any non-zero int as True, so ``disable-model-invocation: 2`` silently disables model invocation without warning the author about the typo. Tightened to accept only ``0`` and ``1`` as integer boolean forms — anything else falls back to *default*. Matches spec (which mentions only 0/1) and the broader principle that ambiguous input should not coerce silently. * ``hidden_from_menu`` admin body parse: Copilot flagged that ``bool(body.get("hidden_from_menu", False))`` treats non-empty strings via Python truthiness, so a malformed client sending ``"false"`` would flip the flag to ``True`` — opposite to obvious intent. Extracted a ``_parse_strict_bool`` helper that accepts only Python ``bool`` or int ``0``/``1`` and returns a 400 on anything else. Applied at both admin create and admin update sites; the install path remains untouched because it derives the flag from the typed ``ParsedSkill.user_invocable`` field (not raw HTTP body). ## Tests * ``test_other_ints_fall_back_to_default`` — ``2`` and ``-1`` no longer silently coerce * ``test_create_skill_hidden_from_menu_string_rejected`` — string ``"false"`` returns 400 * ``test_create_skill_hidden_from_menu_int_zero_and_one_accepted`` — 0 / 1 accepted, 2 rejected with 400 No behaviour change to the main surface — both fixes close latent type-coerce hazards a malformed input could have exploited. |
||
|
Patrick Buckley
|
6d28afbe7a |
feat(skills): wire disable-model-invocation / user-invocable (#571)
The Anthropic Claude Code skill spec defines two invocation-control axes Turnstone was parsing but not consuming: * ``disable-model-invocation: true`` — model can't autoload this skill (only user can invoke by name). Stored on ``ParsedSkill`` and echoed on the parse-preview UI; no install consumer because Turnstone hardcodes ``activation="named"`` on source-installs already. The dataclass docstring spells out the no-op so a future reader doesn't try to wire a translation that's already implicit. * ``user-invocable: false`` — skill stays available to the model but disappears from the user-facing picker. Mapped to ``hidden_from_menu=true`` on ``prompt_templates`` (column pre-allocated by PR #574); consumed by ``list_skills_summary`` (both the standalone-server and console-server impls). ## Surface * Parser: new ``_extract_bool`` helper accepts every YAML 1.1 boolean spelling (true/false/yes/no/on/off/1/0) plus their quoted variants — caught by ``/review`` as a real gap, since YAML's ``safe_load`` returns ``int`` for unquoted ``1``/``0`` and ``str`` for the YAML 1.1 spellings when quoted. * Install handler: derives ``hidden_from_menu`` from ``parsed.user_invocable`` on the source-install path. * Admin: ``CreateSkillRequest`` / ``UpdateSkillRequest`` accept ``hidden_from_menu``; both modals get a checkbox; the parse-preview auto-fill flips it when the source SKILL.md sets ``user-invocable: false``. * Runtime config: ``hidden_from_menu`` joined ``SKILL_RUNTIME_CONFIG_FIELDS`` so admin can override on installed (readonly) skills — same precedent as ``model`` / ``effort``. ## list_skills_summary shared helper Two identical implementations of ``list_skills_summary`` had accreted in ``turnstone/server.py`` and ``turnstone/console/server.py``. Both needed the new ``hidden_from_menu`` filter, so extracted the shared body to ``turnstone/core/web_helpers.skill_summary_rows``. Future spec-uplift fields (e.g. #572's ``argument_hint`` for autocomplete) only touch one place now. ## Tests * ``TestInvocationControl`` — bool / quoted / YAML 1.1 / int variants across both fields * ``test_install_user_invocable_false_sets_hidden_from_menu`` + default-unhidden case * ``test_list_skills_summary_excludes_hidden_from_menu`` — picker filter, admin tab unaffected * ``test_update_skill_readonly_hidden_from_menu_allowed`` — admin can hide/unhide installed skills via PUT (pins the runtime-config membership invariant) * Existing parse-API fixture extended with both new fields plus default-case assertions |
||
|
Patrick Buckley
|
4c8f5acd3e |
feat(skills): ingest when_to_use / model / effort from SKILL.md (#570)
The Anthropic Claude Code skill spec defines three frontmatter fields the parser was previously dropping; this PR wires them through to the existing storage shape so the SKILL.md author's intent survives the import. * ``when_to_use`` — concatenated into ``description`` at parse time with a ``\n\nWhen to use: `` separator. Kept as its own field on ``ParsedSkill`` so the admin parse-preview UI can surface it separately. * ``model`` — passed through to ``create_prompt_template(model=...)`` on the source-install path, seeding the existing ``prompt_templates.model`` column. * ``effort`` — same shape, translates to the existing ``reasoning_effort`` column at the install handler boundary. Re-install short-circuits at the source_url dedup, so admin overrides to either column survive an upstream re-install — covered by a new ``test_reinstall_preserves_admin_model_override`` test that pins the load-bearing invariant. ## Description length cap ``_MAX_DESCRIPTION_LEN`` exported as ``MAX_SKILL_DESCRIPTION_LEN`` (public name) and raised from 1024 to 1536 to match the spec's combined ``description`` + ``when_to_use`` listing budget. All five write surfaces now import the same constant rather than each carrying their own magic number: * ``skill_parser.MAX_SKILL_DESCRIPTION_LEN`` — parse-time cap * ``console_schemas.CreateSkillRequest.description`` — Pydantic * ``console_schemas.UpdateSkillRequest.description`` — Pydantic * ``console/server.admin_create_skill`` — handler slice * ``console/server.admin_update_skill`` — handler slice * ``core/session._exec_skills_create`` — coordinator tool slice * ``core/session._exec_skills_update`` — coordinator tool slice The coordinator sites (last two) were the bug ``/review`` caught: they still capped at 1024 after the rest of the surface bumped to 1536, so a model-issued ``skills(action='create')`` with a 1025-1536 char description would silently truncate. Sharing the constant closes that desync. ## when_to_use truncation guard The ``when_to_use`` concat reserves room for the separator + at least one character of the appended value; below that budget, the addition is dropped entirely. Previously the naive concat could truncate mid-separator and leave the description ending in a dangling ``\n\nWhen ``. ## Tests * ``TestWhenToUse`` — concat semantics, no-description fallback, 1536 truncation * ``TestModelAndEffort`` — extraction + defaults * ``test_install_seeds_model_and_effort_from_frontmatter`` — install path persists both columns * ``test_install_no_model_or_effort_leaves_columns_empty`` — bare SKILL.md doesn't invent values * ``test_reinstall_preserves_admin_model_override`` — admin edits survive an upstream re-install (dedup invariant) * ``test_parses_full_frontmatter`` / ``test_parses_minimal_frontmatter`` extended with the new field assertions |
||
|
Patrick Buckley
|
93af031fc7 |
feat(skills): parse and store Anthropic spec paths + uplift columns
Implements PR1 of issue #569 — parser + storage + admin UI for the Anthropic Claude Code skill spec `paths:` SKILL.md frontmatter field (glob patterns gating model-initiated autoload). The autoload filter that consumes `paths` is deferred to a follow-up PR pending the workstream-CWD design discussion. Migration 056 bundles three additional columns whose consumer PRs are filed but not yet implemented: * `hidden_from_menu` (boolean) — backs the spec's `user-invocable: false` (issue #571). * `arguments` (JSON list) — backs spec `arguments:` named arg slots (issue #572). * `argument_hint` (string) — autocomplete display string (issue #572). The deferred columns surface in `SkillInfo` (response) so consumers can read them, but are deliberately absent from `CreateSkillRequest` and `UpdateSkillRequest` — the create/update handlers don't yet read them and advertising a writable field the handler would silently ignore would be an OpenAPI lie. Surface - Parser: `ParsedSkill.paths` populated from frontmatter; accepts the spec's YAML-list-or-CSV-string shape via the existing `_extract_list` machinery. - Storage: 4 new columns on `prompt_templates`; `SKILL_MUTABLE` extended; `_row_to_dict` calls extended to cast the new bool; protocol + SQLite + PostgreSQL `create_prompt_template` signatures threaded. - HTTP: admin create/update/install/parse handlers plumb `paths` through. Pydantic schemas extended accordingly. - Admin UI: `skill-paths` and `etm-paths` inputs on the create + edit modals; field map and read/write helpers wired across paste-parse, reset, create-send, edit-load, edit-send, and the readonly-disable list. Notable - `_canonicalize_skill_string_list` collapses the list-or-CSV-or-JSON- string normalization shared between admin_create_skill and admin_update_skill. Treats `None` as no-value so a body containing `{"paths": null}` doesn't CSV-split through `str(None)` and store the literal `["None"]`. Will back `arguments` once #572 wires its consumer. Tests - Parser: TestPaths covers YAML list, CSV string, empty, full- frontmatter integration (tests/test_skill_parser.py). - Storage: round-trip suite covers create + read + update for each of the four new columns on both backends (tests/test_storage_skill_spec_uplift.py). - Helper: focused unit tests for the canonicalizer including the regression-net case for the null-corruption bug (tests/test_canonicalize_skill_string_list.py). - HTTP boundary: extended test_parses_full_frontmatter + test_parses_minimal_frontmatter to assert `paths` survives the admin parse endpoint. |
||
|
Patrick Buckley
|
c76a61841e |
fix: PR #118 round 2 — null-safe parser, docs, consistency
- Null-safe extraction for description, license, and compatibility in skill_parser.py — YAML bare keys (e.g. `description:`) no longer produce the literal string "None" - Log warning on skill catalog storage failure instead of silent swallow - Use `enabled == 1` in list_skills_by_activation for consistency with other prompt_templates queries in both storage backends - Add parser tests for YAML null description, license, and compatibility - Update governance.md: document runtime config editing on installed skills, two-column modal layout, SPDX license dropdown, origin badge |
||
|
Patrick Buckley
|
dc464ac313 |
feat: Agent Skills standard compliance + frontend spec fields
Brings skills implementation into full compliance with agentskills.io: Parser: - Read `allowed-tools` (hyphenated, standard) only; stored as `allowed_tools` internally — no underscore fallback - Reject consecutive hyphens in skill names - Extract author/version from standard `metadata:` map with top-level fallback; null-safe (no "None" string for bare YAML keys) - Truncate description at 1024 chars, compatibility at 500 chars (spec caps) with log warnings - Lenient parsing mode (lenient=True) for cross-client import: sanitizes names, returns None on skip, malformed-YAML colon-value retry - Type overloads: strict mode returns ParsedSkill, lenient returns ParsedSkill | None Session: - `<available-skills>` XML catalog in system messages for activation="search" skills (disabled ones filtered out, capped at 30) Tool rename: - `load_skill` tool → `skill` (JSON, session preparers/executors, approval labels, tests, docs) Storage (migration 023): - Add `license` and `compatibility` columns to prompt_templates - skill_license / compatibility params on create_prompt_template across protocol, SQLite, PostgreSQL backends - Add to SKILL_MUTABLE for update_prompt_template API + server: - SkillInfo, CreateSkillRequest, UpdateSkillRequest: license + compatibility fields - Create/update/install endpoints extract and persist both fields - Install endpoint maps parsed.license + parsed.compatibility from imported SKILL.md (previously discarded) - _skill_to_response() includes both fields Admin UI: - Create + edit modals: version, license, compatibility fields - Readonly (imported) skills: "edit" → "view" button, modal title "View Skill", all fields disabled, Save hidden, Cancel → "Close", collapsibles auto-expand, focus on Close button - :disabled CSS for dark-theme modal inputs (bg-highlight, cursor not-allowed, dimmed text) - Fix addEventListener stacking on auto-approve checkboxes → .onchange SDK: license + compatibility on SkillInfo, CreateSkillRequest, UpdateSkillRequest TypeScript interfaces Docs: governance.md, judge.md, tools.md, README, diagram updated |
||
|
Patrick Buckley
|
c28bfc1e58 |
feat: skill discovery — search and install skills from external sources (#111)
* feat: skill discovery — search and install skills from external sources Add discovery UI and API for finding and installing skills from skills.sh registries and GitHub repositories with one-click install, SKILL.md frontmatter parsing, and security scan integration. Core modules: - skill_parser.py: ParsedSkill dataclass, parse_skill_md() with YAML frontmatter support (Anthropic + Hermes tag formats), name validation - skill_sources.py: SkillsShClient (async search + resolve), fetch_skill_from_github (SKILL.md + bundled resource fetching with 256KB cap, text extension filter, GitHub API tree traversal) API: - GET /v1/api/admin/skills/discover — search with installed annotation and scan_status for installed skills - POST /v1/api/admin/skills/install — fetch, parse, duplicate check, create with origin="source" readonly=true, store resources, audit Also fixes pre-existing bug where _skill_to_response omitted scan_status, scan_report, scan_version fields — scan tier badges in the installed skills table were silently empty despite data existing in storage. Admin UI: pill toggle (Installed/Discover), discovery cards with scan tier badges, GitHub import modal with proper focus trap/Escape/backdrop, scoped selectors preventing MCP↔Skills cross-tab state corruption. SDK: discover_skills() + install_skill() on Python (async+sync) and TypeScript console clients. 48 new tests across 3 test files. All 2632 tests pass. * fix: address copilot review — 404 vs 502, O(n) lookups, branch fallback - SkillNotFoundError subclass: install returns 404 when SKILL.md is missing, 502 only for connectivity/upstream errors - get_skill_by_source_url() + list_installed_skill_urls(): indexed storage lookups replace O(n) full-table scans with content blobs - Default branch fallback: tries main then master when URL doesn't specify a branch - Path normalization: strip trailing slash once, remove redundant candidate - SDK install_skill() returns typed SkillInfo with response_model - Tree size guard: skip resource tree if response >2MB |