Version skills too, numbered by the server rather than by their author

repo.KindSkill existed with nothing writing it. Migration 000010 says "agents
and skills version identically", the table has always accepted kind='skill',
and no path on either side ever recorded one — so an edit to a skill left no
record of what it used to say. Agents name their skills and the runtime refuses
to load one whose skill is missing, so a skill changing under a pinned agent is
the same class of problem the last two commits fixed, one layer down.

Skills are numbered differently, and not by preference. An agent's frontmatter
carries `version:`, so its author decides when a change is a new version and can
be refused for rewriting an old one. definition.Skill has no such field, the
skill_definitions table has no such column, and the vocabulary is active |
inactive rather than draft | published. Giving skills an authored version would
mean a migration, a parser change on BOTH sides of the conformance test in
internal/definition — which replays a capture of the real frontend module graph
— and an edit to all 23 shipped skills. That is a feature, not this fix.

So the server assigns it: one after whatever was last published. This is not an
invention. repo.VersionsRepo.LatestVersion was written for exactly this and
says so — "the next published version has to follow what was actually published
rather than what somebody wrote in the frontmatter" — and had no callers
outside its own test.

Because the author never names a version, there is nothing to refuse: an edit
is always a new version. What needs care instead is the opposite — a save that
changed nothing must NOT be one, or every deploy would add a version to all 23
skills and the number would stop meaning anything. Each publish is compared
against the last recorded copy first. Inactive skills are not recorded at all;
inactive is this vocabulary's draft.

Verified against a live stack:

  - first import over 23 unversioned skills: 23 skill version(s) recorded
  - second import, files unchanged: 0 recorded, total still 23
  - one skill edited: 1 recorded, that skill at v1, v2; v1 still holds the
    original text and v2 the edit

The test fails without the change — "after create: 0 version(s), want 1" — and
covers the three behaviours that matter: an edit versions, an identical save
does not, and an inactive skill is not recorded.

Both races noted on the agent path apply here as well: two simultaneous edits
can compute the same next number, and the loser's snapshot is dropped rather
than failing the author's save.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJvibeSc1JYXjatankqM1g

go-api/cmd/importagents/main.go

@@ -7,11 +7,16 @@
 //
 // What it does NOT do, deliberately:
 //
-//   - It does not create versions. §3 says specs are immutable once published
+//   - It does not validate every spec against a running model. Parsing and
-//     and editing publishes a new version; this re-publishes in place, which is
+//     dependency checks happen here; behaviour is what the eval suites are for.
-//     right for a curated set shipped with the deployment and wrong for
+//
-//     authored ones. Version immutability is Phase 3's, and this command is the
+// It DOES record versions, in the same transaction as the definitions. §3 says
-//     thing that makes Phase 3 worth doing rather than a substitute for it.
+// a published version is immutable and editing publishes a new one, and a
+// command that re-published in place was the one path that ignored that: the
+// live row took the new text and nothing recorded what the old one said, so
+// every deploy quietly rewrote v1. A spec whose content has changed without
+// its `version:` being raised is now refused, and refused for the whole set —
+// see the note above the import loop.
 //   - It does not validate tool names against the registry. §3 wants an unknown
 //     tool to fail at publish; today the runtime records and drops one. The
 //     check is cheap to add and belongs here — see the note in run().
