Skip to content

fix(skills): a deprecated library skill says so — listing, assign, Library and Skills tab (abilityai/trinity-enterprise#672) - #3133

Merged
vybe merged 4 commits into
devfrom
feature/672-skill-deprecation-lifecycle
Oct 1, 2026
Merged

vybe merged 4 commits into
devfrom
feature/672-skill-deprecation-lifecycle

Conversation

@webmixgamer

@webmixgamer webmixgamer commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A skills library retires a skill with two frontmatter keys, deprecated: true and superseded-by: <text>. The platform parser dropped both, so a retired skill was listed, assignable and injected exactly like a live one until its directory was deleted. Four skills on the community library's main carry the keys today (two of them are already in the v0.2.0 tag that fresh installs pin), and none of them was marked anywhere.

  • The keys survive. extract_contract carries deprecated (a real boolean) and superseded_by.
    • superseded_by is the author's text: one printable line, at most 200 characters, null unless the skill is deprecated. It is not a resolved skill name, because two of the four live values are sentences.
    • Both spellings are read, and a trinity: block wins.
    • Garbage gets a named frontmatter_invalid:* warning. A library whose CI runs this parser fails on it.
  • Every listing carries them. list_skills / get_skill, GET /api/skills/library (named in the route's explicit SkillInfo(...)) and MCP list_skills (named in its field map).
  • Assigning one still succeeds, and says so. delivery.skills[name].warnings carries deprecated or deprecated:<name> on every delivery outcome, a stopped agent included.
    • The value is read from the library entry, because the delivery report reduces each skill to a status and two of its outcomes never run an injection.
    • The key is present only for a deprecated name and carries lifecycle codes only.
  • Injection is unchanged. A deprecated skill is still injected. The same code rides each injection result and the start endpoint's public projection. The agent's CLAUDE.md is deliberately not annotated, so an agent that already holds one behaves exactly as before.
  • UI. The Library card, the Skills-tab assigned rows and the Skills-tab picker show a warning deprecated badge and a "Superseded by …" line, and an amber note appears after assigning one. Nothing is hidden, dimmed or refused.

No migration: both fields are derived from the synced repository on every parse and never persisted.

Changes

  • Parser: src/backend/services/skill_packaging.py — extract_contract + _first_present / _one_line, and deprecation_warning() (the one producer of the code).
  • Service: src/backend/services/skill_service.py — _parse_skill_info copies the fields, _inject_skills_locked appends the code, and deliver_assigned runs the library read beside delivery and stamps the report.
  • REST: src/backend/routers/skills.py + src/backend/db_models.py::SkillInfo — two fields.
  • Start projection: src/backend/services/agent_service/lifecycle.py — _WARNING_RE keeps deprecated:<name>.
  • MCP: src/mcp-server/src/tools/skills.ts — list_skills maps both fields; descriptions updated.
  • Frontend: LibrarySkillsSection.vue, SkillsPanel.vue and skills/AssignedAgents.vue, with shared helpers in skills/contract.js (supersededLine, DEPRECATED_TITLE) and utils/skillDelivery.js (deprecationText, isDeprecationCode).
  • Docs:
    • requirements §21.4 / §21.5 / §21.6 / §22.2 / §22.3 (written before the code);
    • four feature flows;
    • architecture/backend.md and architecture/mcp-server.md;
    • two learnings fragments;
    • the CSO diff report.

Test plan

  • tests/unit/test_ent672_skill_deprecation.py — 49 tests. Covers the parse table, the warning grammar, the fields through the listing / get / REST route, the code on injected / unchanged / conflict results, the delivery stamp on running / stopped / docker-unreadable / over-budget outcomes, the library read running beside delivery, fail-open on an unreadable library, the start projection, and the un-annotated CLAUDE.md line.
  • src/frontend/tests/unit/skillDeprecation.spec.js — 18 tests, mounted. Covers each of the three surfaces with its own test ids, both post-assign notes and their lifetimes, a live skill unmarked, and an HTML-shaped successor rendered as text.
  • src/mcp-server/src/tools/skills.test.ts — 3 new tests, including a backend that predates the fields.
  • Mutation check: 39 wiring lines mutated one at a time — both router kwargs, both MCP map keys, the parse copy and its defaults, the inject append, the delivery stamp and its fail-open arm, the concurrent read, the projection regex, every parser rule, all six template sites, the badge titles, both notes and their lifetimes. 39 red, each file restored byte-identical.
  • Existing suites, after merging the latest dev:
    • skill, delivery, conflict, start-result and auth-guard suites: 786 passed;
    • MCP: 620 passed, tsc clean;
    • frontend: 3971 passed, with the two known host-only @vue/compiler-sfc load failures;
    • raw-colour, loading-gate and source-text ratchets and check:tokens pass.
  • /verify-local --skip-agent --skip-unit passes on the merged commit: backend image build + import main, isolated boot, integration 70 passed. The unit stage was skipped because it fails on every branch off current dev for order-dependent reasons, and it runs in CI.
  • /cso --diff: no findings at the 8/10 gate — docs/security-reports/cso-diff-2026-09-30-672-skill-deprecation.md.
  • Checked by hand on localhost against the community library's main (four deprecated skills): the Library cards, assigning from a card, the Skills tab picker and assigned row, and the API fields.

Not in this PR

Fixes abilityai/trinity-enterprise#672

🤖 Generated with Claude Code

webmixgamer and others added 3 commits September 30, 2026 15:15
…brary and Skills tab (Abilityai/trinity-enterprise#672)

A skills library retires a skill with two frontmatter keys, `deprecated: true`
and `superseded-by: <text>`. The platform parser dropped both, so a retired
skill was listed, assignable and injected exactly like a live one until its
directory was deleted.

Parser (`skill_packaging.extract_contract`)
- carries `deprecated` (a real boolean) and `superseded_by` (the hyphenated key;
  the underscore spelling is read too; a `trinity:` block wins in either
  spelling; a null key masks nothing)
- `superseded_by` is the author's text — one printable line, at most 200
  characters, None unless the skill is deprecated — not a resolved skill name
- garbage is named, never fatal: `frontmatter_invalid:deprecated`,
  `frontmatter_invalid:superseded-by`
- `deprecation_warning()` is the one producer of the machine code:
  `deprecated:<successor>` when the text is a valid skill name, bare
  `deprecated` otherwise

Surfaces
- `list_skills` / `get_skill`, `GET /api/skills/library` (named in the route's
  explicit `SkillInfo(...)`) and MCP `list_skills` (named in its field map)
  carry both fields
- a deprecated skill is still injected; the code rides every per-skill result
  (injected, unchanged, conflict) and the start endpoint's public projection
- assigning one still succeeds: `delivery.skills[name].warnings` carries the
  code on EVERY delivery outcome. It is read from the library entry, beside
  delivery, because the delivery report reduces results to a status and a
  stopped agent never runs an injection. Present only for a deprecated name;
  lifecycle codes only
- Library card, Skills tab assigned row and Skills tab picker: a warning
  `deprecated` badge beside the name and a "Superseded by …" line; a
  warning-toned note after assigning one. Nothing is hidden, dimmed or refused
- the agent's CLAUDE.md is deliberately not annotated: an agent that already
  holds a deprecated skill behaves exactly as before

Not marked yet: skill-set member lists, the skill-runner picker, and the CLI's
`skills list` table (its JSON output carries the fields).

No migration: both fields are derived from the synced repository on every
parse and never persisted.

Tests: tests/unit/test_ent672_skill_deprecation.py (49),
src/frontend/tests/unit/skillDeprecation.spec.js (17, mounted),
src/mcp-server/src/tools/skills.test.ts (+3). Mutation check: 39 wiring lines
mutated one at a time (both router kwargs, both MCP map keys, the parse copy
and its defaults, the inject append, the delivery stamp and its fail-open
arm, the concurrent read, the projection regex, every parser rule, all six
template sites, the badge titles, both notes and their lifetimes) — 39 red,
each file restored byte-identical.

Refs Abilityai/trinity-enterprise#672

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rt (Abilityai/trinity-enterprise#672)

/cso --diff on the branch: no findings at the 8/10 gate; four candidates
recorded below the gate with their evidence. The one pin the audit added: the
Library card renders a library-authored successor containing markup as
characters — red when the successor line is mutated to v-html.

Refs Abilityai/trinity-enterprise#672

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ad is waited for (Abilityai/trinity-enterprise#672)

Review finding on PR #3133: the docstring claimed the read is "done long
before any delivery outcome". That holds for the budget-bound in_progress
path; a fast outcome (pending_start, docker_unavailable) waits for it — a
cached listing read. Comment only.

Refs Abilityai/trinity-enterprise#672

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe

vybe commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

merge-train (#3143): on this train. /validate-pr and /review came back READY with no criticals. The deprecation value is traced end to end, and breaking it on purpose fails tests. #3134 and #3135 are both bugs already on dev, not ones this PR introduces.

Non-blocking:

  • deliver_assigned waits on the library read with a bare await notes_task (skill_service.py:1993-1998). A hung read would hang the assign response. Wrapping it in asyncio.wait_for with a {} fallback would match the existing fail-open path.
  • skills_warnings (lifecycle.py:253) now counts the informational deprecated code. Nothing reads that count today.
  • The PR flags a deprecated skill but doesn't dim it, which is looser than the issue's "de-emphasizes". That's a product call.

@vybe vybe 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.

merge-train: batch validated on train/#3143 (all Tier 1 + Tier 2 green)

@vybe
vybe merged commit 338f5d3 into dev Oct 1, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants