Skip to content

fix: refresh stale module clones so amplifier-agent update delivers upstream fixes (v0.14.2) - #141

Closed
Salil Das (sadlilas) wants to merge 1 commit into
mainfrom
fix/refresh-stale-module-clones
Closed

fix: refresh stale module clones so amplifier-agent update delivers upstream fixes (v0.14.2)#141
Salil Das (sadlilas) wants to merge 1 commit into
mainfrom
fix/refresh-stale-module-clones

Conversation

@sadlilas

Copy link
Copy Markdown
Collaborator

Summary

amplifier-agent update currently cannot deliver an upstream module fix to an existing
install. This ships the missing piece as v0.14.2.

Module sources are declared with a floating @main ref, but foundation resolves a git
source by returning the existing clone directory whenever it is present and structurally
intact — it never fetches into it (amplifier_foundation/sources/git.py, resolve()):

if cache_path.exists():
    if not self._verify_clone_integrity(cache_path):
        rmtree_robust(cache_path)
    else:
        ...
        return ResolvedSource(active_path=result_path, source_root=cache_path)

No fetch, no ref comparison, no commit check. The clone path is sha256(git_url@ref)[:16],
so a floating-ref clone owns one stable directory that is written exactly once — at first
install — and pinned to whatever commit main pointed at that day, for the life of the
machine.

Reinstalling does not help. uv tool install --reinstall --force empties the tool venv and
every module is genuinely reinstalled, but each one rebuilds from the same frozen clone,
restoring the same stale code and its stale dependency pins.

Why a version bump alone does not fix it

Worth stating explicitly, because it is the intuitive fix and it does not work. I measured a
bare version bump against a machine in the pre-fix state:

Signal Before After bare bump Moved?
amplifier-agent --version 0.14.1 0.14.2 yes
Cold prepare ran yes yes
All 29 modules reinstalled yes yes
Module clone commit pre-fix pre-fix no
Module dependency pin stale stale no

The machine reports the new version, passes doctor, reinstalls everything — and still runs
the old module. That is worse than an obvious failure, because every surface says healthy.

The fix

A one-time migration in the post-install hook deletes cached amplifier-module-* clones
before priming, so the cold prepare that follows has nothing to reuse and clones afresh.

  • Marker at <state_root>/migrations/<id>.done; runs at most once per machine.
  • Wrapped in try/except at the call site and never raises — it cannot fail an install.
  • Ordered before priming deliberately: the migration only removes clones; the prepare that
    follows is what re-creates them.

The bump and the migration must ship together. post_install.main() returns early when
the prepared-bundle cache for the running version already exists. Ship the migration without
the bump and post-install skips priming entirely — deleting the clones and never re-cloning
them, which is strictly worse. The bump is what forces the cold prepare. That coupling is
the reason this is a release PR rather than a plain fix.

Verification

Measured in an isolated container, from a faithful pre-fix state (module clone checked out at
a pre-fix commit and editable-installed, matching dependency pin installed, warm
prepared-bundle cache generated in that state), then running exactly what
amplifier-agent update runs — uv tool install --reinstall --force followed by
amplifier-agent-post-install:

amplifier-agent: migration 0.14.2-refresh-stale-module-clones: removed 20 stale module clone(s)
amplifier-agent: prepared bundle cached at ~/.amplifier-agent/cache/prepared/0.14.2/1e36b1aa08b8ff8d
Check Result
Module clone commit moved off the pre-fix commit to current main
Module dependency pin updated to the current declared constraint
Fixed code present in the importable module yes
Transitive dependency resolved to the new floor yes
amplifier-module-* distributions in venv 29
amplifier-agent doctor all green
Second post-install run cache already prepared; clone untouched; nothing re-wiped

Repo gate (make check equivalent — make is unavailable on this machine, so the three
commands were run directly):

ruff check src/ scripts/ tests/ wrappers/          → All checks passed!
ruff format --check src/ scripts/ tests/ wrappers/ → 147 files already formatted
pyright src/                                       → 0 errors, 0 warnings

No test file was added: tests/ in this repo holds e2e DTU suites and nothing else, by the
explicit convention recorded in [tool.pytest.ini_options].

Scope and limits

Deliberately narrow to amplifier-module-*. Bundle and foundation clones are frozen by
the same mechanism. Widening the filter to amplifier- would re-clone roughly a third more
repositories to fix a problem nobody has reported. Easy to widen later if it ever bites.

Only reaches users who run the post-install hook — that is install.sh and
amplifier-agent update. A user who runs uv tool install directly bypasses it and stays
stale.

The offline path is untested. If the network drops between the wipe and the re-prime, the
user is left with no module clones and a cache miss. post_install swallows the exception and
the next run re-prepares and re-clones, so it self-heals — but that first run is slow and will
fail hard if still offline.

Follow-up, not in this PR

This is a workaround at the app layer. The underlying behaviour — a floating @main clone
written once and never re-fetched — lives in foundation and affects every app that consumes
modules this way, so the same class of staleness will recur until it is addressed there.

A second foundation-level contributor: ModuleActivator._install_dependencies() skips
reinstalling when a distribution of the same name is already present, checked before the
pyproject.toml fingerprint comparison on the following line. The fingerprint check would
otherwise have caught a changed dependency constraint. That guard is why deleting the clone,
rather than merely reinstalling, is the only reliable repair today.

… upstream fixes

Module sources are declared with a floating `@main` ref, but foundation resolves a
git source by returning the existing clone directory whenever it is present and
structurally intact — it never fetches into it. The clone path is derived from
sha256(git_url@ref), so a floating-ref clone owns one stable directory that is written
exactly once, at first install, and stays pinned to whatever commit `main` pointed at
that day for the life of the machine.

Reinstalling does not help. `uv tool install --reinstall --force` empties the tool
venv and every module is genuinely reinstalled, but each rebuilds from that same
frozen clone, restoring the same stale code and its stale dependency pins.

The fix is a one-time migration in the post-install hook that deletes cached
`amplifier-module-*` clones before priming, so the cold prepare that follows has
nothing to reuse and clones afresh. It records a marker under `<state_root>/migrations/`
so it runs at most once per machine, and it is wrapped in try/except at the call site
so it can never fail an install. It is ordered before priming deliberately: the
migration only removes clones; the prepare that follows re-creates them.

The version bump to 0.14.2 is load-bearing, not cosmetic: `post_install.main()`
returns early when the prepared-bundle cache for the running version already exists,
so shipping the migration without a bump would skip priming entirely — deleting the
clones and never re-cloning them. The bump is what forces the cold prepare. That
coupling is why the release bump and the fix travel in one commit.

Verified in an isolated container from a faithful pre-fix state: running exactly what
`amplifier-agent update` runs produced "removed 20 stale module clone(s)", moved the
module clone to current main, updated its dependency pin, resolved the transitive
dependency to the new floor, kept 29 modules installed, and left `doctor` all green.
A second post-install run printed "cache already prepared" and re-wiped nothing.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Salil Das (sadlilas) pushed a commit that referenced this pull request Aug 23, 2026
The bump is not cosmetic. `update` compares versions and exits early with
"Already up to date" when they match, so without a new release tag the
reinstall never runs and no user receives this fix.

Same coupling PR #141 identified, for a different reason: #141 needed the bump
because post-install would early-return on a warm cache; this needs it because
`update` never gets that far.
@sadlilas

Copy link
Copy Markdown
Collaborator Author

Flagging an overlap rather than acting on it — this is your PR, so the call is yours.

#142 supersedes the outcome this PR is after, by a different route. Instead of deleting stale clones from ~/.amplifier/cache, it relocates the clone root to ~/.amplifier-agent/foundation/cache. On every existing machine that directory starts empty, so the first cold prepare after upgrading clones everything fresh from current main — the same result, arrived at as a consequence of ownership rather than as a migration.

Two reasons it may be worth closing this rather than merging both:

  1. After fix: own the module cache, and refresh it by resolving refs instead of deleting clones #142, _module_clone_root() points at a directory amplifier-agent no longer owns. The migration would delete amplifier-app-cli's clones — a cross-application side effect, and app-cli would then re-clone them.
  2. The 0.14.2 version number collides. fix: own the module cache, and refresh it by resolving refs instead of deleting clones #142 now carries 0.14.2 for the same reason yours does: update exits early with "Already up to date" when versions match, so without a bump the reinstall never runs.

Your diagnosis is what made #142 possible — the _module_clone_root() docstring is what identified the shared directory, and the "bump and migration must ship together" finding applies unchanged. Both are cited in #142.

One thing from your "Follow-up, not in this PR" section is still open and unaddressed by #142: ModuleActivator._install_dependencies() skipping reinstall when a distribution of the same name is already present, checked before the pyproject.toml fingerprint comparison. That is still a foundation-level bug.

Salil Das (sadlilas) pushed a commit that referenced this pull request Aug 24, 2026
Completes the absorption of #141 so it can be closed:

- uv.lock version bump, which #141 carried and this branch had missed
- #141's stale-clone diagnosis folded into the 0.14.2 changelog entry
- legacy clones in ~/.amplifier/cache surfaced by doctor and removable via
  cache clear --legacy

#141 deleted those clones unconditionally. That was right while the directory
was the one amplifier-agent used; after the relocation it is foundation's
default for every Amplifier app, keyed with no per-app namespacing, so on a
machine running app-cli they are its live clones and indistinguishable from
our leftovers. Reported and offered rather than deleted.
@sadlilas

Copy link
Copy Markdown
Collaborator Author