@@ -31,9 +36,12 @@ import (
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
 
+	"github.com/krow/krow-backend/go-api/internal/authctx"
 	"github.com/krow/krow-backend/go-api/internal/config"
 	"github.com/krow/krow-backend/go-api/internal/db"
 	"github.com/krow/krow-backend/go-api/internal/definition"
+	"github.com/krow/krow-backend/go-api/internal/domain"
+	"github.com/krow/krow-backend/go-api/internal/repo"
 )
 
 func main() {
@@ -156,15 +164,41 @@ func run(dir, skillDir, orgSlug string, dryRun bool, timeout time.Duration) erro
 	// briefly unloadable, and inside one transaction that is invisible — but
 	// ordering them correctly costs nothing and means a future non-transactional
 	// path is not silently broken.
-	skillsWritten := 0
+	// Versions are recorded through the same transaction, so the history and
+	// the definition it describes cannot disagree: either both land or neither
+	// does.
+	versions := repo.NewVersionsRepo(tx)
+	ident := authctx.Identity{OrgID: orgID, UserID: author}
+
+	skillsWritten, skillVersions := 0, 0
 	for _, sk := range skills {
 		if err := upsertSkill(ctx, tx, orgID, author, sk); err != nil {
 			return fmt.Errorf("%s: %w", sk.name, err)
 		}
 		skillsWritten++
+
+		// Skills are numbered by the server rather than by their author — they
+		// have no `version:` to read. See snapshotSkill in internal/service,
+		// which does the same for the authoring path.
+		recorded, err := snapshotSkill(ctx, versions, ident, sk)
+		if err != nil {
+			return fmt.Errorf("%s: record version: %w", sk.name, err)
+		}
+		if recorded {
+			skillVersions++
+		}
 	}
 
-	inserted, updated := 0, 0
+	//
+	// Snapshot is what refuses a spec that changed without raising its
+	// `version:`. Every such spec is collected rather than the first one
+	// returned, for the same reason the parse errors above are — an operator
+	// who forgot to bump three files should see three. Collecting is safe here
+	// because that refusal comes from comparing a row this code read, not from
+	// a failed statement: the INSERT is ON CONFLICT DO NOTHING, so the
+	// transaction is still healthy and the remaining specs can be checked.
+	inserted, updated, versioned := 0, 0, 0
+	var rewrites []string
 	for _, s := range specs {
 		wasNew, err := upsert(ctx, tx, orgID, author, s)
 		if err != nil {
@@ -175,13 +209,42 @@ func run(dir, skillDir, orgSlug string, dryRun bool, timeout time.Duration) erro
 		} else {
 			updated++
 		}
+
+		err = versions.Snapshot(ctx, ident, repo.SnapshotInput{
+			Kind:         repo.KindAgent,
+			DefinitionID: s.parsed.ID,
+			Version:      s.parsed.Version,
+			Markdown:     s.raw,
+			Name:         s.parsed.Name,
+			Description:  s.parsed.Description,
+			Pages:        s.parsed.Pages,
+		})
+		var apiErr *domain.Error
+		switch {
+		case err == nil:
+			versioned++
+		case errors.As(err, &apiErr) && apiErr.Code == "conflict":
+			rewrites = append(rewrites, fmt.Sprintf("  %s: %s", s.name, apiErr.Message))
+		default:
+			return fmt.Errorf("%s: record version: %w", s.name, err)
+		}
 	}
+
+	if len(rewrites) > 0 {
+		return fmt.Errorf(
+			"%d spec(s) would rewrite a version that is already published:\n%s\n\n"+
+				"Nothing was written. Raise `version:` in the frontmatter of each, or "+
+				"restore the published text.",
+			len(rewrites), strings.Join(rewrites, "\n"))
+	}
+
 	if err := tx.Commit(ctx); err != nil {
 		return fmt.Errorf("commit: %w", err)
 	}
 
-	fmt.Printf("\n%d agent(s) published, %d updated, %d skill(s) written, into %s\n",
+	fmt.Printf("\n%d agent(s) published, %d updated, %d agent version(s) recorded, "+
-		inserted, updated, skillsWritten, orgSlug)
+		"%d skill(s) written, %d skill version(s) recorded, into %s\n",
+		inserted, updated, versioned, skillsWritten, skillVersions, orgSlug)
 	return nil
 }
 
@@ -384,3 +447,40 @@ func upsertSkill(ctx context.Context, tx pgx.Tx, orgID, author string, sk skillS
 		sk.parsed.Status, sk.parsed.Name, sk.parsed.Description, sk.parsed.Pages)
 	return err
 }
