spec: proposal for version aware cli + update flow by Cesar-M-Diaz · Pull Request #54 · nodesource/nsolid-plugin

Cesar-M-Diaz · 2026-07-30T16:14:49Z

No description provided.

coderabbitai · 2026-07-30T16:14:58Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a513be7-df2a-43ba-8c95-f73d71710438

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

_{Comment @coderabbitai help to get the list of available commands.}

dobleuber

Hey good job! Pls review my comments

dobleuber · 2026-07-30T17:08:39Z

+export type UpdateTarget =
+  | 'cli'
+  | 'claude'
+  | 'codex'
+  | 'opencode'
+  | 'antigravity'
+  | 'pi'
+
+export type UpdateOwnership =
+  | 'global-package'
+  | 'native-plugin'
+  | 'package-owned'
+  | 'fallback'
+
+export type UpdateStatus =
+  | 'current'
+  | 'update-available'
+  | 'updated'
+  | 'skipped'
+  | 'not-installed'
+  | 'unknown'
+  | 'failed'


UpdateStatus excludes newer-than-registry, UpdateResult has no latest-version field, and Task 3 requires an unsupported outcome. Since UpdateSummary exposes only these results, update --check --json cannot satisfy its required current, latest, and newer-than-registry output without consumers inferring state from messages.

dobleuber · 2026-07-30T17:09:49Z

+**Given** the CLI was installed globally by npm or pnpm
+**And** the registry reports a newer stable version
+**When** the user runs `nsolid-plugin update`
+**Then** the command displays the current version, target version, package manager, and exact planned operation
+**And** asks for confirmation in an interactive terminal
+**And** after confirmation invokes the detected package manager with a fixed argument array to install `nsolid-plugin@<latest>`


Plain update specifies only newer-registry and equal-version cases, while check mode explicitly recognizes newer-than-registry. A locally newer CLI (for example, after a registry rollback) has no required no-mutation outcome and could be replaced by @latest. Require an explicit skip/error result unless a deliberate downgrade flag is supplied.

dobleuber · 2026-07-30T17:11:14Z

+`packages/core/src/update/inventory.ts`
+
+- Reuses harness adapters and tracking readers to classify each harness as native, fallback, package-owned, or not installed.
+- Reads the running CLI/package metadata.
+- Adds optional version discovery without changing existing installation detection contracts.


Existing detection accepts alternate marketplace IDs and local Pi package sources, but the proposed commands hard-code nsolid-plugin@nodesource, nodesource, and npm:nsolid-pi-plugin. An installation detected from another marketplace or a local Pi source would be reported as installed while the updater acts on a different source. Carry a validated installed ID/source through the plan or reject it as unsupported.

Also review specs/update-flow/spec.md:123-126, :140-145

dobleuber · 2026-07-30T17:12:29Z

+`packages/core/src/update/inventory.ts`
+
+- Reuses harness adapters and tracking readers to classify each harness as native, fallback, package-owned, or not installed.
+- Reads the running CLI/package metadata.
+- Adds optional version discovery without changing existing installation detection contracts.


The current installer can leave fallback artifacts for a harness that also has a native plugin, but inventory classifies each harness as exactly one ownership. --all promises every detected installation, so choosing native leaves fallback artifacts stale and choosing fallback leaves the native plugin stale. Model these as distinct installation targets or define a safe precedence/reconciliation rule.

dobleuber · 2026-07-30T17:14:15Z

+`packages/core/src/update/antigravity-transaction.ts`
+
+- Resolves only known NodeSource staged plugin paths.
+- Creates a temporary backup before replacement.
+- Validates the newly staged root by checking `plugin.json`, `bundle.json`, and canonical skill presence.
+- Restores the backup if reinstall or validation fails.


The transaction backs up only the staged plugin directory. Existing uninstall handling also removes the N|Solid entry from ~/.gemini/config/import_manifest.json; after a failed reinstall, restoring just the directory can leave the plugin unregistered. Snapshot, restore, and validate the manifest/source registration as part of rollback.

Also review specs/update-flow/spec.md:183-196

JungMinu · 2026-07-31T00:27:26Z

+export interface VersionInfo {
+  current?: string
+  latest?: string
+  status: 'current' | 'update-available' | 'newer-than-registry' | 'unknown'


VersionInfo.status includes "newer-than-registry" but the UpdateStatus type (line 158) does not — it only has "current" | "update-available" | "updated" | "skipped" | "not-installed" | "unknown" | "failed". Either add "newer-than-registry" to UpdateStatus or replace it with one of the existing members. The same gap appears in the Update Flow spec at line 35.

JungMinu · 2026-07-31T00:28:38Z

+  verbose?: boolean
+  noColor?: boolean
+  commandRunner?: CommandRunner
+  confirm?: UpdateConfirmation


UpdateConfirmation is referenced here but never defined in the Interfaces and Contracts section. Define it above or replace it with the resolved inline type so implementers have a single source of truth.

JungMinu · 2026-07-31T00:28:52Z

+}
+
+export interface UpdateOptions {
+  harness?: HarnessType


HarnessType is referenced here but not defined or imported in the Interfaces and Contracts section. If it is imported from an existing module (e.g., packages/core/src/harnesses/types.ts), include the import statement so readers know which type contract to implement against.

- Add explicit version, source, installation, and rollback contracts - Handle newer-than-registry and unsupported update outcomes - Preserve native marketplace identities and separate fallback installations - Reject non-canonical Pi sources without mutation - Restore Antigravity staged files and import manifest on rollback - Align proposal, update-flow spec, and implementation tasks

dobleuber

Pls review my comments

dobleuber · 2026-08-03T15:53:06Z

+### Requirement: Plugin payload changes require an update-visible version
+
+Release checking SHALL reject payload changes whose explicit bundle version still matches the most recent release tag.
+
+Release mode SHALL be activated only by `pnpm release:check --release`. For this comparison, “plugin payload files” is the following explicit allowlist:
+
+- `skills/**`
+- `bundle.json`
+- `.claude-plugin/marketplace.json`
+- `.claude-plugin/plugin.json`
+- `.agents/plugins/marketplace.json`
+- `.codex-plugin/plugin.json`
+- `.claude-mcp.json`
+- `.mcp.json`
+- `plugin.json`
+- `mcp_config.json`
+- `scripts/mcp-wrapper.js`
+
+#### Scenario: Skill changes retain the previous release version
+
+**Given** committed plugin payload files differ from the most recent release tag
+**And** `bundle.json.version` still equals the version represented by that tag
+**When** the maintainer runs `pnpm release:check --release`
+**Then** it fails with guidance to prepare a new semantic version
+**And** prevents a release that version-keyed harness caches would treat as unchanged


The release payload allowlist excludes packages/core/src/** and packages/pi-plugin/index.js, although these are published runtime inputs. A runtime or security fix can retain the old version and pass release:check --release, then collide with npm’s immutable version policy. Check publishable package inputs or generated package contents too.

dobleuber · 2026-08-03T15:54:06Z

+export type MarketplaceVersionSource =
+  | {
+      kind: 'git'
+      repository: string
+      revision?: string
+      manifestPath: string
+    }


also at 425-435

Git revision remains optional, Antigravity installs an unpinned repository URL, and npm/pnpm commands pin only a semantic version without registry, tarball, or integrity identity. The bytes executed can differ from those inspected during planning. Require immutable Git commits/refs and package registry/integrity binding with post-update verification.

dobleuber · 2026-08-03T15:55:44Z

+export type UpdateSource =
+  | { kind: 'none' }
+  | { kind: 'global-package'; packageManager: 'npm' | 'pnpm'; packageName: 'nsolid-plugin' }
+  | {
+      kind: 'claude-marketplace'
+      pluginId: string
+      marketplace: string
+      scope: ClaudePluginScope
+      versionSource: MarketplaceVersionSource
+    }
+  | {
+      kind: 'codex-marketplace'
+      pluginId: string
+      marketplace: string
+      versionSource: MarketplaceVersionSource
+    }
+  | ({
+      kind: 'pi-package'
+      spec: 'npm:nsolid-pi-plugin'
+    } & PiPackageLocation)
+  | {
+      kind: 'unsupported'
+      source: string
+      reason: 'local' | 'git' | 'pinned' | 'ambiguous' | 'conflicting' | 'untracked' | 'unsupported-manager'
+    }
+  | { kind: 'antigravity-git'; url: 'https://github.com/NodeSource/nsolid-plugin.git'; layout: AntigravityLayout }
+  | { kind: 'fallback'; bundleVersion?: string; executor?: FallbackPackageExecutor }


The refresh command receives only --harness; the fallback source does not carry an exact installation/path set. Existing tracking permits shared skill paths and records no field-level MCP ownership, so reconciliation can affect a sibling installation or overwrite a user-modified same-name MCP entry. Pass and validate the installation identity and owned paths, including shared-path/reference rules.

dobleuber · 2026-08-03T15:56:31Z

+**And** the output identifies the conflicting destination without exposing its contents
+
+#### Scenario: OpenCode or fallback refresh fails
+
+**Given** a tracked direct/fallback installation was backed up
+**When** exact-package execution, skill reconciliation, MCP merge, tracking update, or post-install validation fails
+**Then** the updater restores the prior tracked skill directories, affected configuration, and tracking state
+**And** restores stale tracked assets removed during reconciliation
+**And** preserves unrelated OpenCode/fallback artifacts
+**And** reports whether rollback succeeded
+**And** exits non-zero


Rollback is delegated to the exact-package child process. If npm exec or pnpm dlx times out, crashes, or is killed after mutation, that child cannot restore state, and no parent-owned journal or next-run recovery is specified. The transaction needs a durable parent-owned snapshot and recovery mechanism.

dobleuber · 2026-08-03T15:57:37Z

236-239,374-380,432-443`

The plan displays projectRoot, but CommandSpec.cwd is optional and the command only adds --approve. Since project detection is based on the current working directory, execution can target a different project unless it runs with the captured root and revalidates it.

dobleuber · 2026-08-03T15:58:24Z

+### Requirement: Plugin payload changes require an update-visible version
+
+Release checking SHALL reject payload changes whose explicit bundle version still matches the most recent release tag.
+
+Release mode SHALL be activated only by `pnpm release:check --release`. For this comparison, “plugin payload files” is the following explicit allowlist:
+
+- `skills/**`
+- `bundle.json`
+- `.claude-plugin/marketplace.json`
+- `.claude-plugin/plugin.json`
+- `.agents/plugins/marketplace.json`
+- `.codex-plugin/plugin.json`
+- `.claude-mcp.json`
+- `.mcp.json`
+- `plugin.json`
+- `mcp_config.json`
+- `scripts/mcp-wrapper.js`
+
+#### Scenario: Skill changes retain the previous release version
+
+**Given** committed plugin payload files differ from the most recent release tag
+**And** `bundle.json.version` still equals the version represented by that tag
+**When** the maintainer runs `pnpm release:check --release`
+**Then** it fails with guidance to prepare a new semantic version
+**And** prevents a release that version-keyed harness caches would treat as unchanged


“Most recent semantic-version tag” does not define v prefixes, annotated tags, ancestry, shallow clones, local versus remote tags, or missing/malformed-tag behavior. The task requests tests for malformed and missing tags but does not specify the expected result, allowing inconsistent release guards.

dobleuber · 2026-08-03T15:59:40Z

+- A completed check whose result is `current`, `update-available`, `newer-than-registry`, `unsupported`, or evidence-only `unknown` is informational and exits zero. A timeout, invalid response, or other operational lookup/validation failure is `failed` and remains non-zero.
+- A mutating update with `newer-than-registry` performs no downgrade and exits zero; a mutating `unsupported` result exits non-zero with manual guidance.
+- A declined plan produces `skipped` results and exits zero.


Exit behavior is defined for checks, unsupported targets, newer versions, and declined updates, but not for not-installed, mutating unknown, or an empty --all. Automation cannot reliably distinguish a successful no-op from a failed update.

dobleuber

Good job!

spec: proposal for version aware cli + update flow

052fd91

Cesar-M-Diaz requested review from dobleuber and mafesernaarboleda July 30, 2026 16:14

dobleuber reviewed Jul 30, 2026

View reviewed changes

JungMinu reviewed Jul 31, 2026

View reviewed changes

JungMinu approved these changes Jul 31, 2026

View reviewed changes

docs: finalize update flow specification

abb2132

coderabbitai Bot approved these changes Aug 3, 2026

View reviewed changes

dobleuber reviewed Aug 3, 2026

View reviewed changes

spec: address update flow review gaps

5812b0f

dobleuber approved these changes Aug 3, 2026

View reviewed changes

Cesar-M-Diaz merged commit 5812b0f into cesar/update-flow Aug 4, 2026
6 checks passed

Uh oh!

Conversation

Cesar-M-Diaz commented Jul 30, 2026

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

dobleuber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dobleuber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dobleuber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading