chore: sync modules to v0.8.0 - #50
Conversation
Document gradle as its own type, multi-package-manager autoSetup families, and schema-2 receipts keyed by package-manager token. Co-authored-by: Cursor <cursoragent@cursor.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
YoniMelki
left a comment
YoniMelki
left a comment
There was a problem hiding this comment.
Automated thorough-review pass (3 independent lenses + independent per-finding scoring). Two findings verified at 75/100 confidence (an upstream silent-overwrite bug amplified by this PR's package-manager expansion, and a cache-schema migration that churns under mixed plugin versions) and one at 50/100 (a background-lock timing margin issue with a cheap fix available). Requesting changes so these get an explicit answer before merge — details inline.
|
|
||
| function runJfSetup(pm, serverId, repoKey) { | ||
| const args = ["setup", pm, "--server-id", serverId, "--repo", repoKey]; | ||
| function runJfSetup(packageManager, serverId, repoKey) { |
There was a problem hiding this comment.
[Confidence: 75/100] Option C triples exposure to an open, unaddressed silent-overwrite bug upstream
runJfSetup unconditionally calls jf setup <pm> --server-id --repo, and applySetupResult in eager-setup-receipt.mjs only ever records {repoKey, status, configuredAt, reason} — no prior value, no detection of what it replaced.
Upstream issue JFROG/jfrog-agent-hooks#57 (open, github.jfrog.info) documents that jf setup <pm> silently repoints a machine's existing package-manager config to a different JFrog instance with no detection, no backup, and no undo. This PR's "Option C" expansion means one autoSetup-governed type can now trigger 2-3 of these unconditional overwrites in one session instead of 1 (e.g. pypi → pip + pipenv + uv), and neither the PR body nor commit messages acknowledge #57.
Suggest: either land a repoint-guard/backup alongside this expansion, or at minimum flag #57 explicitly in the PR description so reviewers can make an informed call before merging.
| typeof data.servers === "object" | ||
| !data || | ||
| typeof data !== "object" || | ||
| data.schemaVersion !== RECEIPT_SCHEMA_VERSION |
There was a problem hiding this comment.
[Confidence: 75/100] Schema v1→v2 discard causes wipe-and-rerun churn when two plugin versions share the cache file
normalizeReceipt discards the entire file whenever schemaVersion !== 2 — including a validly-stamped 1, not just malformed/missing data. The pre-PR (v0.7.0) code unconditionally writes schemaVersion: 1 with no awareness schema 2 exists, and both versions read/write the same shared path (~/.jfrog/skills-cache/package-setup.json).
This module ships into more than one independently-versioned IDE plugin (e.g. Cursor's jfrog plugin vs. a Claude Code counterpart). A user running both, with only one updated to v0.8.0, gets a perpetual downgrade/wipe cycle rather than a one-time migration: whichever tool ran last each session flips the schema and forces the other to wipe-and-rerun every jf setup call on its next session.
Suggest: either make the old-schema write path tolerant of a newer file it doesn't understand (skip writing rather than downgrading), or version-gate more defensively so this only fires once per real upgrade rather than on every version mismatch.
| function staleThresholdMs(pmCount) { | ||
| return Math.max(PER_PM_TIMEOUT_MS * Math.max(pmCount, 1), 120_000); | ||
| function staleThresholdMs(packageManagerCount) { | ||
| return Math.max(PER_PACKAGE_MANAGER_TIMEOUT_MS * Math.max(packageManagerCount, 1), 120_000); |
There was a problem hiding this comment.
[Confidence: 50/100] Lock stale-threshold has near-zero margin under max Option C fan-out
With all 8 governed types at full package-manager-family size, max job count is 13, giving staleThresholdMs = 60_000 * 13 = 780_000ms. Worst-case actual worker runtime (all 13 jf setup calls hitting their own 60s timeout, plus the worker's own 5s jf setup --help probe) is ≈785s — longer than the 780s threshold. A legitimately-still-running worker's lock can be reclaimed by a second session, letting two jf setup processes race against the same shared config files (~/.npmrc, ~/.docker/config.json, etc.).
Note syncWorkerTimeoutMs (a sibling function) already adds a +30_000 buffer that this function does not — applying the same buffer here would close the gap cheaply. Triggering the actual worst case requires every package-manager setup call to hang for its full timeout simultaneously (a broad, sustained outage), so this is a real but comparatively rare compound scenario.
3 participants
Vendored from
jfrog-agent-hooks/v0.8.0ofJFROG/jfrog-agent-hooks.Updated paths under
plugins/jfrog:Also updated:
.github/scripts/sync-modules-vendor.json(source pin + copies manifest).Not included: hooks/hooks.json assembly — verify sessionStart argv wiring separately.
Source: https://github.jfrog.info/JFROG/jfrog-agent-hooks/actions/runs/1201471