Skip to content

feat(repo): add public agent skills for thymian sampler check and test - #313

Draft
Dariant31 wants to merge 7 commits into
mainfrom
dvs/334_reuseable_thymian_skill
Draft

feat(repo): add public agent skills for thymian sampler check and test#313
Dariant31 wants to merge 7 commits into
mainfrom
dvs/334_reuseable_thymian_skill

Conversation

@Dariant31

Copy link
Copy Markdown
Collaborator

What

Adds a top-level skills/ directory exposing two public, reusable Agent Skills so external projects can install them into their own repos with the skills CLI:

  • thymian-sampler-check — diagnose and fix failing thymian sampler check transactions (samples, hooks, the runHooks trap, decision guide).
  • thymian-test — triage and resolve thymian test conformance findings (the three resolution paths, spec-change/sampler-rehash coupling).

Plus a skills/README.md with install instructions and layout.

Why

The skills were authored and battle-tested on a consumer project (see thymianofficial/thymian-internal#344 / closes #334) and are written generically — "for any API", no repo-specific knowledge. Per review feedback, they belong at the thymian level so any Thymian user can pull them in:

npx skills add https://github.com/thymianofficial/thymian/tree/main/skills

Notes

  • Skill content is identical to the validated versions (one prettier trailing-comma reformat in a code example from this repo's pre-commit hook).
  • The consumer-specific overlay skill (thymian-on-restlos) intentionally stays in the consumer repo — only the generic siblings are published here.
  • The install URLs reference main, so they resolve once this PR merges.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings July 7, 2026 07:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new top-level skills/ directory that publishes two reusable, public Agent Skills intended for external repositories to install via the skills CLI, with accompanying installation/docs guidance.

Changes:

  • Added thymian-sampler-check skill documenting how to diagnose/fix thymian sampler check failures via samples/hooks.
  • Added thymian-test skill documenting how to triage/resolve thymian test conformance findings and the spec↔sampler coupling.
  • Added skills/README.md describing available skills, install commands, and directory layout.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
skills/thymian-test/SKILL.md New public skill for resolving thymian test conformance findings and workflows.
skills/thymian-sampler-check/SKILL.md New public skill for diagnosing thymian sampler check failures and hook/sample mechanics.
skills/README.md Public entrypoint README with install instructions and skill index.

Comment thread skills/thymian-sampler-check/SKILL.md Outdated
Comment thread skills/thymian-sampler-check/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Comment thread skills/README.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
@Markus-Ende
Markus-Ende requested review from atennert and matthyk July 7, 2026 07:34
@matthyk

matthyk commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@Dariant31 could you have a look at the copilot comments? I have not tried yet, but you could also use a skill optimizer skill like this: https://github.com/mcollina/skills/tree/main/skills/skill-optimizer

Dariant31 and others added 2 commits July 16, 2026 12:21
Expose the thymian-sampler-check and thymian-test agent skills in a
top-level skills/ directory so external projects can install them with
the skills CLI (npx skills add). Skill content is identical to the
versions validated on a consumer repo; adds a README with install
instructions and layout.

Closes #334

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Correct the utils.request documentation in thymian-sampler-check: options
({ runHooks, authorize, forStatusCode }) are the 3rd parameter, and
authorize: false does not skip the authorize hook — only runHooks: false
does; the login example now shows the real 3-arg call shape.

Align thymian-test and the README with the actual reporting model: the
Summary line counts errors/warnings/hints/info, configurable rule
severities are error | warn | hint | off, and info is a finding-only
severity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 05:22
@Dariant31
Dariant31 force-pushed the dvs/334_reuseable_thymian_skill branch from 8d12e4a to c491aad Compare July 16, 2026 05:22
@Dariant31

Dariant31 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

@matthyk All 5 Copilot comments are addressed in c491aad — each claim was verified against the source before fixing:

  • utils.request / authorize docs (2 comments): confirmed in create-hook-utils.ts that the authorize hook runs whenever runHooks is true (the default), so { authorize: false } never disables it. The utils table now documents the real options shape ({ runHooks?, authorize?, forStatusCode? }, 3rd parameter), the login example uses the actual 3-arg call with { runHooks: false }, and the "runHooks trap" section spells out that { runHooks: false, authorize: true } is how you keep only the authorize hook.
  • Severity model / summary line (3 comments): frontmatter, README, and the result-model section now use the real Summary: X error(s), Y warning(s), Z hint(s), W info finding(s) output; the severity docs cover error | warn | hint | off as configurable values with info as finding-only (per rule-severity.ts / rule-violation.ts).

Branch is rebased on main.

Update: the skill-optimizer pass you suggested is applied in b839001. Its static rules (activation design, context budget, regression triage) produced: an imperative Non-negotiables checklist front-loaded at the top of each skill (the fragile behaviors — meta.json, runHooks, spec-rehash — were previously buried mid-file), removal of gotcha bullets duplicating the same lessons (net context cost stays flat: +16/−5 lines), and a wording scan that found no soft phrasing around mandatory behavior. The optimizer's benchmark loop / release gates need a with-vs-without model eval harness, which doesn't exist here — that part is genuinely follow-up material.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread skills/thymian-sampler-check/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Apply the static rules from mcollina's skill-optimizer (activation
design, context budget, regression triage): front-load the fragile
high-value behaviors of each skill into an imperative Non-negotiables
checklist at the top, and remove gotcha bullets that duplicated the
same lessons, keeping the net context cost flat. Trigger lists,
decision tables, and integrated examples were audited and kept; no
soft phrasing around mandatory behavior remained.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 05:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

Comment thread skills/thymian-test/SKILL.md
Comment thread skills/thymian-test/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Comment thread skills/thymian-sampler-check/SKILL.md Outdated
Comment thread skills/thymian-test/SKILL.md Outdated
Address the second Copilot review round, verified against source:

- Summary line matches cli-report-renderer.ts ("X error(s), Y
  warning(s), Z hint(s), W info(s)." — no "finding(s)"/"across N
  run(s)"), which changed on main since the skills were written.
- ruleSeverity documented as the minimum severity threshold deciding
  which rules run (default: error), distinct from per-rule overrides.
- Sampler abort quoted with the real VersionMismatchError wording, and
  the spec hash described as the single tree-wide version in the
  samples-root meta.json instead of per-sample hashes.
- Sample directory layout no longer implies media-type folders are
  bracketed; brackets are literal for path-parameter folders only.
- Removed the "exactly one path" wording that contradicted the
  Path 1 + Path 2 guidance for added headers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 05:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread skills/thymian-sampler-check/SKILL.md
Comment thread skills/thymian-test/SKILL.md
Address the third Copilot review round plus a full self-audit of every
factual claim against the source:

- Quote the exact check failure wording ("Expected status code X, but
  received Y") in the thymian-test frontmatter and README.
- Non-negotiable 2 in thymian-sampler-check now carves out genuine
  contract mismatches, which are fixed in the spec or API, matching the
  decision guide.
- Use exact registered rule ids (thymian/response-body-must-conforms-
  to-schema, thymian/response-headers-must-conform-to-schema,
  rfc9110/origin-server-should-send-etag, rfc9110/origin-server-should-
  send-last-modified) and drop references to rules that do not exist on
  main (must-send-www-authenticate, location-header-for-201,
  304-or-412-when-if-none-match-fails).
- sampler init documented without the nonexistent --spec flag; config
  described via its real keys (specifications, targetUrl).
- utils.info gotcha reworded to be reporter-agnostic.

Verified unchanged claims: hook filename patterns and extensions
(constants.ts), last-authorize-only (hook-runner.ts), overwrite keeps
hooks (write-samples-to-dir.ts), types.d.ts location, Config not
found / Unknown type / Reason: output strings, --incremental and
--for-transaction flags.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 06:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings July 16, 2026 11:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copilot AI review requested due to automatic review settings July 20, 2026 07:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

skills/thymian-test/SKILL.md:123

  • This section frames the samples-root meta.json version as a hash of the spec only, but the implementation stores format.toHash() (a hash of the loaded format graph). That hash can also change across Thymian upgrades / format-loading changes even when the OpenAPI input file is unchanged, so limiting the cause to “spec changed” can mislead users triaging VersionMismatchError after a tool upgrade.
## The coupling that surprises people: spec changes re-hash the sampler

If the sampler plugin is configured, `thymian test` and `thymian sampler check` share the recorded transactions, and the samples tree stores the **hash of the spec it was generated from** (the `version` in the samples-root `meta.json` — one hash for the whole tree). **Editing the spec (or regenerating it) changes the spec's hash — both `test` and `check` then abort** with a `VersionMismatchError` ("The loaded samples were generated at … Did you forget to regenerate the samples?"). After any spec change you must:

skills/thymian-test/SKILL.md:162

  • The VersionMismatchError troubleshooting entry attributes the mismatch solely to “the spec changed”, but the stored version is format.toHash() (loaded format hash). Users can hit this after a Thymian upgrade or any change in format loading even if the spec file didn’t change, so the cause description should be broadened.
**Error: `VersionMismatchError` — "The loaded samples were generated at … Did you forget to regenerate the samples?" (test aborts)**
Cause: the spec changed, so its hash no longer matches the `version` stored in the samples-root `meta.json`. Solution: `npx thymian sampler init --overwrite`, re-apply hand-edited bodies, delete orphan sample dirs, then re-run.

Comment on lines +248 to +249
**Error: `VersionMismatchError` — "The loaded samples were generated at … Did you forget to regenerate the samples?" (check aborts)**
Cause: the spec changed, so its hash no longer matches the `version` stored in the samples-root `meta.json` (one hash for the whole tree). Solution: `npx thymian sampler init --overwrite`, then re-apply any hand-edited request bodies and delete sample dirs for responses no longer in the spec.
@matthyk
matthyk marked this pull request as draft July 27, 2026 06:33
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.

3 participants