Skip to content

fix(skills): a library skill marked user-invocable: false is reported that way (#3134) - #3422

Merged
vybe merged 2 commits into
devfrom
feature/3134-user-invocable-hyphen
Oct 9, 2026
Merged

vybe merged 2 commits into
devfrom
feature/3134-user-invocable-hyphen

Conversation

@vybe

@vybe vybe commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #3134 — independent fix, one of ten small bug fixes in the trinity-pm chain-easy-1008 run. Base is dev. Draft until the operator merges.

What

  • src/backend/services/skill_packaging.py::extract_contract: user_invocable is read with the existing _first_present(sources, ("user-invocable", "user_invocable")). A library skill marked user-invocable: false (the Claude Code frontmatter spelling, the only one the agent server reads) is now reported user_invocable: false on GET /api/skills/library and MCP list_skills. The wire name stays user_invocable; the underscore spelling keeps working.
  • Precedence: a trinity: block wins over both flat spellings, whichever spelling it uses; within one level the hyphen wins; a present-but-null key does not hide the other spelling; a non-bool still reads as True.
  • Docs: docs/memory/feature-flows/skill-injection.md, docs/memory/requirements/skills.md, and docs/user-docs/automation/skills-and-playbooks.md (the user doc taught the underscore spelling, which the agent server never reads).
  • Tests: eight cases in tests/unit/test_ent183_skill_packages.py.

Rulings carried (orchestrator, on the operator's behalf — plan file)

  • TD-1: _first_present over the sources, not the issue's suggested one-liner — as recommended. The one-liner reads the merged dict, so a flat hyphen key would beat a trinity: block that uses the underscore, breaking the acceptance criterion "a trinity: block still wins". The sweep confirmed it: with the one-liner in place, two of the eight cases fail.
  • TD-2: real booleans only — as recommended.
  • TD-3: the same cross-spelling flaw in allowed-tools stays out — as recommended; see Handoffs.

Review + security

/review reading pass (claude-fable-5-1, report-only): MERGEABLE, zero critical. Every reader of the platform-side flag is display or catalog only (Library → Skills chip, the agent Skills tab chip, MCP list_skills). Everything that gates execution, connector exposure or portal hints reads the agent server's own parser, which this branch does not touch; assignment never consults the flag. /cso --diff: no findings.

Correction to the issue (review I1): on the library's current tip only project-intake carries user-invocable: false. adjust-playbook is explicitly true and skill-builder has no key. One chip flips, not three.

Fixed after review: d44b207a — I2, the user-doc line.

Evidence sweep after the fix (claude-opus-5-5): all 10 unit files that import skill_packaging ran shuffled (seed 12345), one file per process — 387 passed, 1 failed. The failure is test_ent236_skills_lifecycle.py::TestNonRepoDirectoryRecovers::test_real_repo_still_pulls ('cloned' == 'pulled'). It fails the same way on dev f6bedcd on the engineer, without shuffling, and it tests skill_source_clone, which this branch does not change; it looks environment-dependent (real git against the test's empty fake .git). The builder marked the sweep RED on principle; nothing this branch touches is red. With dev's parser, four of the eight spelling cases fail. git merge-tree against dev is clean.

Tests

Counts above. Not run here: the full unit island (CI).

Before merge

Handoffs (not in this diff)

  • allowed-tools (skill_packaging.py, around line 208) has the same cross-spelling flaw.
  • abilityai/trinity-skills tools/validate.py runs a copy of this parser pinned by SHA. It asserts only on frontmatter_invalid* warnings, so nothing it expects changes; the library picks the new behaviour up when that pin moves.

🤖 Generated with Claude Code

Trinity Agent (trinity) and others added 2 commits October 8, 2026 19:59
The platform contract parser read only `user_invocable`, while Claude Code,
the skill libraries and the agent server use `user-invocable`. A library
skill marked `user-invocable: false` was therefore reported invocable by
GET /api/skills/library and MCP list_skills.

Read both spellings per source through the existing `_first_present`
helper (hyphen first), so a `trinity:` block still wins whichever spelling
either level uses and a null value masks nothing. Non-bool values still
read as invocable. Wire name stays `user_invocable`.

Mutation check (scratch copy): the issue's literal merged-scope form turns
[trinity-underscore-beats-flat-hyphen] and [null-hyphen-does-not-mask] red;
the underscore-only original turns [hyphen],
[trinity-hyphen-beats-flat-underscore], [flat-hyphen-beats-underscore] and
[hyphen-wins-in-trinity] red.

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

@trinity-ability trinity-ability 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/20261009-0735 (#3424)

@vybe
vybe merged commit 8cd434f into dev Oct 9, 2026
30 of 31 checks passed
webmixgamer added a commit that referenced this pull request Oct 9, 2026
Brings the branch up to ed59049 (12 commits, incl. #3406 public-link
sessions, #3422 `user-invocable:` in the library, #3418 safe_yaml).

One conflict, in docs/user-docs/automation/skills-and-playbooks.md: this
PR's prose kept, with the `user-invocable:` key spelling (the hyphen is
the only spelling the agent server reads). The Skills flow's Run table
now names the listing field and the frontmatter key apart.

PublicChat.vue merged cleanly with #3406 (this PR's "/ for skills"
placeholder kept).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants