Skip to content

Cli versioned resource plugins#332

Merged
daniel-de-vera merged 10 commits into
mainfrom
cli-versioned-resource-plugins
May 27, 2026
Merged

Cli versioned resource plugins#332
daniel-de-vera merged 10 commits into
mainfrom
cli-versioned-resource-plugins

Conversation

@scott-cotton
Copy link
Copy Markdown
Member

@scott-cotton scott-cotton commented May 13, 2026

Depends on:

This PR addresses signadot/community#116

What changes for users

Plugin references take the form name[@<semver>] everywhere a user types or sees a plugin reference:

signadot rp apply -f my-plugin.yaml       # version comes from the YAML name: field
signadot rp list                          # NAME column renders foo@1.2.0 (or bare foo at default)
signadot rp versions my-plugin            # new: every published version
signadot rp get my-plugin                 # latest
signadot rp get my-plugin@1.2.0           # specific version
signadot rp delete my-plugin@1.2.0

In YAML, the version travels with name: as an @<semver> suffix — there is no separate version: field. A bare name publishes the plugin at the default version (0.0.0):

name: my-plugin@1.2.0
spec: { ... }

A stray top-level version: field is silently ignored, like any other unknown top-level field.

What get / list look like

For plugins published at the default version, get -o yaml returns the bare name: my-plugin (no @0.0.0 suffix), matching the pre-versioning CLI surface byte-for-byte. For any concretely-versioned plugin, the response carries the version in the name field as a single token. get -o yaml round-trips back into apply -f without renaming.

Server contract

  • Identity on the wire is a single name field; the version is the optional @semver suffix.
  • Versions are immutable: re-applying an existing (name, version) — including the implicit default 0.0.0 — returns 409. The CLI maps that to resource plugin %q version %q already exists; versions are immutable — bump the version field to publish a new revision, and uses default version (0.0.0) when the user never set one.
  • get and delete accept ?version=<v> or omit/?version=latest for the highest semver.
  • ListResourcePlugins returns latest-only; per-plugin version listing is the new versions subcommand.

Test plan

  • go test ./... passes locally.
  • Loader unit tests cover bare name, @version suffix, and the now-ignored top-level version: field.
  • --help for resourceplugin, get, versions renders the new NAME[@VERSION] usage.
  • e2e: apply a plugin with bare name, GET it back, see name: my-plugin.
  • e2e: apply a plugin with name: foo@1.2.0, list, get by version, list versions, delete by version.
  • e2e: apply same plugin twice with same version → see the 409 message.

Before merging

  • Re-pin go-sdk to the merged tag from go-sdk#80 (currently on the versioned-resource-plugins branch tip).

scott-cotton and others added 2 commits May 13, 2026 18:13
Adapt the CLI to the versioned-resource-plugins API. Plugin references take
the form name[@<semver>] everywhere the user types or sees a plugin:

- YAML loader splits the top-level name on '@' and sets both Name and Version
  on the request body.
- apply detects 409 (ApplyResourcePluginConflict) and reports versions are
  immutable, prompting the user to bump the version field.
- get / delete take NAME[@Version] positionally; concrete versions flow into
  the new WithVersion param. Omitted version resolves to latest on the
  server.
- delete -f file also uses the version from the loaded YAML.
- Table NAME column and details Name: line render the joined name@version
  form so the output grep-pastes back into other commands.
- New `versions` subcommand calls ListResourcePluginVersions and reuses the
  same table printer; server returns versions highest-semver-first.
- go-sdk pinned to a versioned-resource-plugins commit (pre-merge); will
  re-pin to the merged tag before opening the PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review findings on the prior commit:

- Accept the version as either a `name: foo@1.2.0` suffix or a separate
  top-level `version:` field, since the wire model exposes them
  separately and users coming from the SDK/Notion design will reach
  for the top-level form. Reject if the version is supplied in both
  places (even when the values agree) so the spec has a single source
  of truth.
- Type-check the top-level `version:` so a YAML float (e.g. `version:
  1.0`) fails loudly rather than being silently dropped.
- Preserve `version` through `utils.extractName` so `delete -f file`
  picks up the top-level form too (the `@version` suffix already
  survived because it travels inside the `name` value).
- Sharpen apply's user-facing messages: the 409 conflict text now says
  "default version (0.0.0)" instead of `version ""` when the user
  never set one, and the success message always renders the effective
  `name@version` so the user sees what was actually published.

