mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-26 13:54:48 -06:00
9309162ac0
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