feat(skills): support disable-model-invocation skill frontmatter - #76464
feat(skills): support disable-model-invocation skill frontmatter#76464charlesvien wants to merge 6 commits into
Conversation
Parse the disable-model-invocation frontmatter field into SkillInfo, preserve it through manifest saves (previously editing a skill silently stripped it), emit it from serializeSkillMarkdown, and surface it in the skills UI as a badge plus an edit toggle. Generated-By: PostHog Code Task-Id: b9824961-d68a-4ac2-b272-7053ecc0e497
…invocation A value like `disable-model-invocation: true # manual only` misparsed to false because extractYamlValue keeps trailing comments, and a later manifest save would then silently drop the field from disk. Strip an unquoted trailing comment before the boolean comparison and cover the commented forms with tests. Generated-By: PostHog Code Task-Id: b9824961-d68a-4ac2-b272-7053ecc0e497
Comment stripping in parseYamlBoolean ran on the already-unquoted value, so a quoted non-boolean like "true # manual only" was mangled into boolean true. Move comment handling into extractYamlValue's plain-scalar path (quoted content is literal in YAML and never contains a comment) and add a regression test. Generated-By: PostHog Code Task-Id: b9824961-d68a-4ac2-b272-7053ecc0e497
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Reviews (1): Last reviewed commit: "fix(skills): strip trailing YAML comment..." | Re-trigger Greptile |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
@haacked this is your PostHog/code#3776 remade in the monorepo (main there is frozen after the desktop import). Your commits are intact, only paths moved. Can you review and approve if it looks good? |
There was a problem hiding this comment.
Touches a security-sensitive surface (skill "manual invocation only" flag), but the author is on the owning team with STRONG familiarity, and the diff itself carries the flag through the export schema, team-skills API metadata, install path, and web bundler — closing exactly the gap veria-ai's inline comment flagged, which is now marked resolved and confirmed by veria-ai's follow-up review of "no open security concerns, risk 0/10", plus positive reactions from greptile and a security bot. Extensive new tests cover the round-trip.
- Author wrote 100% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
- PR description says team skill publish/install 'doesn't survive that round trip yet', but the diff actually implements it via a metadata key — the description text appears stale relative to the final commits; worth a quick description update but doesn't indicate hidden risk since the diff closes the gap rather than opening one.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 154L, 12F substantive, 397L/16F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (397L, 16F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ d9e4168 · reviewed head 10a64dc |
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Mechanical port of a security feature (manual-invocation-only skills) by an owning-team author with STRONG familiarity; the diff confirms the previously flagged gap (flag not surviving team-skill publish/install) is actually fixed with tests, and the security bot's review reflects no remaining concerns.
- Author wrote 100% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 154L, 12F substantive, 397L/16F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (397L, 16F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ d9e4168 · reviewed head d2dfb7c |
Ports PostHog/code#3776 into the monorepo (PostHog/code
mainis frozen after the desktop import; open PRs there are remade here). Commits preserve the original authorship.Problem
Claude Code skills support
disable-model-invocation: truein SKILL.md frontmatter: the agent never invokes the skill on its own — only an explicit user invocation (slash command or composer skill tag) runs it. The bundled agent CLI already honors the field at runtime (both locally and in cloud sandboxes, since skill bundles ship SKILL.md verbatim), but PostHog Desktop was blind to it: the field wasn't parsed, wasn't shown anywhere in the skills UI, and — worse — editing a skill in the manifest editor silently stripped it from the file.Changes
parseSkillFrontmatternow extractsdisable-model-invocation, exposed as an optionaldisableModelInvocationonSkillInfo.serializeSkillMarkdown(the shared SKILL.md serialization contract) emits the field, so manifest saves round-trip it instead of dropping it.Not covered: team skill publish/install — the team-skills API only stores name/description/body, so the flag doesn't survive that round trip yet.
Related (different feature, no overlap): PostHog/code#3735 adds always-on skills.
How did you test this code?
pnpm --filter @posthog/workspace-server exec vitest run src/services/skills: 111 passed).pnpm --filter @posthog/ui exec vitest run src/features/skillspassed.pnpm typecheckfor shared, workspace-server, host-router, ui, and web; Biome check on changed files.products/desktop/,pnpm install --frozen-lockfile,pnpm typecheck(24/24 clean), workspace-server skills tests (124), UI skills tests and the full@posthog/sharedsuite (780) all pass.Automatic notifications
Docs update
N/A
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
This PR is a mechanical port of PostHog/code#3776 (author: haacked) into
products/desktop/, done with Claude Code following the repo's /porting-code-prs skill:git am -3 --directory=products/desktop/over the source patch series, preserving the original commits and authorship. It applied without conflicts; no source changes beyond the path relocation.Created with PostHog Code