feat(plugin-cli): add update-package command#1126
Conversation
Lets publishers edit an already-published profile (license, authors, security contacts, name/description/keywords) without cutting a new release. Dry-run by default; `--yes` writes via putRecord and bumps lastUpdated. Refuses to change the slug, and surfaces a clear rename warning when the manifest slug has drifted from the published one. Closes the gap from the publish flow's "Profile updates aren't supported yet" warning, which now points users at this command.
🦋 Changeset detectedLatest commit: fa90364 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Scope checkThis PR changes 1,299 lines across 6 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-perf-coordinator | fa90364 | May 20 2026, 09:40 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-i18n | fa90364 | May 20 2026, 09:40 PM |
|
/review |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | fa90364 | May 20 2026, 09:41 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | fa90364 | May 20 2026, 09:42 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | fa90364 | May 20 2026, 09:42 PM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
Pull request overview
Adds a new emdash-plugin update-profile command (and programmatic API) to update an already-published com.emdashcms.experimental.package.profile record in-place (dry-run by default; --yes applies via com.atproto.repo.putRecord), and updates publish messaging to direct users to this command.
Changes:
- Added
updateProfileprogrammatic API (diff + optional write) and comprehensive unit tests usingMockPds. - Added new
update-profileCLI command with human and--jsonoutput modes, plus wiring into the CLI entrypoint. - Updated
publishwarning text to point users atemdash-plugin update-profile, and added a changeset for the new feature.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-cli/tests/update-profile.test.ts | New test suite covering diffing, writes, refusal modes, and forward-compatible field preservation. |
| packages/plugin-cli/src/update-profile/api.ts | New exported programmatic API for reading/diffing/updating profile records. |
| packages/plugin-cli/src/index.ts | Registers the new update-profile command in the CLI. |
| packages/plugin-cli/src/commands/update-profile.ts | Implements the new CLI command (manifest load, auth/session, output formatting, error handling). |
| packages/plugin-cli/src/commands/publish.ts | Updates subsequent-publish warning to direct users to update-profile. |
| .changeset/big-hands-peel.md | Declares a minor bump for @emdash-cms/plugin-cli and documents the new command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** Authenticated client against the publisher's PDS. */ | ||
| publisher: PublishingClient; | ||
| /** Publisher DID. Used to construct AT URIs for display/output. */ | ||
| did: Did; |
| if (sibling !== null) { | ||
| throw new UpdateProfileError( | ||
| "POSSIBLE_RENAME", | ||
| `No profile at ${profileUri}, but the publisher already has a profile at slug "${sibling}". If you renamed the plugin in your manifest, that would orphan every release under "${sibling}" — revert the slug to "${sibling}" in emdash-plugin.jsonc, or publish the rename under the new slug as a fresh package (releases under "${sibling}" stay where they are).`, |
| json: { | ||
| type: "boolean", | ||
| description: | ||
| "Emit a single-line JSON object on stdout instead of human output. Success: {profile, diffs, written, cid?}. Failure: {error: {code, message}}. Human-readable progress goes to stderr.", |
| /** | ||
| * Reroute consola to stderr so stdout stays clean for `--json` consumers. | ||
| * Returns a restore function. Mirrors the publish command's helper so the | ||
| * two share a contract for json-mode pipe consumers. | ||
| */ | ||
| function redirectConsolaToStderr(): () => void { | ||
| const previous = consola.options.reporters?.slice() ?? []; | ||
| consola.setReporters([ | ||
| { | ||
| log(logObj) { | ||
| const level = logObj.type ?? "info"; | ||
| const tag = logObj.tag ? `[${logObj.tag}] ` : ""; | ||
| const args = logObj.args ?? []; | ||
| const message = args.map((a) => (typeof a === "string" ? a : JSON.stringify(a))).join(" "); | ||
| process.stderr.write(`${level}: ${tag}${message}\n`); | ||
| }, | ||
| }, | ||
| ]); | ||
| return () => { | ||
| consola.setReporters(previous); | ||
| }; |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adversarial review. Most of the surface here is solid — dry-run-by-default, identity-field preservation, and the POSSIBLE_RENAME diagnostic all do the right thing. Two behavior issues worth addressing before this lands, both around the manifest-as-source-of-truth model.
| diffs.push({ field, before, after: undefined }); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Silent data loss when a CLI user removes an optional field from their manifest.
Flow:
- Publisher publishes a plugin with
name: "My Plugin"in their manifest. The PDS profile record getsname: "My Plugin". - Publisher edits the manifest, removes the
namekey (maybe by accident, maybe because they thought the field was optional and didn't realize it was already on the record). - Publisher runs
update-profile --yes.
profileUpdateInputFromManifest only sets input.name if profile.name !== undefined, so input.name stays unset → normaliseInput leaves out.name undefined → buildProfileCandidate here treats after === undefined && before !== undefined as "user cleared the field" and deletes name from the PDS record. Same for description and keywords.
Dry-run does mitigate this (the user sees - name in the diff before --yes), but the doc comment at the top of update-profile.ts doesn't warn that removing a manifest key clears the PDS field, and the model is the opposite of publish (where the existing profile wins for these fields — that's exactly what result.ignoredProfileFields warns about). The publish warning now points users at update-profile, so they're trained to expect the same semantics.
At minimum: document this prominently and have the dry-run output flag deletes louder (the current ~ vs - distinction in renderDiffLine is easy to miss). Better: require explicit opt-in to clear an optional field (e.g. --clear name), or treat "absent in manifest" as "don't touch on PDS" and add a separate --clear mechanism.
| rkey: options.slug, | ||
| record: candidate, | ||
| skipValidation: true, | ||
| }); |
There was a problem hiding this comment.
TOCTOU between fetchExistingProfile and unsafePutRecord — concurrent writes silently overwrite.
We read the record on line 166, then write back on line 245 without passing a swapRecord precondition. If anything mutates the record between those calls (another update-profile invocation, a future admin UI flow, a manual PDS edit, even an aggregator-driven write if that ever exists), our putRecord silently overwrites it with our pre-modification view plus our diff applied — the other writer's changes vanish without warning.
Most realistic trigger: a publisher with a profile change pending in another shell, or pairing this CLI with manual PDS edits during the experimental phase.
com.atproto.repo.putRecord accepts swapRecord (CID-based optimistic concurrency); unsafePutRecord doesn't expose it today. Either thread swapRecord: existing.cid through (and add a STALE error code surfacing the conflict), or at minimum document that this command races with concurrent writers and is not safe to run while other edits are in flight.
| `Candidate profile record did not match the lexicon after merge.`, | ||
| { slug: options.slug, issues: candidateValidation }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
LEXICON_VALIDATION_FAILED error message blames the wrong party for valid-looking caller input.
Message says "a failure here genuinely indicates a record-shape regression in update-profile or in the lexicon itself." But validateInput only catches empty arrays, missing author names, and contacts-with-no-contact-info. The lexicon additionally enforces:
keywords: maxLength 5, each element maxLength 128 / 64 graphemesauthors: maxLength 32, name maxGraphemes 64, url maxLength 1024, url must parse as a generic URIsecurity: maxLength 8, url must parse, email maxLength 256name: maxGraphemes 100,description: maxGraphemes 140license: maxLength 256
A programmatic caller passing e.g. keywords: ["a", "b", "c", "d", "e", "f"] or authors: [{ name: "x", url: "not a url" }] would land at LEXICON_VALIDATION_FAILED with a misleading message that points at the implementation instead of the caller's input. The validation tree is dumped into detail.issues, but the top-level message is still wrong.
Either (a) extend validateInput to mirror the lexicon's structural rules so callers get INVALID_INPUT with a specific message, or (b) reword LEXICON_VALIDATION_FAILED to acknowledge it can also be caller input that bypassed validateInput.
| // trying to catch is a publisher renaming their plugin; a real | ||
| // publisher with that situation will still get the message on retry. | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Bare catch {} swallows auth/permission failures and degrades to a misleading PROFILE_NOT_FOUND.
If listRecords fails because the OAuth scope doesn't permit it, the session expired mid-flow, or the PDS returned 5xx, we silently return null and the caller throws PROFILE_NOT_FOUND with no hint that the rename-detection path didn't actually run. A publisher who just renamed their plugin will get a flat "no record" error and no idea their other profiles weren't even checked.
The comment justifies this for transient network failures, but ClientResponseError distinguishes auth/permission errors from transient ones (InvalidToken, AuthRequired, etc.). At minimum, re-throw on AuthRequired / InvalidToken so the user sees a real "re-login" message instead of PROFILE_NOT_FOUND.
|
Review posted successfully with 4 line-anchored findings:
|
"Profile" overloaded with the publisher's atproto profile (app.bsky.actor.profile / src/profile.ts). The command operates on com.emdashcms.experimental.package.profile — the per-package metadata record — so "package" describes the user-facing surface without the clash. Renames the command, its api module, exported types (PackageUpdateInput, UpdatePackageError, etc.), error codes (PACKAGE_NOT_FOUND, PACKAGE_INVALID), and updates the publish warning. The underlying record is still a profile record; docstrings keep that term where it refers to the lexicon shape.
ascorbic
changed the title
Drop UpdatePackageOptions.did — derive from publisher.did so the two can't disagree (Copilot). POSSIBLE_RENAME now lists every other package the publisher owns rather than singling out an arbitrary one, so a publisher with multiple plugins doesn't see a misleading rename pointer at an unrelated slug (Copilot). Flip the optional-field policy: missing from manifest = preserve existing value, matching `publish` semantics. Removing `description` or `keywords` by accident no longer wipes the published value (ask-bonk). Add atproto swapRecord optimistic concurrency: PublishingClient gains swapRecord on putRecord/unsafePutRecord; update-package threads existing.cid through and surfaces concurrent edits as STALE_RECORD instead of silently overwriting (ask-bonk). Rethrow auth/permission errors from the sibling-scan path so an expired token surfaces as itself rather than a misleading PACKAGE_NOT_FOUND (ask-bonk). Reword LEXICON_VALIDATION_FAILED to name caller-input length/grapheme caps as the likely cause rather than blaming update-package (ask-bonk). Update --json help text to match the actual output shape (Copilot). Extract redirectConsolaToStderr into cli-output.ts, shared between publish and update-package (Copilot).
What does this PR do?
Adds
emdash-plugin update-package, a CLI command that lets publishersedit an already-published package's metadata record (the registry's
com.emdashcms.experimental.package.profilerecord — distinct from thepublisher's atproto profile at
app.bsky.actor.profile) without cuttinga new release. Closes the gap from the publish flow's "Profile updates
aren't supported yet" warning, which now points users at this command.
Behaviour:
from the publisher's PDS, diffs the lexicon-controlled fields, and
prints what would change.
--yeswrites viacom.atproto.repo.putRecordand bumpslastUpdated.--jsonmirrors the publish command's machine-readableoutput shape.
license,authors,security,name,description,keywords. Identity ($type,id,slug,type)and unknown forward-compatible fields (e.g.
sectionsfrom Registry: long-form sections (description, installation, changelog, faq, security) end-to-end #1030)pass through verbatim.
published record but a different-slug package exists for the
publisher, surfaces a clear
POSSIBLE_RENAMEwarning so a manifestrename doesn't quietly orphan releases under the old slug.
Closes #1032
Parent: #1026 (Discussion #296)
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (N/A — CLI only, no admin UI changes)AI-generated code disclosure
Screenshots / test output
```
Test Files 17 passed (17)
Tests 282 passed (282)
```
18 new test cases cover dry-run diff detection, apply-with-write
semantics, unknown-field preservation, `INVALID_INPUT` refusals for
empty arrays / contacts with no url-or-email, `POSSIBLE_RENAME`
detection, `SLUG_MISMATCH` for hand-edited records, name-only diffs,
and keywords array-order changes.