+
+// snapshotSkill records a skill version, numbered by the server.
+//
+// Skills carry no `version:` in their frontmatter, so unlike an agent there is
+// no author-supplied number to honour or to refuse. The number is one after
+// whatever was last published, and a skill whose text has not changed since
+// then is not published again — otherwise every deploy would add a version to
+// all 23 of them.
+//
+// Reports whether it wrote one, so the run can say how many changed.
+func snapshotSkill(ctx context.Context, versions *repo.VersionsRepo,
+	ident authctx.Identity, sk skillSpec) (bool, error) {
+
+	latest, err := versions.LatestVersion(ctx, ident, repo.KindSkill, sk.parsed.ID)
+	if err != nil {
+		return false, err
+	}
+	if latest > 0 {
+		stored, err := versions.Load(ctx, ident, repo.KindSkill, sk.parsed.ID, latest)
+		if err == nil && stored != nil && stored.Markdown == sk.raw {
+			return false, nil // unchanged since the last publish
+		}
+	}
+
+	if err := versions.Snapshot(ctx, ident, repo.SnapshotInput{
+		Kind:         repo.KindSkill,
+		DefinitionID: sk.parsed.ID,
+		Version:      latest + 1,
+		Markdown:     sk.raw,
+		Name:         sk.parsed.Name,
+		Description:  sk.parsed.Description,
+		Pages:        sk.parsed.Pages,
+	}); err != nil {
+		return false, err
+	}
+	return true, nil
+}

go-api/internal/httpserver/definitions_api_test.go

@@ -1,6 +1,7 @@
 package httpserver_test
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -883,3 +884,232 @@ func TestAgentCreateRejectsAnUnknownToolName(t *testing.T) {
 		t.Fatalf("a real tool was refused: status %d (%v)", ok.code, ok.body)
 	}
 }