Add focused tests for the loader's name/version resolution rules.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton scott-cotton marked this pull request as ready for review May 14, 2026 08:25
@scott-cotton scott-cotton requested a review from foxish May 14, 2026 16:17
scott-cotton and others added 3 commits May 15, 2026 22:34
Pick up the go-sdk regen against the symmetric-name swagger
(sandboxes #28a6d0e9d). models.ResourcePlugin no longer carries a
separate Version field; identity is a single `name` of the form
"bareName[@semver]" both on input and on output.

- subst.go: the YAML loader still accepts a name@version suffix or a
  top-level `version:` field (CLI ergonomics), and still rejects the
  two forms together. After resolution it joins them into the wire
  shape on rp.Name; rp.Version is gone.
- apply.go: split rp.Name into a bare URL-path name and a version for
  the user-facing conflict message and the "Created …" line.
- delete.go: when --filename is used, split rp.Name to recover the
  version that flows into the ?version= query param.
- printers.go: the table NAME column and details Name line just use
  rp.Name now — the server already returns it in joined form.
- SKILL.md: clarify that the wire identity is the combined `name`
  field; the top-level `version:` is CLI YAML sugar only and never
  appears in server-returned JSON/YAML.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that the wire identity is a single name field, supporting two YAML forms for the same thing widens the input space beyond what get -o yaml produces. The CLI only accepts the suffix form (name: foo@1.2.0) — a top-level version: field is silently ignored, like any other unknown top-level field.

- Drop the optionalStringField helper and the XOR check from the YAML loader; rp.Name now comes straight from the YAML name: value (already the wire form).
- Trim the related test cases; keep the bare-name and name-suffix ones.
- Drop version from utils.extractName's keep-list for delete-by-file — name alone carries the identity now.
- SKILL.md: drop the 'Equivalent form' block.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pick up the server-side bare-default change (sandboxes #b1faf65f5) and
mirror its display rule on the CLI side. The post-success message after
'apply' for a plugin published at the default version now reads
'Created resource plugin foo', matching the bare 'name: foo' the server
will return on subsequent GETs and the pre-versioning CLI output. The
@semver suffix continues to appear for any concretely-versioned plugin.

Drop the effectiveNameRef helper that explicitly rendered '@0.0.0';
formatNameRef (bare when version is empty, joined otherwise) is the
single display rule everywhere now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton
Copy link
Copy Markdown
Member Author

The latest push (3 commits, 23bc855..7577984) adapts this PR to the symmetric-name wire shape introduced in signadot/signadot#7007. Summary of what changed from the previous state of the branch:

  • name@version is the wire form, end to end. The previous shape carried name and version as two body fields on PUT and as two response fields on GET. Now the body and the response both use a single name field of the form bareName[@semver]. get -o yaml pastes straight back into apply -f.
  • Top-level YAML version: is dropped. The earlier review feedback had us accept either name: foo@1.2.0 or name: foo + version: 1.2.0. Once the wire identity collapsed to a single token, supporting two YAML forms for the same thing only widened the input space beyond what get -o yaml produces. The loader now reads name: as the wire form directly; a stray version: key is silently ignored, like any other unknown top-level field. Users who hand-wrote the two-field form should fold the version into the name suffix.
  • Default-version plugins display bare. A plugin published at 0.0.0 (no @semver in apply) is shown as name: my-plugin everywhere — the CLI's apply success message, get/list/versions output, and the JSON body the server returns. Concrete versions still render name: my-plugin@1.2.0. This preserves byte-for-byte compatibility with the pre-versioning CLI surface for the typical "I don't care about versioning" user.

The CLI commits in this push correspond 1:1 to the three layers of change:

  1. 23bc855 resourceplugin: adapt to combined name wire field — go-sdk bump; split req.Name for the URL-path arg in apply/delete; use rp.Name directly in printers (it's already joined on the wire).
  2. 2577541 resourceplugin: drop top-level YAML version field — remove optionalStringField and the XOR check from subst.go; trim related test cases; drop the version key from utils.extractName's keep-list for delete -f.
  3. 7577984 resourceplugin: render default-version plugins as bare name — go-sdk re-bump for the bare-default server change; drop effectiveNameRef in favor of formatNameRef (which already produces bare when version is empty).

Net: this PR no longer adds any local string-splitting "cleverness" to paper over a server asymmetry — the server now exposes the same shape the CLI always wanted.

@davixcky davixcky self-requested a review May 19, 2026 14:04
@davixcky
Copy link
Copy Markdown
Contributor

davixcky commented May 19, 2026

NAME        CREATED                       STATUS
foo@2.0.0   2026-05-19T14:50:09.656114Z   0 resources
foo@1.5.0   2026-05-19T14:53:39.50125Z    0 resources
foo@1.1.0   2026-05-19T14:50:09.055472Z   0 resources
foo@1.0.0   2026-05-19T14:50:07.937688Z   0 resources

IMO this is verbose, we are repeating the same word over an over (foo)

Also the CREATED time doesn't seem to be aligned with others, for example sb list

Match sandbox/routegroup/planrunnergroup, which all render the CREATED
column via timeago ("3 minutes ago") rather than the raw ISO timestamp.
Detail view is untouched — it already uses utils.FormatTimestamp.

This is a pre-existing inconsistency (predates versioning), but folded
into this PR since the user-visible `rp list` surface is being touched
anyway.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton
Copy link
Copy Markdown
Member Author

NAME        CREATED                       STATUS
foo@2.0.0   2026-05-19T14:50:09.656114Z   0 resources
foo@1.5.0   2026-05-19T14:53:39.50125Z    0 resources
foo@1.1.0   2026-05-19T14:50:09.055472Z   0 resources
foo@1.0.0   2026-05-19T14:50:07.937688Z   0 resources

IMO this is verbose, we are repeating the same word over an over (foo)

Yes but IMO it should be verbose because this is tabular output and hence both expected and easier to automate, which in my mind outweighs the repetition. wdyt in light of this?

Also the CREATED time doesn't seem to be aligned with others, for example sb list

This PR did not change the time, it was pre-existing problem. But, I agree it would be better to be aligned, so I've added that in 5577eba.

@davixcky
Copy link
Copy Markdown
Contributor

davixcky commented May 20, 2026

Yes but IMO it should be verbose because this is tabular output and hence both expected and easier to automate, which in my mind outweighs the repetition. wdyt in light of this?

I guess if you want to automate, why not use output json instead?

The reason of saying this, is because also aligned with our current web and how others vendors also do it
image

I will suggest something like this

> signadot rp versions foo
Name: foo

TABLE

@foxish
Copy link
Copy Markdown
Member

foxish commented May 20, 2026

Maybe we can do a regular version of the command that just lists the plugin name and latest version, and an --all-versions flag that can list the full expanded table (for using with CLI automation)?

@davixcky
Copy link
Copy Markdown
Contributor

Can be a flag, but not --all-version cause we are already showing all the versions. I think is more --full-version

@foxish
Copy link
Copy Markdown
Member

foxish commented May 20, 2026

Can be a flag, but not --all-version cause we are already showing all the versions. I think is more --full-version

I was saying by default when listed, show just the latest version (1 entry per plugin), and then --all-versions would be the one showing all versions.

@scott-cotton
Copy link
Copy Markdown
Member Author

I guess if you want to automate, why not use output json instead?

To me, it is not a question of whether there exists an alternative which is more expensive and takes more care to craft.

There is a long long history of tools outputting tables and using things like awk, cut, paste, sed, etc. IMO by far the gold standard for CLI tools outputting to stdout is for piping. And the example you give is a photo of a UI, not tabular text output for piping, it doesn't really address the issue IMO.

It is worth noting that one could separate the version in a different column. The reason it's not like that now is to make the presentation of name[@Version] uniform everywhere. But, in any output which is a table/matrix, repetition in some columns at least is expected IMO.

I am ok with @foxish's suggestion to only show the latest version unless a flag is provided, which would be another way to reduce the repetition across columns. I guess it might make it less ergonomic to navigate the version history, requiring a separate call to the versions subcommand, but the tradeoff seems reasonable to me.

@foxish
Copy link
Copy Markdown
Member

foxish commented May 20, 2026

It is worth noting that one could separate the version in a different column.

I think this is a good idea also, to have the xyz@vN identifier being shorthand but allowing the long form split representation for outputs. I don't feel strongly about this, but if it is not too brittle to split, it could be nicer.

Address PR review feedback that the combined `name@version` token, while
fine as a shorthand for input, is redundant when displayed row-by-row.

- `rp list`: split into NAME | VERSION | CREATED | STATUS. A plugin
  published at the default version (bare wire name) renders as
  VERSION=0.0.0, explicit rather than blank, so the column has no
  ambiguous cells.

- `rp versions NAME`: drop the NAME column entirely. The caller passed
  the name as an argument; repeating it on every row was noise. Output
  is still a plain table — no header line above it — so the shape stays
  uniform across subcommands. New row type and printer function avoid
  conditional column rendering.

JSON / YAML output is unchanged (both still emit the combined `name`
field exactly as the server returns it).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton
Copy link
Copy Markdown
Member Author

@davixcky @foxish

Please see last commit regarding the listing /versions.

Upon doing that, I realised I (or we) had conflated the rp list with the rp versions list through the discussion. In fact rp list was already latest only, and the repeated text example was from the versions command.

Last commit

  • splits version into its own column in both rp list and rp versions
  • removes the name from the output of rp versions

wdyt?

…rnings)

Address findings #3, #4, #5, #6 from the versioned-resource-plugins UX pass.

#4 — apply immutable-version error: the previous code matched on
*ApplyResourcePluginConflict, but the go-sdk's transport middleware
(FixAPIErrors) intercepts every 4xx/5xx response before the per-endpoint
typed reader runs, so that type is never in the error chain and the
catch-and-rewrite was dead code. Users saw the generic '409 Conflict:
plugin version already exists'. Switch to errors.As against
*transport.APIError + Code==409 (the same pattern used in planexec/logs).

Also #11 — branch the message text on the bare-name vs. concrete-version
case: a bare-name author has no '@semver' suffix to bump, so the
'bump the version field' advice was a dead end; now they get a concrete
'add an @<semver> suffix to publish a new revision (e.g. my-plugin@0.0.1)'.

#5 — apply success line: previously stayed silent on the version for
bare-name authors ('Created resource plugin my-plugin') while
explicit-version authors saw '@0.0.1'. The asymmetry hid the versioning
rule from exactly the population that most needed to learn it. Always
echo the explicit version on the apply success line (rendering '@0.0.0'
for bare publishes). The list/table/get-yaml output continues to bare
the default version for backward compatibility with pre-versioning
clients — this one message is explicit on purpose, as a teaching moment.

#3 — bare-form delete: previously silently removed only the latest
version with no indication that others remained, a footgun for users
who assumed 'delete my-plugin' meant 'delete the plugin'. Resolve the
latest version explicitly before the delete (one List call), then
report which version was removed and how many remain, e.g.
'Deleted resource plugin my-plugin@0.0.1.
 1 other version(s) remain (use 'signadot resourceplugin versions my-plugin' to list).'

#6 — delete-in-use: previously surfaced the server's generic
'400 Bad Request: plugin is currently in use' with no hint about which
sandbox(es) were holding the version. Enrich client-side: on that
specific 400 we GET the plugin and surface Status.Resources sandbox
names: 'resource plugin foo@1.2.0 is still referenced by sandbox(es): a, b'.
Falls back to the original error if the enrichment can't be done.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton
Copy link
Copy Markdown
Member Author

Pushed 7343d25 covering the four hot-path CLI findings from the UX pass:

#4 apply immutable-version error wording. The previous errors.As(err, *ApplyResourcePluginConflict) was dead code: the go-sdk's transport middleware FixAPIErrors intercepts every 4xx/5xx response before the per-endpoint typed reader runs, so that type is never in the error chain. Users saw the raw 409 Conflict: plugin version already exists. Switched to matching *transport.APIError + Code == 409 (same pattern as internal/command/planexec/logs.go:164).

Folded in finding #11 while we're here: bare-name authors got the dead-end advice "bump the version field" they didn't have. The branch now emits a concrete add an @<semver> suffix to publish a new revision (e.g. my-plugin@0.0.1) for that case.

#5 apply success silent on version for bare-name authors. Was emitting Created resource plugin my-plugin for bare publishes vs Created resource plugin my-plugin@0.0.1 for explicit-version publishes. The asymmetry hid the versioning rule from exactly the population that most needs to see it. Now always echoes @version on the success line (@0.0.0 for bare publishes), as a teaching moment. The list/table/get-yaml display still bares the default version for backward compatibility — this single line is explicit on purpose.

#3 bare-form delete silently removes only the latest. Now resolves the latest version up-front (one List call), deletes it explicitly, then reports both what was removed and how many versions remain:

Deleted resource plugin my-plugin@0.0.1.
1 other version(s) remain (use 'signadot resourceplugin versions my-plugin' to list).

Small race remains (someone publishes a newer version between the List and the Delete); we still delete the version we resolved, which matches the user's mental model of "delete whatever was latest when I asked".

#6 delete-in-use doesn't name holding sandboxes. Server's generic 400 Bad Request: plugin is currently in use is now enriched client-side: on that specific 400 we GET the plugin and surface the sandbox names from Status.Resources:

resource plugin foo@1.2.0 is still referenced by sandbox(es): customer-sb, orders-sb

Falls back to the original error if the enrichment GET fails.

go test ./... green. SKILL.md updated to reflect the new error wording and delete warnings.

scott-cotton and others added 2 commits May 22, 2026 00:13
…lugin

Address findings #8 and #13 from the versioned-resource-plugins UX pass.

#8 — versions --help was one short line with no example. Expand the
short / long / example block to spell out: that the NAME argument is
the bare name (no @semver suffix); that 'get NAME@VERSION' is the way
to fetch a single version's spec; and that the output is sorted
highest-semver first.

#13 — versions <nonexistent> was returning exit 0 with empty rows,
because the server replies 200 with an empty payload for an unknown
name (it has no equivalent of the 404 'get' returns). A resource
plugin always has at least one version once it exists — publishing
creates the (name, version) row — so an empty list reliably means
'no such plugin'. Surface that as an error instead of silently
exiting 0, matching the behavior of 'resourceplugin get <nonexistent>'.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pick up the server-side ?version=all support from
signadot/signadot#7025 (go-sdk regen on the
versioned-resource-plugins branch) and expose it via two changes to
'rp list':

- New --all-versions / -A flag. When set, the CLI passes
  ?version=all to the list endpoint and the table renders one row
  per (name, version), sorted by name then semver-descending. The
  existing NAME / VERSION column split (c67307a) makes the expanded
  output naturally scannable.

- When --all-versions is NOT set (i.e. the default latest-only
  list), a cheap second list-with-?version=all is issued to count
  how many bare names show up more than once. If any do, a footer
  hint appears under the table:
    (3 plugins have multiple versions — pass --all-versions to
    expand, or 'resourceplugin versions NAME')
  This addresses finding #7 from the UX pass: the default list
  previously gave the user no indication that a row might be one of
  many for that name. The hint is supplementary — errors fetching
  it are silently dropped, so the exit status reflects only the
  primary list call.

The hint is skipped under JSON/YAML output (those callers can
compute it themselves from the all-versions payload if they care).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scott-cotton
Copy link
Copy Markdown
Member Author

Pushed 559ca79 picking up the go-sdk regen of versioned-resource-plugins (with the new ?version=all schema from signadot/signadot#7025).

signadot rp list --all-versions (or -A). Calls the list endpoint with ?version=all and renders one row per (name, version), sorted by name then semver-descending. The existing NAME / VERSION column split makes the expanded view naturally scannable.

Multi-version hint on the default list. Addresses finding #7. When the user runs the plain rp list (no flag), the CLI issues a cheap second list-with-?version=all and counts how many bare names show up more than once. If any do, a footer hint appears under the table:

NAME         VERSION   CREATED      STATUS
echo-test    0.0.2     5m ago       0 resources
mariadb      1.4.1     2h ago       1 resource
…
(3 plugins have multiple versions — pass --all-versions to expand, or 'resourceplugin versions NAME')

The hint is supplementary — errors fetching it are silently dropped, so the exit status reflects only the primary list call. The hint is skipped under JSON/YAML output (those callers can compute it from the all-versions payload if they need to).

go test ./... green.

Copy link
Copy Markdown
Contributor

@daniel-de-vera daniel-de-vera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@daniel-de-vera
Copy link
Copy Markdown
Contributor

@foxish, @davixcky I will go ahead and merge this one (further changes can be addressed in a separate PR).

@daniel-de-vera daniel-de-vera merged commit 2866367 into main May 27, 2026
@daniel-de-vera daniel-de-vera deleted the cli-versioned-resource-plugins branch May 27, 2026 12:34
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.

4 participants