feat(marketplace): update installed marketplaces - #35
Merged
Conversation
Contribute one update participant covering every installed marketplace, from the marketplace root plugin rather than from a discovered child: a marketplace whose current commit fails to load contributes no child definitions, and updating is exactly how a user fixes that. Gathering reports a clone by the commit its checkout holds, preferring a tag reachable from it, and a referenced local directory as live without running a single Git command against it. A clone's remote is fetched with its tags and its default branch re-resolved, so a renamed default branch is followed rather than reported as a missing ref, and an available version is reported only when the target differs from the current commit. Fetching is non-interactive on the same terms cloning is, which is now one exported definition rather than two: an update walks every installed marketplace, so one credential prompt would stall the whole run. Applying re-reads the checkout, moves it onto the target detached, and runs the validation and dependency installation adding a marketplace runs. A checkout carrying modified tracked files is refused, as is a target the installed commit is not an ancestor of; both are reported as detail while gathering, so a dry run says what is in the way. Untracked files block nothing — dependency installation writes them into every checkout — except one occupying a path the target tracks, which an ordinary checkout already refuses and which is reported rather than forced past. When preparation fails after the checkout moved, the previous commit is restored and the failure says that installed dependencies were not. `marketplace list` gains the same label as a version column, read out of the checkout with no remote contacted.
Describe the marketplace update participant in the plugin guide: version labels, live references, what a dry run does and does not touch, the two blocking conditions and their remedies, and the restoration a failed preparation performs. Note the version column `marketplace list` now reports. Correct the updates specification's implementation note, which said no participant was contributed yet, and complete change 0013 in both documentation indexes.
Listing and updating both ask whether an installed marketplace is a live reference, and both answered it with their own `lstat`. Move the question to the module that owns marketplace storage so the two cannot drift, and settle the manager's Git execution handle once in its constructor rather than rebuilding it on every read.
…ures as themselves Three failures the update participant reported as something else. A fetch runs inside the checkout it names, so Git applies that checkout's own `core.sshCommand` and it outranks both configuration files. The probe that decides whether to inject batch mode read only the global and system files, whose rationale — a clone creates the repository it writes into and never applies the local scope of wherever the caller is standing — does not transfer to a fetch. So a deploy key pinned in one marketplace's checkout was silently overridden on every update. The probe now takes the checkout when the operation has one, asks it first, and a clone still passes none, leaving clone behaviour byte-identical. A fetch that fails was caught by the same handler as an unreadable checkout and told the user to run `tx marketplace remove`. An offline run, an expired credential, and a moved repository are failures of the fetch rather than of the installation, and advising removal for one is advising a user to lose a working marketplace. They are now reported as themselves, keeping the version label the checkout already answered. The restoration that follows a failed preparation was unguarded, so a restore that itself failed replaced the failure the user has to act on and left the checkout on the new commit with nothing saying so. The preparation failure stays the headline, the restoration failure is reported beside it, and the commit the checkout is stuck on is named. Record as an open question, rather than implement, whether a lockfile tx's own dependency installation rewrote should count as a user modification for the blocking rule.
Two ways the update participant fell short of what it promises, both found reviewing the branch. The restoration after a failed preparation was an ordinary checkout, so it refused whenever preparation had rewritten a tracked file — a dependency install rewriting a committed lockfile before failing is ordinary — and left the marketplace on the commit that had just failed validation, which is the outcome the restoration exists to prevent. It is now forced, and only it: a move forward still refuses to overwrite anything. Nothing forcing can discard is the user's, because the blocking checks found the checkout clean before tx moved it, so every tracked change undone was made after that by the preparation being undone. A failed fetch was reported with Git's message verbatim. Git quotes the URL it was working with, so a marketplace installed from a source carrying a token printed that token to the terminal and into whatever collects it. The message now has the recorded remote's credential runs taken out of it, through the same redaction the clone failure uses, with the host and path left intact for the user to read. The remote-URL read `marketplace list` performed inline becomes the one function both callers use.
…olumn on its own Two version labels that could come back wrong. A publisher who tags the commit a user already has, advancing nothing else, changes only what that commit is called. Gathering read the label before fetching, so it reported the abbreviated hash while a later `marketplace list` reported the tag — two spellings of one commit, where the label is required to be the same one. It is read again once the fetch has brought the tags in; the pre-fetch read stays, because a fetch that fails still has to report the version the checkout is on. `marketplace list` read a checkout's source and its version under one `try`, so a checkout whose origin was removed reported an unknown version too, though its HEAD and its tags were perfectly readable. Each column is now read on its own and falls back on its own.
There was a problem hiding this comment.
Pull request overview
Adds first-class support for updating installed marketplace checkouts via the generic tx update lifecycle, ensuring updates are safe (refuse dirty checkouts), efficient (fetch + move, not re-clone), and recoverable (forced rollback on preparation failure). This aligns the marketplace plugin with the Updates spec and makes marketplace list report a local version label consistently with tx update.
Changes:
- Introduces a marketplace update participant that gathers installed marketplaces (including “live” symlink references) and applies updates by moving to the remote default branch commit, re-preparing, and rolling back on failure.
- Extends marketplace Git utilities to support commit/label reads, modified-tracked-file detection, ancestry checks, non-interactive fetch, and safe restore semantics (forced rollback only).
- Updates CLI output and documentation so
marketplace listincludes a version label column and specs/indexes reflect Change 0013 as complete, with comprehensive new/updated tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/standalone.test.ts | Updates standalone CLI expectation for marketplace list to include the version column (live). |
| test/plugin-system.test.ts | Updates CLI list expectations for local marketplace sources to include the live version column. |
| test/marketplaces.test.ts | Expands manager listing tests to include version, validates git describe usage, and adds coverage for missing-remote version reads. |
| test/marketplace-updates.test.ts | New end-to-end tests for marketplace update gather/apply behavior, blocking conditions, rollback, ordering, and environment rules. |
| test/marketplace-plugin.test.ts | Verifies marketplace plugin contributes an update participant and that tx update --dry-run reports installed marketplaces correctly. |
| test/helpers.ts | Adds fixtureGit and commitFixtureFiles utilities to make Git fixture behavior deterministic and reusable. |
| test/cli.test.ts | Updates marketplace list CLI output expectation to include the version column. |
| plugins/marketplace/updater.ts | New MarketplaceUpdater implementing UpdateParticipant for gathering and applying marketplace updates with rollback and redacted fetch failures. |
| plugins/marketplace/storage.ts | Adds shared isMarketplaceReference predicate so listing and updating agree on “live reference” detection. |
| plugins/marketplace/manager.ts | Adds version column support, shared Git execution helpers, non-interactive remote operations, commit/label/ancestry/dirty checks, and restore/move primitives. |
| plugins/marketplace/index.ts | Updates marketplace list output format and registers the marketplace update participant during plugin initialization. |
| docs/specs/updates/index.md | Updates narrative to reflect that Change 0013 now contributes the first update participant. |
| docs/manual/plugins.md | Documents version column and marketplace update behavior, including non-interactive fetch semantics and credential redaction. |
| docs/index.yml | Marks Change 0013 as complete in the docs index data. |
| docs/index.md | Marks Change 0013 as complete in the rendered docs index table. |
| docs/changes/0013-update-installed-marketplaces.md | Marks status complete and checks off implementation/testing/documentation tasks; records open question about tool-rewritten lockfiles. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
A cloned marketplace is frozen at the commit it was installed from: an author pushes a fix and every user stays on the old code until they notice, remove the marketplace, and add it again. Remove-and-add is not a correct update either — it re-clones the whole repository for what is usually a few commits, and it passes the "already installed" check by first destroying the installation, so any failure between the two commands leaves the user with nothing rather than with the old version. This makes the marketplace plugin a participant in
tx update, built on the lifecycle from 0012, so that an update can never end in less than what you started with.Change: docs/changes/0013-update-installed-marketplaces.md. Specs: Updates, Plugin System.
What it does
Gathering walks installed marketplaces in discovery order. A symbolic link is a live reference: it is reported with a
livelabel and no available version, and Git is not reached at all, because its contents are whatever the target directory holds when you runtx. A clone reports its current commit's label — preferring a tag reachable from that commit over an abbreviated hash — fetches its remote with tags and re-resolves the remote's default branch so a branch rename is followed, and reports an available version only when the target differs from what it holds.Applying moves the checkout onto the target commit, detached, then runs the same validation and dependency installation that adding a marketplace runs, since the new commit may declare different plugins or different dependencies, and reports the resulting label. When that preparation fails, the recorded previous commit is restored and the failure is reported with the restoration stated — and with installed dependencies explicitly not restored, rather than claiming otherwise.
Two conditions block an update, and both are reported as item detail so a dry run says what is in the way before anything refuses for real: a tracked file the user modified, and a current commit that is not an ancestor of the target (a rewritten upstream history, which
txwill not resolve by guessing). Applying re-reads the checkout and repeats those checks, adding one that only matters at that moment — an untracked file occupying a path the target commit tracks, which cannot be both kept and moved onto.marketplace listgains a version column showing the same label, with no remote contact.Review notes
Two P1 defects were found and fixed during pre-PR review. The rollback used an unforced checkout, so a preparation that rewrote a tracked file before failing left the marketplace sitting on the commit that had just failed validation — the exact outcome restoration exists to prevent. It is now a forced restore, and only the restore: the forward move still refuses to overwrite anything, and nothing forcing can discard is the user's, because the blocking checks proved the checkout clean before it moved. Separately, a failed fetch reported Git's message verbatim, which prints an embedded credential; it now goes through the same redaction the clone failure path uses.
One open question is deferred, recorded in the change document and deliberately not implemented: should a lockfile that
tx's own dependency installation rewrote count as a user modification? A marketplace that commitsbun.lockand whose install rewrites it acquires a modified tracked file the user never produced, and the blocking rule then refuses every later update until they resolve it by hand. Exempting a path becausetxwrote it is a hole in a rule whose whole value is that it has none; refusing on a modification the tool itself made is a trap. Either direction needs an amendment to the Marketplace Updates spec section that owns the rule, so this change records the question and changes nothing. It concerns the blocking rule at gather and apply time only — the restoration path already discards what a failed preparation rewrote.Test plan
bun run checkpasses with 100% statement, function, and line coverage.tx marketplace add owner/repoagainst a repository with at least one tag, thentx marketplace list— the middle column shows the tag name, and the command completes with no network access.tx update --dry-run— the marketplace is listed with its current label and the new available version, andgit -C ~/.tx/marketplaces/<name> rev-parse HEADis unchanged afterwards (a dry run fetches but never moves the checkout).tx updateon that same marketplace — it reports the new label,rev-parse HEADnow matches the remote default branch, and the checkout is detached.tx updateagain with nothing new upstream — the marketplace reports up to date and is not revalidated.tx marketplace add "file://$PWD/somedir", thentx update --dry-run— the reference reportslivewith nothing to apply, and its directory is untouched.tx update --dry-run— the item reports the modification as the blocking detail; runtx updateand confirm it refuses and the edit survives.tx update— it refuses, names the path, and the file survives. — not exercised by hand; covered by the automated untracked-collision case, which asserts the refusal and that the file survives intact.tx update --dry-run— the item reports the non-ancestor condition rather than offering to move. — not exercised by hand; covered by the automated non-ancestor case, which asserts the checkout did not move.tx update— the failure message is Git's, with the credential redacted, and no token appears in the output. — not exercised by hand; covered by the automated redaction case, which asserts the token and username are gone while host and path survive.Verification evidence
bun run checkis green: 407 tests, 0 failures, 100% statement, function, and line coverage, build succeeds.The first seven items were additionally exercised by hand against a real local Git repository served over
file://, withXDG_DATA_HOMEpointed at a throwaway directory.marketplace listshoweddemo v1.0.0 <source>, preferring the reachable tag over an abbreviated hash and contacting no remote. After a new upstream commit and av1.1.0tag,tx update --dry-runreporteddemo v1.0.0 -> v1.1.0andrev-parse HEADwas byte-identical afterwards, confirming the dry run fetches without moving the checkout.tx updatethen reporteddemo updated to v1.1.0, left the checkout detached atv1.1.0, and a secondtx updatereporteddemo v1.1.0 up to date. A local reference added from a directory reportedlively live up to datewith nothing to apply. Finally, editing a tracked file in the checkout producedblocked: modified tracked files (demo/index.ts); resolve them in the checkoutas dry-run detail, and the realtx updaterefused with that message, exited 1, leftrev-parse HEADunchanged, and the edit survived intact.Worth noting from that run: the marketplace's own plugin failed to load throughout, and the participant still gathered, reported, and applied it correctly — which is the "a participant must work while the things it manages are broken" requirement, observed rather than only asserted.