AX-1848 Moving from session start hook to skills (skills sync in from jfrog-skills)#33
Open
MatanEden1 wants to merge 9 commits into
Open
AX-1848 Moving from session start hook to skills (skills sync in from jfrog-skills)#33MatanEden1 wants to merge 9 commits into
MatanEden1 wants to merge 9 commits into
Conversation
Replace the always-on SessionStart hook (which injected the full jfrog-mcp-management template into every session) with three discoverable skills: - jfrog-mcp-install — add an MCP via the JFrog Agent Guard - jfrog-mcp-list — list catalog + currently configured MCPs - jfrog-mcp-remove — remove an installed MCP Shared pre-flight, key-rules/troubleshooting, and runtime-permission references live in jfrog-mcp-shared/. A silent Step 0 gate (scripts/agent-guard-check.mjs) keeps the skills inert unless the JFrog Agent Guard is enabled, preserving the hook's gating behavior. Retire the hook machinery: hooks/hooks.json, scripts/inject-instructions.mjs, scripts/validate-hook-injector.mjs, templates/jfrog-mcp-management.md, and the validate-inject-instructions.yml CI workflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… @jfrog/agent-guard
Replace the shell-enumeration table with syntax-family detection (POSIX export vs. fish vs. Windows setx) so uncommon POSIX shells (ksh, dash, ash) are still handled correctly instead of falling through unhandled. Drop a startup-file heuristic that produced noisy false positives in cluttered home directories, and a confirmation command that didn't actually verify anything; replace with canonical per-family defaults plus a quick existence check before writing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Drop stale "hook" wording now that MCP management lives in skills, not the retired SessionStart hook. - Bump Node.js floor 14 → 18 to match the built-in fetch used by the Agent Guard check (already required on main; README was wrong). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… conf.v6 The Step-0 activation check parsed ~/.jfrog/jfrog-cli.conf.v6 directly to find the default server's URL and access token. Two problems on current JFrog CLI setups made the check falsely report "disabled", silently aborting every jfrog-mcp-* skill: - Newer CLIs do not persist `accessToken` in conf.v6, so token resolution returned null even with a valid default server. - The platform URL is sometimes stored only as an `/artifactory`-suffixed URL, which is wrong for the `/ml/core/...` settings endpoint (404). Switch Path B back to `jf config export` (the approach the removed SessionStart hook used), which returns the platform-root `url` and the access token. This also re-aligns with the skills' own rule against parsing conf.v6 directly (the CLI masks tokens there). Also correct the README prerequisite: the env var is `JFROG_URL` (not `JFROG_PLATFORM_URL`), and only `jf config show` takes `--format` (`jf config export` emits base64 and has no such flag). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two AX-1476 acceptance criteria the skills were missing: - When the JFrog platform is reachable but the MCP registry is disabled on the account (settings API returns value:false), agent-guard-check.mjs now exits 2 and the skills tell the user to contact their admin / IT, instead of silently aborting. Auth/permission/transport failures (401/403/other/timeout/network) stay exit 1 (silent) — only the unambiguous "disabled via the API" signal maps to exit 2. - Step 0 may now proceed on a non-zero exit when the user explicitly asks to use the Agent Guard, or the workspace already has an @jfrog/agent-guard mcpServers entry. The exit-code contract and the exceptions live once in the shared references/agent-guard-activation.md; the three skills' Step 0 sections link to it rather than each copying the block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror the content changes made on the jfrog-skills PR (CodeRabbit review
fixes + context trims) back into this repo, keeping claude-plugin's own
structure (root-level scripts/agent-guard-check.mjs and ../../scripts/
references — NOT jfrog-skills' in-skill script layout).
- agent-guard-check.mjs: only an exact `false` for mcpGatewayPluginEnabled
is registry-disabled; missing/malformed values return the ordinary error
result instead of exit 2.
- install SKILL.md: --server rendered conditionally (pass it whenever the ID
came from an existing mcpServers entry OR jf config; omit only on the
JFROG_URL+token env path); corrected the unset-${VAR} behaviour (loads with
a warning, tool calls fail at runtime); deduped the server/project
resolution onto agent-guard-common.md; shortened description.
- list/remove SKILL.md: shortened descriptions; comma-separated allowed-tools;
remove now resolves scope across .mcp.json + ~/.claude.json and preserves
the OAuth cache for surviving scopes.
- persisting-env-vars.md: no setx for secrets; $SHELL is the login shell.
- agent-guard-common.md: document the legacy JF_URL/JF_ACCESS_TOKEN fallback.
- runtime-permissions.md / agent-guard-activation.md: consistent wording.
Skill routing + install/list smoke-tested against the real catalog on sonnet
with this repo's layout.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rog/jfrog-skills
The jfrog-mcp-{install,list,remove,shared} skill packages and
scripts/agent-guard-check.mjs were committed directly under skills/, but
everything under skills/ is vendored from jfrog/jfrog-skills via
sync-skills.mjs (which wholesale-replaces the tree). These skills live
upstream in jfrog/jfrog-skills PR 51 and bundle their own
jfrog-agent-guard-check.mjs, so committing them here is redundant and
would be overwritten on the next pin bump.
This branch now only removes the SessionStart hook and its machinery
(hooks.json, inject-instructions.mjs, validate-hook-injector.mjs, the CI
workflow, and the injected template) plus the README wording updates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Retires the legacy SessionStart hook that force-injected a ~480-line MCP-management template into every session. JFrog MCP management now lives in discoverable skills, and those skills are vendored into this repo via sync from
jfrog/jfrog-skillsrather than hand-committed here — so this branch removes the hook machinery and drops the hand-maintained skill copies to avoid a second source of truth.Net change vs
main(pure removal — 882 deletions, 2 insertions)hooks/hooks.json— the SessionStart hook definitionscripts/inject-instructions.mjs— the injector that force-fed the templatescripts/validate-hook-injector.mjs— its validatortemplates/jfrog-mcp-management.md— the ~480-line always-on template.github/workflows/validate-inject-instructions.yml— CI for the injectorREADME.md— prerequisites tweak reflecting the hook→skills moveWhere the skills live now
The three MCP skills (
jfrog-mcp-install/-list/-remove) + their shared references and Step-0 gate script are maintained injfrog/jfrog-skills(PR #51 there) and arrive in this plugin through the vendored sync, not by hand-committing them here. The intermediate commit that hand-ported them (a139d3e) is superseded byc3b1d93, which drops them in favor of the sync.Test plan
hooks/hooks.json,inject-instructions.mjs,validate-hook-injector.mjs,templates/jfrog-mcp-management.md) remain in the repo or CI.🤖 Generated with Claude Code