+
+// TestPublishedVersionCannotBeRewritten covers §3: a published version is
+// immutable, and editing publishes a NEW one.
+//
+// The failure this guards against was silent rather than loud. Editing a
+// published agent without raising the frontmatter version used to answer 200:
+// the live row took the new text, the append-only history kept the old, and
+// two different definitions were both called v1. runtime.LoadAgentVersion
+// resolves a pin by returning the CURRENT definition whenever the pinned
+// number equals the current one, so a conversation "pinned to v1" then ran the
+// rewritten instructions while the audit trail showed the originals.
+func TestPublishedVersionCannotBeRewritten(t *testing.T) {
+	r := newRBAC(t)
+
+	const published = `---
+id: pinned-agent
+name: Pinned Agent
+description: published, and therefore immutable at this version
+status: published
+version: 1
+pages:
+  - candidates
+---
+
+## Instructions
+The original instructions.
+`
+
+	res := r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown":   published,
+		"visibility": "personal",
+	})
+	if res.code != http.StatusCreated {
+		t.Fatalf("create published agent: status %d (%v)", res.code, res.body)
+	}
+	id, _ := res.record(t)["id"].(string)
+	if id == "" {
+		t.Fatal("created agent has no id")
+	}
+
+	// Same version number, different body: refused.
+	rewritten := strings.Replace(published,
+		"The original instructions.", "Rewritten instructions.", 1)
+	res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+id,
+		map[string]any{"markdown": rewritten})
+	if res.code != http.StatusConflict {
+		t.Fatalf("rewriting published v1: status %d, want 409 (%v)", res.code, res.body)
+	}
+
+	// And the refusal actually protected something — the live definition is
+	// unchanged, not merely reported as unchanged.
+	res = r.as(r.admin, "GET", "/api/v1/agent-definitions/"+id, nil)
+	if res.code != http.StatusOK {
+		t.Fatalf("re-read agent: status %d (%v)", res.code, res.body)
+	}
+	md, _ := res.record(t)["markdown"].(string)
+	if !strings.Contains(md, "The original instructions.") {
+		t.Errorf("the refused edit still changed the stored definition:\n%s", md)
+	}
+	if strings.Contains(md, "Rewritten instructions.") {
+		t.Errorf("the refused edit was applied anyway:\n%s", md)
+	}
+
+	// Republishing the SAME version with the SAME content stays a no-op, so a
+	// save that changes nothing is not turned into an error.
+	res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+id,
+		map[string]any{"markdown": published})
+	if res.code != http.StatusOK {
+		t.Errorf("republishing v1 unchanged: status %d, want 200 (%v)", res.code, res.body)
+	}
+
+	// Raising the version is the supported way to publish a change.
+	bumped := strings.Replace(rewritten, "version: 1", "version: 2", 1)
+	res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+id,
+		map[string]any{"markdown": bumped})
+	if res.code != http.StatusOK {
+		t.Fatalf("publishing v2: status %d, want 200 (%v)", res.code, res.body)
+	}
+	res = r.as(r.admin, "GET", "/api/v1/agent-definitions/"+id, nil)
+	md, _ = res.record(t)["markdown"].(string)
+	if !strings.Contains(md, "Rewritten instructions.") {
+		t.Errorf("v2 did not take the new text:\n%s", md)
+	}
+
+	// A draft carries no such promise: it is not published, so it may be
+	// rewritten in place as often as its author likes.
+	const draft = `---
+id: draft-agent
+name: Draft Agent
+description: still a draft
+status: draft
+version: 1
+pages:
+  - candidates
+---
+
+## Instructions
+First draft.
+`
+	res = r.as(r.admin, "POST", "/api/v1/agent-definitions", map[string]any{
+		"markdown": draft, "visibility": "personal",
+	})
+	if res.code != http.StatusCreated {
+		t.Fatalf("create draft: status %d (%v)", res.code, res.body)
+	}
+	draftID, _ := res.record(t)["id"].(string)
+	res = r.as(r.admin, "PATCH", "/api/v1/agent-definitions/"+draftID,
+		map[string]any{"markdown": strings.Replace(draft, "First draft.", "Second draft.", 1)})
+	if res.code != http.StatusOK {
+		t.Errorf("rewriting a draft at the same version: status %d, want 200 (%v)", res.code, res.body)
+	}
+}
+
+// TestSkillVersionsAreRecordedAndServerNumbered covers the skill half of §3.
+//
+// Skills carry no `version:` in their frontmatter, so unlike an agent there is
+// no author-supplied number to honour and nothing to refuse: the server takes
+// the next one after whatever was last published. Before this, skills were
+// never versioned at all — repo.KindSkill existed with nothing writing it, and
+// an edit to a skill left no record of what it used to say.
+func TestSkillVersionsAreRecordedAndServerNumbered(t *testing.T) {
+	r := newRBAC(t)
+	ctx := context.Background()
+
+	count := func(definitionID string) int {
+		t.Helper()
+		var n int
+		if err := r.h.Pool.QueryRow(ctx,
+			`SELECT count(*) FROM definition_versions
+			  WHERE org_id = $1::uuid AND kind = 'skill' AND definition_id = $2`,
+			r.orgID, definitionID).Scan(&n); err != nil {
+			t.Fatalf("count skill versions: %v", err)
+		}
+		return n
+	}
+	stored := func(definitionID string, version int) string {
+		t.Helper()
+		var md string
+		if err := r.h.Pool.QueryRow(ctx,
+			`SELECT markdown FROM definition_versions
+			  WHERE org_id = $1::uuid AND kind = 'skill'
+			    AND definition_id = $2 AND version = $3`,
+			r.orgID, definitionID, version).Scan(&md); err != nil {
+			t.Fatalf("read skill v%d: %v", version, err)
+		}
+		return md
+	}
+
+	const first = `---
+id: versioned-skill
+name: Versioned Skill
+description: a skill that should acquire a history
+status: active
+pages:
+  - candidates
+---
+
+# Versioned Skill
+The first body.
+`
+
+	res := r.as(r.admin, "POST", "/api/v1/skill-definitions", map[string]any{
+		"markdown":   first,
+		"visibility": "personal",
+	})
+	if res.code != http.StatusCreated {
+		t.Fatalf("create skill: status %d (%v)", res.code, res.body)
+	}
+	id, _ := res.record(t)["id"].(string)
+	if got := count("versioned-skill"); got != 1 {
+		t.Fatalf("after create: %d version(s), want 1", got)
+	}
+
+	// An edit is always a new version — the author names no number, so there
+	// is nothing to rewrite and nothing to refuse.
+	second := strings.Replace(first, "The first body.", "The second body.", 1)
+	res = r.as(r.admin, "PATCH", "/api/v1/skill-definitions/"+id,
+		map[string]any{"markdown": second})
+	if res.code != http.StatusOK {
+		t.Fatalf("edit skill: status %d (%v)", res.code, res.body)
+	}
+	if got := count("versioned-skill"); got != 2 {
+		t.Fatalf("after an edit: %d version(s), want 2", got)
+	}
+
+	// v1 still says what it said. This is the whole point: before, the text
+	// was simply gone.
+	if md := stored("versioned-skill", 1); !strings.Contains(md, "The first body.") {
+		t.Errorf("v1 no longer holds the original text:\n%s", md)
+	}
+	if md := stored("versioned-skill", 2); !strings.Contains(md, "The second body.") {
+		t.Errorf("v2 does not hold the new text:\n%s", md)
+	}
+
+	// Saving the same text again is not a publish. Without this every save
+	// would add a version and the number would stop meaning anything.
+	res = r.as(r.admin, "PATCH", "/api/v1/skill-definitions/"+id,
+		map[string]any{"markdown": second})
+	if res.code != http.StatusOK {
+		t.Fatalf("re-saving unchanged: status %d (%v)", res.code, res.body)
+	}
+	if got := count("versioned-skill"); got != 2 {
+		t.Errorf("re-saving unchanged text added a version: %d, want 2", got)
+	}
+
+	// An inactive skill is the skill vocabulary's draft: not in service, so
+	// not recorded.
+	const inactive = `---
+id: inactive-skill
+name: Inactive Skill
+description: not in service
+status: inactive
+pages:
+  - candidates
+---
+
+# Inactive Skill
+Nothing here is published.
+`
+	res = r.as(r.admin, "POST", "/api/v1/skill-definitions", map[string]any{
+		"markdown": inactive, "visibility": "personal",
+	})
+	if res.code != http.StatusCreated {
+		t.Fatalf("create inactive skill: status %d (%v)", res.code, res.body)
+	}
+	if got := count("inactive-skill"); got != 0 {
+		t.Errorf("an inactive skill was versioned: %d, want 0", got)
+	}
+}

