Skip to content

fix(init): install the skills we already ship when the pinned source can't be reached - #692

Open
filip131311 wants to merge 1 commit into
mainfrom
filip/skills-install-fallback
Open

fix(init): install the skills we already ship when the pinned source can't be reached#692
filip131311 wants to merge 1 commit into
mainfrom
filip/skills-install-fallback

Conversation

@filip131311

Copy link
Copy Markdown
Collaborator

Fixes #614.

Reproduced on a published build

0.18.1 never reached npm, but the canaries did — and canaries get no tag either, so this is live for every prerelease user right now, not only for the tarball in the report.

$ npm i -g @swmansion/argent@0.18.1-next.7        (clean VM)
$ argent init -y

Running: npx skills add …/packages/skills/skills#v0.18.1-next.7 --skill * -y
◇  Skills installation failed.
│  fatal: Remote branch v0.18.1-next.7 not found in upstream origin

◇  Summary ──────────────╮
│  Skills installed      │   ← after a visible failure
╰────────────────────────╯
EXIT: 0        skills on disk: 0

The fix

The package already ships every skill. SKILLS_DIR resolves inside the installed package, so the bundled copy is that version's skills by construction — the pinned ref buys canonical provenance, not fresher content. argent init --local --from <tgz> installed all fifteen while plain argent init from the same package installed none. Only the decision was missing, and the comment above the source choice already claimed it existed.

A failed install now retries against that bundled copy. The same fallback already exists on the update path (skills.ts:108-124), so this brings init in line rather than inventing a mechanism.

The retry decision is structural, not textual. The interactive path captures no output to match on, and git's missing-ref wording is a translated string — so a text rule would silently stop working outside an English toolchain. It retries when the source wasn't already the bundled copy and the bundled copy is readable; everything else fails as before, cheaply.

The summary keys on the outcome, and a fallback says so out loud. That's not just honesty: the lockfile then records a machine-local path instead of a shared source, which the user should know. A bundled source chosen deliberately — offline, --from, unknown version — stays an ordinary success, so the e2e path doesn't start crying wolf.

Two smaller lies went with it. The "install manually" hint printed the identical failing command; it now names the source that could work. And the reported cause was whichever line came out first — usually npm WARN using --force rather than the clone failure.

Telemetry gains source, used_fallback and fallback_reason. Without it, a release whose tag was never pushed looks perfectly healthy: every install quietly rescued, nobody filing anything.

Verified live

Same canary, same VM, fix grafted in:

▲  Could not install skills from …#v0.18.1-next.7: Failed to clone repository
│  Falling back to the copy bundled with this package.
◇  Skills installed from the bundled copy.
◇  Skills installed locally ─── (lock now records a local path; a later
                                 argent update on a tagged release restores it)

skills on disk: 15

Deliberately not in this PR

  • The exit code. I had planned to exit 1 on total failure and dropped it: skills failing doesn't stop MCP config, rules and agents from landing; the e2e already downgrades a missing skills-lock to skip, so exiting 1 would convert a tolerated state into a hard CI failure; and the one rule that mentions init tells the agent to hand the command to a human, not run it — so "an agent reads the exit code" doesn't hold. Worth its own change once telemetry shows how often skills genuinely fail.
  • A confirm before falling back in interactive mode. runNpxSkills uses stdio: "inherit" and there's no SIGINT handler, so Ctrl-C kills argent along with the TUI — the confirm would be unreachable for the case that justified it.
  • The release-process half. A publish-time tag check would prevent the whole class, but it belongs in CI. Note it can't be scoped to canaries: the publish workflow creates no tag for any channel, which is why the reported case was a stable version.

Corrections to my own analysis, for the record

  • I claimed the fallback's cost was that teammates inherit a machine-local lock. Wrong.gitignore excludes skills-lock.json and says why in as many words: it "bakes in machine-specific absolute skill paths, so it must never be committed." The cost is local refresh semantics, not team contamination, and it doesn't argue for making the fallback opt-in.
  • I described this as "canaries are never tagged". More accurately, publishing and tagging are decoupled for every channel — which is exactly why the reported failure was a stable 0.18.1.

Tests

7 new cases: pinned success touches nothing else; the #614 case falls back and is disclosed; a generic failure also falls back (proving the decision isn't regex-gated); no self-retry when the source is already bundled; no retry when the bundled copy is unreadable; both-failed names a usable command and not the failing one; and the scope flag survives the retry.

runNpxSkills moved to a leaf module so a test can assert which source each attempt used — the precise regression this is about.

Mutation-verified: disabling the fallback fails 4. Installer suite 542 green, telemetry 282 green.

…can't be reached

Skills are installed from a git ref pinned to the running version. Publishing and
tagging are decoupled — the publish workflow creates no tag at all — so a
published version whose tag was never pushed can never resolve. The clone fails,
nothing is installed, and the run still reported "Skills installed" and exited 0.

The package ships every skill inside itself, so the bytes were on disk the whole
time. `argent init --local --from <tgz>` installed all fifteen while plain
`argent init` from the same package installed none. Only the decision was
missing, and the comment above the source choice already claimed it was there.

A failed install now retries against that bundled copy. The same fallback already
exists on the update path, so this brings init in line rather than inventing
anything.

The retry decision is structural, not a match on the error text: the interactive
path captures no output to match against, and the usual cause — git reporting a
missing ref — is a translated string, so a text rule would quietly stop working
for anyone not running an English toolchain. It retries when the source was not
already the bundled copy and the bundled copy is readable, and otherwise fails as
before. Every failure the retry cannot rescue simply fails again, locally and
cheaply.

The summary now says what happened rather than which method was chosen, and a
fallback says so out loud. That matters beyond honesty: the lockfile then records
a path on this machine instead of a shared source, which is a real thing for the
user to know. A bundled source chosen deliberately — offline, --from, unknown
version — stays an ordinary success and reads as one.

Two smaller lies went with it. After the pinned ref failed, the "you can install
manually" hint printed the identical failing command; it now names the source
that could actually work. And the reported cause was whatever came out first,
which was usually an npm warning about --force rather than the clone failure.

Telemetry gains the source and whether the fallback was used. Without it a
release whose tag was never pushed looks perfectly healthy — every install
quietly rescued, nobody filing anything.

Deliberately not here: changing init's exit code. Skills failing does not stop
MCP config, rules and agents from landing, the e2e already tolerates a missing
skills lock, and the one rule that mentions running init tells the agent to hand
the command to a human rather than run it. Worth its own change once telemetry
shows how often skills genuinely fail.

Also not here: the release-process half. A publish-time check that the tag exists
would prevent the whole class, but it belongs in CI.

Verified on a published canary in a clean VM: the same install that produced zero
skills now produces fifteen, and says where they came from.

Fixes #614
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.

argent init: skills install fails for a published version whose git tag isn't pushed, and the summary still reports "Skills installed"

1 participant