Follow-up: #142 now absorbs this PR completely, so it can be closed without losing anything. Element by element:

This PR ships Where it lives in #142
pyproject.toml0.14.10.14.2 Same bump, same reasoning
uv.lock — workspace member bump Absorbed. #142 had genuinely missed this; caught by diffing against your PR
CHANGELOG.md — the stale-clone diagnosis Folded into #142's 0.14.2 entry, including the "bump is load-bearing" argument
post_install.py_refresh_stale_module_clones() + marker Split in two — below

The refresh behaviour is now permanent rather than one-shot. #142 relocates the clone root, so the new root starts empty on every existing machine and the first cold prepare clones from current main. post_install then repeats that on every version change. No marker file: the version-keyed prepared-bundle cache already supplies the idempotence, since the refresh is only reachable on a cold cache.

The one thing #142 deliberately does not copy is the unconditional delete. Your migration was correct while ~/.amplifier/cache was the directory amplifier-agent used. After the relocation it is foundation's default for every Amplifier app, keyed sha256(git_url@ref)[:16] with no per-app namespacing — so on a machine also running amplifier-app-cli those are its live clones, indistinguishable from our leftovers, and deleting them could pull one out from under a running session.

Your underlying concern is kept. doctor now reports them with the app-cli caveat, and cache clear --legacy removes them on request:

[INFO] legacy module clones: 2 in /root/.amplifier/cache (391 KB) — no longer used by
amplifier-agent. amplifier-app-cli may still use them; remove with
`amplifier-agent cache clear --legacy` only if you do not run it.

Verified in a clean container: advisory never fails --strict, a real turn leaves them untouched, plain cache clear leaves them, --legacy removes exactly the amplifier-module-* ones (your narrow scope preserved — the bundle clone survives), and a second run is a no-op.

Your _module_clone_root() docstring is what identified the shared directory in the first place; the decoupling followed from it.

Still unaddressed by either PR, from your follow-up section: ModuleActivator._install_dependencies() skipping reinstall on a name match, checked before the pyproject.toml fingerprint comparison. Foundation-level, still open.

@sadlilas

Copy link
Copy Markdown
Collaborator Author

Correction to my previous comment: the doctor advisory and cache clear --legacy flag I described there have been removed from #142. It now ships no route to those directories at all — no deletion, no flag, no report.

The reasoning I gave was wrong in one respect: I framed the clones in ~/.amplifier/cache as stranded disk. For amplifier-app-cli users — a large population — they are not stranded, they are live. And from inside amplifier-agent the two cases are indistinguishable, since clones are keyed sha256(git_url@ref)[:16] with no per-application namespacing.

So an affordance to delete them is, for a substantial share of the people who would see it, a button that breaks a different application. The caveat I had put in the help text documented the damage rather than preventing it. Reporting alone is the same footgun with extra steps, because the only actionable follow-up is a manual rm -rf with identical blast radius.

Everything else in my previous comment stands: the version bump, the uv.lock bump, your changelog narrative, and the refresh behaviour are all absorbed, with the refresh now permanent rather than one-shot.

Net effect for a user upgrading: your intent — never leave someone silently running stale modules — is fully achieved by the relocation, since the new clone root starts empty and everything re-clones from current main. The old directory simply stops being consulted and is left untouched. Agent-only users who want that disk back can remove it themselves; nothing in the product will suggest it.

Salil Das (sadlilas) added a commit that referenced this pull request Aug 24, 2026
…f deleting clones (#142)

amplifier-agent wrote its module clones into ~/.amplifier, a tree owned by amplifier-app-cli, and those clones were frozen at first install forever.

Nothing in this repository caused the coupling: foundation resolves its storage root from AMPLIFIER_HOME and falls back to ~/.amplifier, and this app never set it. AMPLIFIER_HOME is now bound to <agent home>/foundation before amplifier_foundation is imported.

Foundation keys each clone on sha256(git_url@ref) and reuses any directory that exists. With a floating @main the key never changes while the commit it names does. Rather than deleting directories to work around that, floating refs are now resolved to their current commit before prepare(), so the key identifies the content: an unmoved branch downloads nothing, a moved one clones fresh, and an unreachable remote falls back to the commit already on disk.

Not pinning: bundle.md still declares @main and is never rewritten, so module fixes still reach users without an engine release.

Also: relocated recipe session state, the context-intelligence reader root, provider rate-limit state and the ChatGPT OAuth token (with copy-forward) out of app-cli's tree; made update check module drift when the engine is already current; and made every storage root honour $AMPLIFIER_AGENT_HOME.

Supersedes #141. Requires microsoft/amplifier-bundle-skills#61.

Release: 0.15.0
@sadlilas
Salil Das (sadlilas) deleted the fix/refresh-stale-module-clones branch August 24, 2026 20:36
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.

1 participant