go-api/internal/service/definitions.go

@@ -234,6 +234,11 @@ func (s *DefinitionsService) CreateAgent(ctx context.Context, ident authctx.Iden
 		input.OwnerUserID = &ident.UserID
 	}
 
+	if err := s.refusePublishedRewrite(ctx, ident, repo.KindAgent,
+		agent.ID, markdown, agent.Status, agent.Version); err != nil {
+		return nil, err
+	}
+
 	rec, err := s.repo.InsertAgent(ctx, ident, input)
 	if err != nil {
 		return nil, err
@@ -296,6 +301,11 @@ func (s *DefinitionsService) UpdateAgent(ctx context.Context, ident authctx.Iden
 		input.Status = &agent.Status
 		input.Version = &agent.Version
 		input.Pages = agent.Pages
+
+		if err := s.refusePublishedRewrite(ctx, ident, repo.KindAgent,
+			agent.ID, markdown, agent.Status, agent.Version); err != nil {
+			return nil, err
+		}
 	} else if statusRaw, ok := patch["status"]; ok && statusRaw != nil {
 		status, isStr := statusRaw.(string)
 		if !isStr || (status != "draft" && status != "published" && status != "archived") {
@@ -425,7 +435,12 @@ func (s *DefinitionsService) CreateSkill(ctx context.Context, ident authctx.Iden
 		input.OwnerUserID = &ident.UserID
 	}
 
-	return s.repo.InsertSkill(ctx, ident, input)
+	rec, err := s.repo.InsertSkill(ctx, ident, input)
+	if err != nil {
+		return nil, err
+	}
+	_ = s.snapshotSkill(ctx, ident, rec)
+	return rec, nil
 }
 
 // UpdateSkill validates and applies updates to an authored skill definition.
@@ -483,7 +498,12 @@ func (s *DefinitionsService) UpdateSkill(ctx context.Context, ident authctx.Iden
 		input.Status = &status
 	}
 
-	return s.repo.UpdateSkill(ctx, ident, id, input)
+	rec, err := s.repo.UpdateSkill(ctx, ident, id, input)
+	if err != nil {
+		return nil, err
+	}
+	_ = s.snapshotSkill(ctx, ident, rec)
+	return rec, nil
 }
 
 // DeleteSkill removes a skill definition following idempotent delete semantics.
@@ -515,6 +535,52 @@ func (s *DefinitionsService) DeleteSkill(ctx context.Context, ident authctx.Iden
 
 /* ── Publishing ─────────────────────────────────────────────────────────── */
 
+// refusePublishedRewrite fails a publish that would change a version already
+// published, BEFORE anything is written.
+//
+// snapshotIfPublished below deliberately never fails a save: the author's work
+// is already stored and losing it to protect a record of it is the wrong trade.
+// That is right for a recording failure — the disk, the pool, the network — and
+// wrong for exactly one case. When repo.VersionsRepo.Snapshot refuses because
+// the version already says something different, that is not the history failing
+// to record; it is §3 firing. Swallowing it leaves two different definitions
+// both called v2: the live row the runtime serves, and the snapshot the history
+// shows. runtime.LoadAgentVersion resolves a pin by returning the CURRENT
+// definition whenever the pinned number equals the current one, so the run gets
+// the changed text while the audit trail says otherwise.
+//
+// So the conflict is detected here instead, before the write, where refusing
+// costs the author nothing but a version bump. The post-write snapshot keeps
+// its original contract for every other kind of failure.
+//
+// A concurrent publish of the same number with different content can still slip
+// past this check and be caught by the unique index afterwards, where it is
+// swallowed as before. That leaves the live row ahead of its snapshot, which is
+// the pre-existing behaviour and not something this guard makes worse.
+func (s *DefinitionsService) refusePublishedRewrite(ctx context.Context, ident authctx.Identity,
+	kind repo.VersionKind, definitionID, markdown, status string, version int) error {
+
+	if s.versions == nil || status != "published" || definitionID == "" || version < 1 {
+		return nil
+	}
+
+	stored, err := s.versions.Load(ctx, ident, kind, definitionID, version)
+	if err != nil || stored == nil {
+		// Absent (the ordinary case for a new version) or unreadable. Either
+		// way there is no published text to contradict, so this is not the
+		// place to fail the save.
+		return nil
+	}
+	if stored.Markdown == markdown {
+		return nil // republishing the same version unchanged is a no-op
+	}
+
+	return domain.Conflict(fmt.Sprintf(
+		"version %d of %q is already published and says something different; "+
+			"raise the version in the frontmatter to publish a change",
+		version, definitionID))
+}
+
 // snapshotIfPublished records an immutable copy when a definition is published.
 //
 // §3: editing publishes a NEW version, and a published version never changes.
@@ -563,16 +629,6 @@ func (s *DefinitionsService) snapshotIfPublished(ctx context.Context, ident auth
 
 	name, _ := rec["name"].(string)
 	description, _ := rec["description"].(string)
-	var pages []string
-	if raw, ok := rec["pages"].([]string); ok {
-		pages = raw
-	} else if raw, ok := rec["pages"].([]any); ok {
-		for _, p := range raw {
-			if str, ok := p.(string); ok {
-				pages = append(pages, str)
-			}
-		}
-	}
 
 	return s.versions.Snapshot(ctx, ident, repo.SnapshotInput{
 		Kind:         kind,
@@ -581,7 +637,93 @@ func (s *DefinitionsService) snapshotIfPublished(ctx context.Context, ident auth
 		Markdown:     markdown,
 		Name:         name,
 		Description:  description,
-		Pages:        pages,
+		Pages:        recordPages(rec),
+	})
+}
+
+// recordPages reads a record's pages, which arrive as []string from the
+// repository and as []any when they have been through JSON.
+func recordPages(rec domain.Record) []string {
+	if raw, ok := rec["pages"].([]string); ok {
+		return raw
+	}
+	raw, ok := rec["pages"].([]any)
+	if !ok {
+		return nil
+	}
+	pages := make([]string, 0, len(raw))
+	for _, p := range raw {
+		if str, isStr := p.(string); isStr {
+			pages = append(pages, str)
+		}
+	}
+	return pages
+}
+
+// snapshotSkill records an immutable copy of a skill, numbered by the server.
+//
+// Skills carry no version. An agent's frontmatter names one, so its author
+// decides when a change is a new version and can be refused for rewriting an
+// old one. A skill has no such field, and giving it one would mean a migration,
+// a parser change on BOTH sides of the conformance test in
+// internal/definition, and an edit to all 23 shipped skills — a feature, not
+// the fix this is.
+//
+// So the number is the server's: one after whatever was last published. That
+// is what repo.VersionsRepo.LatestVersion was written for ("the next published
+// version has to follow what was actually published rather than what somebody
+// wrote in the frontmatter") and what migration 000010 means by "agents and
+// skills version identically". It was built and never wired to anything.
+//
+// Because the author never names a version, there is nothing here to refuse:
+// an edit is always a NEW version, and a save that changed nothing is not a
+// version at all. The comparison against the last published copy is what keeps
+// the history from filling with keystrokes.
+//
+// Two simultaneous edits can both compute the same next number; one wins and
+// the other's snapshot is dropped, leaving a version unrecorded. That is the
+// same narrow race the agent path has, and the same reason it is tolerated
+// here: failing an author's save to record it is the wrong trade.
+func (s *DefinitionsService) snapshotSkill(ctx context.Context, ident authctx.Identity,
+	rec domain.Record) error {
+
+	if s.versions == nil || rec == nil {
+		return nil
+	}
+	// Only a skill that is in service. "inactive" is the skill vocabulary's
+	// equivalent of a draft — see definition.SkillStatuses.
+	if status, _ := rec["status"].(string); status != "active" {
+		return nil
+	}
+
+	markdown, _ := rec["markdown"].(string)
+	definitionID, _ := rec["definition_id"].(string)
+	if markdown == "" || definitionID == "" {
+		return nil
+	}
+
+	latest, err := s.versions.LatestVersion(ctx, ident, repo.KindSkill, definitionID)
+	if err != nil {
+		return err
+	}
+	if latest > 0 {
+		stored, err := s.versions.Load(ctx, ident, repo.KindSkill, definitionID, latest)
+		if err == nil && stored != nil && stored.Markdown == markdown {
+			return nil // unchanged since the last publish
+		}
+	}
+
+	name, _ := rec["name"].(string)
+	description, _ := rec["description"].(string)
+
+	return s.versions.Snapshot(ctx, ident, repo.SnapshotInput{
+		Kind:         repo.KindSkill,
+		DefinitionID: definitionID,
+		Version:      latest + 1,
+		Markdown:     markdown,
+		Name:         name,
+		Description:  description,
+		Pages:        recordPages(rec),
 	})
 }