fix(office): normalize skill slugs to canonical form on write and sync (#3257)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>

apps/backend/internal/agent/runtime/lifecycle/skill/paths.go

@@ -2,17 +2,15 @@ package skill
 
 import (
 	"path/filepath"
-	"regexp"
 	"strings"
+
+	"github.com/kandev/kandev/internal/common/skillslug"
 )
 
-// validSlugRe matches slugs that are safe for use in shell commands
-// and on-disk paths. Anything outside this set is dropped during
+// isValidSlug reports whether the given slug is safe for use in shell
+// commands and on-disk paths. Anything outside this set is dropped during
 // delivery to avoid path-traversal or shell-quoting hazards.
-var validSlugRe = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
-
-// isValidSlug reports whether the given slug is safe.
-func isValidSlug(s string) bool { return s != "" && validSlugRe.MatchString(s) }
+func isValidSlug(s string) bool { return skillslug.WellFormed(s) }
 
 // isValidPathComponent reports whether the given filename is a single
 // safe path component (no separators, no traversal). Used when writing

apps/backend/internal/office/repository/sqlite/skills.go

@@ -3,10 +3,13 @@ package sqlite
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/jmoiron/sqlx"
 
 	"github.com/kandev/kandev/internal/office/models"
 )
@@ -117,6 +120,27 @@ func (r *Repository) ListSystemSkills(
 	return skills, nil
 }
 
+// ListNonSystemSkills returns all is_system = false skills for a
+// workspace, ordered by slug. Used by the system-skill sync's
+// slug-migration pass: user/provider-imported rows that need a
+// well-formed-but-non-canonical slug normalized to canonical, and the
+// conflict check before inserting a newly-bundled canonical slug.
+func (r *Repository) ListNonSystemSkills(
+	ctx context.Context, workspaceID string,
+) ([]*models.Skill, error) {
+	var skills []*models.Skill
+	err := r.ro.SelectContext(ctx, &skills, r.ro.Rebind(
+		`SELECT * FROM office_skills WHERE workspace_id = ? AND is_system = 0 ORDER BY slug`),
+		workspaceID)
+	if err != nil {
+		return nil, err
+	}
+	if skills == nil {
+		return []*models.Skill{}, nil
+	}
+	return skills, nil
+}
+
 // UpdateSkill updates an existing skill.
 func (r *Repository) UpdateSkill(ctx context.Context, skill *models.Skill) error {
 	skill.UpdatedAt = time.Now().UTC()
@@ -141,6 +165,176 @@ func (r *Repository) UpdateSkill(ctx context.Context, skill *models.Skill) error
 	return err
 }
 
+// NormalizeSkillSlug atomically changes a non-system skill's slug and
+// rewrites matching desired_skills references on all active office agents.
+// The skill ID and old slug are both part of the update predicate, so a
+// concurrent writer cannot cause references to be rewritten for a different
+// row. It returns false when the row no longer matches that identity.
+func (r *Repository) NormalizeSkillSlug(
+	ctx context.Context,
+	workspaceID, skillID, oldSlug, newSlug string,
+) (bool, error) {
+	tx, err := r.db.BeginTxx(ctx, nil)
+	if err != nil {
+		return false, fmt.Errorf("begin skill slug normalization: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	result, err := tx.ExecContext(ctx, tx.Rebind(`
+		UPDATE office_skills
+		SET slug = ?, updated_at = ?
+		WHERE id = ? AND workspace_id = ? AND slug = ? AND is_system = 0
+	`), newSlug, time.Now().UTC(), skillID, workspaceID, oldSlug)
+	if err != nil {
+		return false, err
+	}
+	updated, err := result.RowsAffected()
+	if err != nil {
+		return false, fmt.Errorf("check skill slug normalization: %w", err)
+	}
+	if updated == 0 {
+		return false, nil
+	}
+
+	updates, err := collectAgentDesiredSkillUpdates(ctx, tx, workspaceID, oldSlug, newSlug)
+	if err != nil {
+		return false, err
+	}
+	if err := applyAgentDesiredSkillUpdates(ctx, tx, workspaceID, updates); err != nil {
+		return false, err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return false, fmt.Errorf("commit skill slug normalization: %w", err)
+	}
+	return true, nil
+}
+
+type agentDesiredSkillUpdate struct {
+	id       string
+	original string
+	desired  string
+}
+
+func collectAgentDesiredSkillUpdates(
+	ctx context.Context,
+	tx *sqlx.Tx,
+	workspaceID, oldSlug, newSlug string,
+) ([]agentDesiredSkillUpdate, error) {
+	rows, err := tx.QueryxContext(ctx, tx.Rebind(`
+		SELECT id, COALESCE(desired_skills, '') AS desired_skills
+		FROM agent_profiles
+		WHERE workspace_id = ? AND deleted_at IS NULL
+	`), workspaceID)
+	if err != nil {
+		return nil, fmt.Errorf("list agents for skill slug normalization: %w", err)
+	}
+	defer func() { _ = rows.Close() }()
+
+	updates := make([]agentDesiredSkillUpdate, 0)
+	for rows.Next() {
+		var update agentDesiredSkillUpdate
+		if err := rows.Scan(&update.id, &update.original); err != nil {
+			return nil, fmt.Errorf("scan agent for skill slug normalization: %w", err)
+		}
+		update.desired, _ = replaceSkillSlugInAgentList(update.original, oldSlug, newSlug)
+		if update.desired != update.original {
+			updates = append(updates, update)
+		}
+	}
+	if err := rows.Err(); err != nil {
+		return nil, fmt.Errorf("read agents for skill slug normalization: %w", err)
+	}
+	if err := rows.Close(); err != nil {
+		return nil, fmt.Errorf("close agents for skill slug normalization: %w", err)
+	}
+	return updates, nil
+}
+
+func applyAgentDesiredSkillUpdates(
+	ctx context.Context,
+	tx *sqlx.Tx,
+	workspaceID string,
+	updates []agentDesiredSkillUpdate,
+) error {
+	now := time.Now().UTC()
+	for _, update := range updates {
+		result, err := tx.ExecContext(ctx, tx.Rebind(`
+			UPDATE agent_profiles
+			SET desired_skills = ?, updated_at = ?
+			WHERE id = ? AND workspace_id = ? AND deleted_at IS NULL
+				AND COALESCE(desired_skills, '') = ?
+		`), update.desired, now, update.id, workspaceID, update.original)
+		if err != nil {
+			return fmt.Errorf("update agent %s for skill slug normalization: %w", update.id, err)
+		}
+		rowsAffected, err := result.RowsAffected()
+		if err != nil {
+			return fmt.Errorf("check agent %s for skill slug normalization: %w", update.id, err)
+		}
+		if rowsAffected != 1 {
+			return fmt.Errorf("agent %s changed during skill slug normalization", update.id)
+		}
+	}
+	return nil
+}
+
+// replaceSkillSlugInAgentList mirrors the office desired_skills reader. It
+// accepts both the canonical JSON-array format and the legacy CSV format, and
+// emits a deduplicated JSON array only when a value changes.
+func replaceSkillSlugInAgentList(raw, oldSlug, newSlug string) (string, bool) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" || raw == "null" {
+		return raw, false
+	}
+	values, ok := parseAgentSkillList(raw)
+	if !ok {
+		return raw, false
+	}
+
+	out := make([]string, 0, len(values))
+	seen := make(map[string]bool, len(values))
+	changed := false
+	for _, value := range values {
+		if value == oldSlug {
+			value = newSlug
+			changed = true
+		}
+		if value == "" || seen[value] {
+			if value != "" {
+				changed = true
+			}
+			continue
+		}
+		seen[value] = true
+		out = append(out, value)
+	}
+	if !changed {
+		return raw, false
+	}
+	encoded, err := json.Marshal(out)
+	if err != nil {
+		return raw, false
+	}
+	return string(encoded), true
+}
+
+func parseAgentSkillList(raw string) ([]string, bool) {
+	if strings.HasPrefix(raw, "[") {
+		var values []string
+		if err := json.Unmarshal([]byte(raw), &values); err != nil {
+			return nil, false
+		}
+		return values, true
+	}
+	parts := strings.Split(raw, ",")
+	values := make([]string, len(parts))
+	for i, part := range parts {
+		values[i] = strings.TrimSpace(part)
+	}
+	return values, true
+}
+
 // SkillConfigFields is the subset of office_skills columns a config import
 // owns (see UpdateSkillConfigFields).
 type SkillConfigFields struct {

apps/backend/internal/office/repository/sqlite/skills_slug_atomic_test.go

@@ -0,0 +1,123 @@
+package sqlite_test
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/kandev/kandev/internal/office/models"
+)
+
+func TestNormalizeSkillSlugAtomicallyUpdatesAgentReferences(t *testing.T) {
+	repo := newTestRepo(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{
+		ID:          "skill-1",
+		WorkspaceID: "ws-1",
+		Name:        "Custom skill",
+		Slug:        "custom-skill",
+		SourceType:  "inline",
+	}
+	if err := repo.CreateSkill(ctx, skill); err != nil {
+		t.Fatalf("create skill: %v", err)
+	}
+	agent := &models.AgentInstance{
+		ID:            "agent-1",
+		WorkspaceID:   "ws-1",
+		Name:          "Agent",
+		DesiredSkills: `["custom-skill","other-skill","custom-skill"]`,
+	}
+	if err := repo.CreateAgentInstance(ctx, agent); err != nil {
+		t.Fatalf("create agent: %v", err)
+	}
+
+	changed, err := repo.NormalizeSkillSlug(ctx, "ws-1", skill.ID, "custom-skill", "kandev-custom-skill")
+	if err != nil {
+		t.Fatalf("normalize skill slug: %v", err)
+	}
+	if !changed {
+		t.Fatal("normalize skill slug reported no change")
+	}
+
+	gotSkill, err := repo.GetSkillBySlug(ctx, "ws-1", "kandev-custom-skill")
+	if err != nil {
+		t.Fatalf("get normalized skill: %v", err)
+	}
+	if gotSkill.ID != skill.ID {
+		t.Fatalf("normalized skill ID = %q, want %q", gotSkill.ID, skill.ID)
+	}
+	if _, err := repo.GetSkillBySlug(ctx, "ws-1", "custom-skill"); err == nil {
+		t.Fatal("old skill slug still resolves after normalization")
+	}
+
+	gotAgent, err := repo.GetAgentInstance(ctx, agent.ID)
+	if err != nil {
+		t.Fatalf("get agent: %v", err)
+	}
+	var desired []string
+	if err := json.Unmarshal([]byte(gotAgent.DesiredSkills), &desired); err != nil {
+		t.Fatalf("decode desired_skills %q: %v", gotAgent.DesiredSkills, err)
+	}
+	want := []string{"kandev-custom-skill", "other-skill"}
+	if len(desired) != len(want) || desired[0] != want[0] || desired[1] != want[1] {
+		t.Fatalf("desired_skills = %v, want %v", desired, want)
+	}
+}
+
+func TestNormalizeSkillSlugRollsBackWhenCanonicalSlugIsTaken(t *testing.T) {
+	repo := newTestRepo(t)
+	ctx := context.Background()
+
+	oldSkill := &models.Skill{
+		ID:          "skill-old",
+		WorkspaceID: "ws-1",
+		Name:        "Custom skill",
+		Slug:        "custom-skill",
+		SourceType:  "inline",
+	}
+	takenSkill := &models.Skill{
+		ID:          "skill-taken",
+		WorkspaceID: "ws-1",
+		Name:        "Canonical skill",
+		Slug:        "kandev-custom-skill",
+		SourceType:  "inline",
+	}
+	for _, skill := range []*models.Skill{oldSkill, takenSkill} {
+		if err := repo.CreateSkill(ctx, skill); err != nil {
+			t.Fatalf("create skill %s: %v", skill.ID, err)
+		}
+	}
+	agent := &models.AgentInstance{
+		ID:            "agent-1",
+		WorkspaceID:   "ws-1",
+		Name:          "Agent",
+		DesiredSkills: `["custom-skill"]`,
+	}
+	if err := repo.CreateAgentInstance(ctx, agent); err != nil {
+		t.Fatalf("create agent: %v", err)
+	}
+
+	changed, err := repo.NormalizeSkillSlug(ctx, "ws-1", oldSkill.ID, "custom-skill", "kandev-custom-skill")
+	if err == nil {
+		t.Fatal("normalization should fail when the canonical slug is taken")
+	}
+	if changed {
+		t.Fatal("normalization reported a change after a rolled-back transaction")
+	}
+
+	gotOld, err := repo.GetSkillBySlug(ctx, "ws-1", "custom-skill")
+	if err != nil {
+		t.Fatalf("old skill row was not preserved: %v", err)
+	}
+	if gotOld.ID != oldSkill.ID {
+		t.Fatalf("old skill ID = %q, want %q", gotOld.ID, oldSkill.ID)
+	}
+	gotAgent, err := repo.GetAgentInstance(ctx, agent.ID)
+	if err != nil {
+		t.Fatalf("get agent: %v", err)
+	}
+	if gotAgent.DesiredSkills != agent.DesiredSkills {
+		t.Fatalf("desired_skills = %q after rollback, want %q", gotAgent.DesiredSkills, agent.DesiredSkills)
+	}
+}

apps/backend/internal/office/skills/service.go

@@ -8,6 +8,7 @@ import (
 
 	settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
 	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/common/skillslug"
 	"github.com/kandev/kandev/internal/office/models"
 	"github.com/kandev/kandev/internal/office/shared"
 
@@ -25,6 +26,8 @@ type skillRepo interface {
 	GetSkillBySlug(ctx context.Context, workspaceID, slug string) (*models.Skill, error)
 	ListSkills(ctx context.Context, workspaceID string) ([]*models.Skill, error)
 	ListSystemSkills(ctx context.Context, workspaceID string) ([]*models.Skill, error)
+	ListNonSystemSkills(ctx context.Context, workspaceID string) ([]*models.Skill, error)
+	NormalizeSkillSlug(ctx context.Context, workspaceID, skillID, oldSlug, newSlug string) (bool, error)
 	UpdateSkill(ctx context.Context, skill *models.Skill) error
 	DeleteSkill(ctx context.Context, id string) error
 
@@ -130,13 +133,22 @@ func (s *SkillService) countSkillUsage(ctx context.Context) map[string]int {
 
 // ValidateAndPrepareSkill validates and prepares a skill for creation.
 // Call this before CreateSkill to auto-generate slug and validate uniqueness.
+//
+// Slug handling follows AC-001.11/AC-001.12: a caller-supplied slug must be
+// well-formed or the request is rejected outright (no coercion); a
+// well-formed slug is then normalized to its canonical (kandev-prefixed)
+// form before the uniqueness check runs, so both the persisted slug and the
+// duplicate check operate on the same canonical value.
 func (s *SkillService) ValidateAndPrepareSkill(ctx context.Context, skill *models.Skill) error {
 	if skill.Name == "" {
 		return fmt.Errorf("skill name is required")
 	}
 	if skill.Slug == "" {
 		skill.Slug = GenerateSlug(skill.Name)
+	} else if !skillslug.WellFormed(skill.Slug) {
+		return fmt.Errorf("invalid skill slug %q: must contain only letters, digits, underscore, and hyphen", skill.Slug)
 	}
+	skill.Slug = skillslug.Normalize(skill.Slug)
 	if err := s.validateSlugUnique(ctx, skill.WorkspaceID, skill.Slug, ""); err != nil {
 		return err
 	}
@@ -156,12 +168,19 @@ func prepareSkillPackageMetadata(skill *models.Skill) {
 	)
 }
 
-// ValidateSkillUpdate validates a skill update for slug uniqueness.
+// ValidateSkillUpdate validates a skill update for slug uniqueness. Slug
+// handling mirrors ValidateAndPrepareSkill: reject a not-well-formed slug
+// outright (AC-001.11), otherwise normalize to canonical before the
+// uniqueness check (AC-001.12).
 func (s *SkillService) ValidateSkillUpdate(ctx context.Context, skill *models.Skill) error {
-	if skill.Slug != "" {
-		return s.validateSlugUnique(ctx, skill.WorkspaceID, skill.Slug, skill.ID)
+	if skill.Slug == "" {
+		return nil
 	}
-	return nil
+	if !skillslug.WellFormed(skill.Slug) {
+		return fmt.Errorf("invalid skill slug %q: must contain only letters, digits, underscore, and hyphen", skill.Slug)
+	}
+	skill.Slug = skillslug.Normalize(skill.Slug)
+	return s.validateSlugUnique(ctx, skill.WorkspaceID, skill.Slug, skill.ID)
 }
 
 func (s *SkillService) validateSlugUnique(ctx context.Context, workspaceID, slug, excludeID string) error {

apps/backend/internal/office/skills/service_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/jmoiron/sqlx"
 	_ "github.com/mattn/go-sqlite3"
 
+	settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
 	"github.com/kandev/kandev/internal/common/logger"
 	"github.com/kandev/kandev/internal/office/models"
 	"github.com/kandev/kandev/internal/office/repository/sqlite"
@@ -35,6 +36,10 @@ func newTestSkillService(t *testing.T) *skills.SkillService {
 	}
 	t.Cleanup(func() { _ = db.Close() })
 
+	if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
+		t.Fatalf("provide settings store: %v", err)
+	}
+
 	repo, err := sqlite.NewWithDB(db, db, nil)
 	if err != nil {
 		t.Fatalf("new repo: %v", err)
@@ -86,8 +91,92 @@ func TestValidateAndPrepareSkill_AutoGeneratesSlug(t *testing.T) {
 	if err := svc.ValidateAndPrepareSkill(ctx, skill); err != nil {
 		t.Fatalf("validate: %v", err)
 	}
-	if skill.Slug != "code-review" {
-		t.Errorf("slug = %q, want %q", skill.Slug, "code-review")
+	if skill.Slug != "kandev-code-review" {
+		t.Errorf("slug = %q, want %q", skill.Slug, "kandev-code-review")
+	}
+}
+
+// TestValidateAndPrepareSkill_NormalizesExplicitSlugToCanonical covers
+// AC-001.12: a well-formed but non-canonical caller-supplied slug is
+// normalized to canonical before persisting.
+func TestValidateAndPrepareSkill_NormalizesExplicitSlugToCanonical(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{
+		WorkspaceID: "ws-1",
+		Name:        "My Skill",
+		Slug:        "my-skill",
+		SourceType:  "inline",
+	}
+	if err := svc.ValidateAndPrepareSkill(ctx, skill); err != nil {
+		t.Fatalf("validate: %v", err)
+	}
+	if skill.Slug != "kandev-my-skill" {
+		t.Errorf("slug = %q, want %q", skill.Slug, "kandev-my-skill")
+	}
+}
+
+// TestValidateAndPrepareSkill_CanonicalSlugUnchanged covers AC-001.6: a
+// slug that is already canonical is left byte-identical.
+func TestValidateAndPrepareSkill_CanonicalSlugUnchanged(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{
+		WorkspaceID: "ws-1",
+		Name:        "Already Canonical",
+		Slug:        "kandev-already-canonical",
+		SourceType:  "inline",
+	}
+	if err := svc.ValidateAndPrepareSkill(ctx, skill); err != nil {
+		t.Fatalf("validate: %v", err)
+	}
+	if skill.Slug != "kandev-already-canonical" {
+		t.Errorf("slug = %q, want %q", skill.Slug, "kandev-already-canonical")
+	}
+}
+
+// TestValidateAndPrepareSkill_RejectsNotWellFormedSlug covers AC-001.11: a
+// not-well-formed caller-supplied slug is rejected outright, never coerced.
+func TestValidateAndPrepareSkill_RejectsNotWellFormedSlug(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{
+		WorkspaceID: "ws-1",
+		Name:        "Bad Slug",
+		Slug:        "not a valid slug!",
+		SourceType:  "inline",
+	}
+	err := svc.ValidateAndPrepareSkill(ctx, skill)
+	if err == nil {
+		t.Fatal("expected error for not-well-formed slug")
+	}
+	if skill.Slug != "not a valid slug!" {
+		t.Errorf("slug was coerced from its original input: %q", skill.Slug)
+	}
+}
+
+// TestValidateAndPrepareSkill_UniquenessCheckedOnCanonicalValue covers the
+// second half of AC-001.12: a request whose canonical slug already exists
+// is rejected, even when the request itself supplied the non-canonical
+// form.
+func TestValidateAndPrepareSkill_UniquenessCheckedOnCanonicalValue(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	existing := &models.Skill{WorkspaceID: "ws-1", Name: "Existing", Slug: "kandev-my-skill", SourceType: "inline"}
+	if err := svc.ValidateAndPrepareSkill(ctx, existing); err != nil {
+		t.Fatalf("validate existing: %v", err)
+	}
+	if err := svc.CreateSkill(ctx, existing); err != nil {
+		t.Fatalf("create existing: %v", err)
+	}
+
+	colliding := &models.Skill{WorkspaceID: "ws-1", Name: "Colliding", Slug: "my-skill", SourceType: "inline"}
+	if err := svc.ValidateAndPrepareSkill(ctx, colliding); err == nil {
+		t.Fatal("expected duplicate slug error against the canonical form")
 	}
 }
 
@@ -143,6 +232,47 @@ func TestValidateAndPrepareSkill_RejectsSameSlugInSameWorkspace(t *testing.T) {
 	}
 }
 
+// --- ValidateSkillUpdate tests ---
+
+func TestValidateSkillUpdate_NormalizesWellFormedSlugToCanonical(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{WorkspaceID: "ws-1", Name: "Existing", Slug: "kandev-existing", SourceType: "inline"}
+	if err := svc.ValidateAndPrepareSkill(ctx, skill); err != nil {
+		t.Fatalf("validate: %v", err)
+	}
+	if err := svc.CreateSkill(ctx, skill); err != nil {
+		t.Fatalf("create: %v", err)
+	}
+
+	skill.Slug = "renamed"
+	if err := svc.ValidateSkillUpdate(ctx, skill); err != nil {
+		t.Fatalf("update: %v", err)
+	}
+	if skill.Slug != "kandev-renamed" {
+		t.Errorf("slug = %q, want %q", skill.Slug, "kandev-renamed")
+	}
+}
+
+func TestValidateSkillUpdate_RejectsNotWellFormedSlug(t *testing.T) {
+	svc := newTestSkillService(t)
+	ctx := context.Background()
+
+	skill := &models.Skill{WorkspaceID: "ws-1", Name: "Existing", Slug: "kandev-existing", SourceType: "inline"}
+	if err := svc.ValidateAndPrepareSkill(ctx, skill); err != nil {
+		t.Fatalf("validate: %v", err)
+	}
+	if err := svc.CreateSkill(ctx, skill); err != nil {
+		t.Fatalf("create: %v", err)
+	}
+
+	skill.Slug = "not a valid slug!"
+	if err := svc.ValidateSkillUpdate(ctx, skill); err == nil {
+		t.Fatal("expected error for not-well-formed slug")
+	}
+}
+
 func TestValidateAndPrepareSkill_RejectsInvalidSourceType(t *testing.T) {
 	svc := newTestSkillService(t)
 	ctx := context.Background()
@@ -167,24 +297,27 @@ func TestListSkillsWithUsage(t *testing.T) {
 		t.Fatalf("create: %v", err)
 	}
 
-	// The lazy system-skill sync fires on the first ListSkills call
-	// for a workspace, populating bundled system rows alongside the
-	// user skill created above. The test asserts the *user* skill
-	// appears with the correct usage count; the system rows are
-	// validated separately in system_sync_test.go.
+	// The lazy system-skill sync fires on the first ListSkills call for
+	// a workspace, populating bundled system rows alongside the user
+	// skill created above. It also normalizes the well-formed but
+	// non-canonical "my-skill" slug to "kandev-my-skill" (AC-003.8),
+	// so the test looks for the row under its normalized slug. The
+	// normalization mechanics themselves are covered in
+	// system_sync_migration_test.go; this test only asserts the *user*
+	// skill survives sync with the correct usage count.
 	list, err := svc.ListSkillsWithUsage(ctx, "ws-1")
 	if err != nil {
 		t.Fatalf("list: %v", err)
 	}
 	var userRow *skills.SkillWithUsage
 	for _, row := range list {
-		if row.Slug == "my-skill" {
+		if row.Slug == "kandev-my-skill" {
 			userRow = row
 			break
 		}
 	}
 	if userRow == nil {
-		t.Fatalf("user skill not found in list of %d rows", len(list))
+		t.Fatalf("normalized user skill not found in list of %d rows", len(list))
 	}
 	if userRow.UsedByCount != 0 {
 		t.Errorf("used_by_count = %d, want 0", userRow.UsedByCount)

apps/backend/internal/office/skills/system_sync.go

@@ -5,16 +5,20 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/google/uuid"
+	"github.com/jackc/pgx/v5/pgconn"
 	"go.uber.org/zap"
 	"gopkg.in/yaml.v3"
 
 	settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
 	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/common/skillslug"
 	"github.com/kandev/kandev/internal/office/configloader"
 	"github.com/kandev/kandev/internal/office/models"
 )
@@ -34,6 +38,7 @@ var retiredDefaultSkillReplacements = map[string]string{
 	"kandev-task-comment":  "kandev-task-ops",
 	"kandev-tasks":         "kandev-task-ops",
 	"kandev-team":          "kandev-team-admin",
+	"memory":               "kandev-memory",
 }
 
 // SystemSkillSpec is the parsed view of a single embedded SKILL.md
@@ -61,6 +66,18 @@ type SystemSyncRepo interface {
 	UpdateSkill(ctx context.Context, skill *models.Skill) error
 	DeleteSkill(ctx context.Context, id string) error
 
+	// ListNonSystemSkills returns every user/provider-imported row
+	// (is_system = false) in the workspace. Used by the bundled-insert
+	// slug-conflict pre-check (AC-003.3) and the user-skill slug
+	// normalization pass (AC-003.8/AC-003.9).
+	ListNonSystemSkills(ctx context.Context, workspaceID string) ([]*models.Skill, error)
+
+	// NormalizeSkillSlug atomically changes a non-system skill's slug and
+	// rewrites all matching agent desired_skills references. It returns
+	// false when the row no longer matches the supplied identity, which
+	// means another writer already handled or removed it.
+	NormalizeSkillSlug(ctx context.Context, workspaceID, skillID, oldSlug, newSlug string) (bool, error)
+
 	// Agent-profile access used to scrub a deleted system skill's ID
 	// out of every agent_profiles.skill_ids JSON array in the same
 	// workspace, so retiring a bundled skill doesn't leave dangling
@@ -72,10 +89,44 @@ type SystemSyncRepo interface {
 // SyncReport summarises one sync pass across all workspaces. The
 // startup caller surfaces this as a single log line so operators can
 // see exactly which slugs landed where after a kandev upgrade.
+//
+// Every per-workspace list is sorted lexicographically by slug; the
+// scoped `<workspace_id>:<slug>` entries in the aggregate report are
+// sorted by workspace ID then slug (AC-003.5).
 type SyncReport struct {
 	Inserted []string
 	Updated  []string
 	Removed  []string
+
+	// Blocked lists retired system skills whose configured replacement
+	// was not found in this workspace this pass (e.g. its insert was
+	// withheld by a slug conflict). The retired row is left in place
+	// rather than deleted, so no agent silently loses the capability
+	// (AC-003.1); a later pass retries once the conflict resolves.
+	Blocked []string
+
+	// Normalized lists user-skill slugs rewritten from a well-formed
+	// but non-canonical form to their canonical kandev-prefixed form,
+	// as "<old>->new>" entries (AC-003.8).
+	Normalized []string
+
+	// Conflicted lists slugs left untouched because normalizing (or
+	// inserting a bundled row) would collide with another row's
+	// current slug (AC-003.3/AC-003.9).
+	Conflicted []string
+}
+
+// workspaceSyncLocks serializes SyncSystemSkills passes per
+// workspace. SyncSystemSkills has multiple concurrent call sites in
+// one process (lazy per-workspace sync, startup sync, config import,
+// etc.); a pass that can't acquire the lock waits rather than
+// skipping, so two concurrent passes over the same workspace never
+// interleave their insert/retire/normalize phases (AC-003.7).
+var workspaceSyncLocks sync.Map // map[string]*sync.Mutex
+
+func lockForWorkspace(wsID string) *sync.Mutex {
+	v, _ := workspaceSyncLocks.LoadOrStore(wsID, &sync.Mutex{})
+	return v.(*sync.Mutex)
 }
 
 // SyncSystemSkills idempotently reconciles the office_skills table
@@ -112,45 +163,123 @@ func SyncSystemSkills(
 
 	var report SyncReport
 	for _, wsID := range workspaceIDs {
-		ins, upd, rem, err := syncWorkspace(ctx, repo, wsID, bundledBySlug)
+		result, err := syncWorkspace(ctx, repo, wsID, bundledBySlug, log)
 		if err != nil {
 			log.Error("system skill sync failed for workspace",
 				zap.String("workspace_id", wsID), zap.Error(err))
 			continue
 		}
-		report.Inserted = append(report.Inserted, scope(wsID, ins)...)
-		report.Updated = append(report.Updated, scope(wsID, upd)...)
-		report.Removed = append(report.Removed, scope(wsID, rem)...)
+		report.Inserted = append(report.Inserted, scope(wsID, result.Inserted)...)
+		report.Updated = append(report.Updated, scope(wsID, result.Updated)...)
+		report.Removed = append(report.Removed, scope(wsID, result.Removed)...)
+		report.Blocked = append(report.Blocked, scope(wsID, result.Blocked)...)
+		report.Normalized = append(report.Normalized, scope(wsID, result.Normalized)...)
+		report.Conflicted = append(report.Conflicted, scope(wsID, result.Conflicted)...)
 	}
+	sort.Strings(report.Inserted)
+	sort.Strings(report.Updated)
+	sort.Strings(report.Removed)
+	sort.Strings(report.Blocked)
+	sort.Strings(report.Normalized)
+	sort.Strings(report.Conflicted)
 	log.Info("system skills synced",
 		zap.Int("workspaces", len(workspaceIDs)),
 		zap.Int("bundled", len(specs)),
 		zap.Strings("inserted", report.Inserted),
 		zap.Strings("updated", report.Updated),
 		zap.Strings("removed", report.Removed),
+		zap.Strings("blocked", report.Blocked),
+		zap.Strings("normalized", report.Normalized),
+		zap.Strings("conflicted", report.Conflicted),
 	)
 	return report, nil
 }
 
-// syncWorkspace handles one workspace. Returns the (insert, update,
-// remove) slug lists for the report. Errors propagate; the caller
-// logs and continues to the next workspace so one bad row doesn't
-// gate the rest.
+// workspaceSyncResult carries one workspace's sync outcome between
+// syncWorkspace's phases and back to SyncSystemSkills, which scopes
+// each list into the aggregate SyncReport.
+type workspaceSyncResult struct {
+	Inserted   []string
+	Updated    []string
+	Removed    []string
+	Blocked    []string
+	Normalized []string
+	Conflicted []string
+}
+
+// syncWorkspace handles one workspace: reconcile bundled system
+// skills (insert/update), retire orphaned system rows, then normalize
+// non-canonical user-skill slugs. Errors propagate; the caller logs
+// and continues to the next workspace so one bad row doesn't gate the
+// rest. Holds the per-workspace lock for the whole pass so concurrent
+// SyncSystemSkills call sites never interleave these phases
+// (AC-003.7).
 func syncWorkspace(
 	ctx context.Context,
 	repo SystemSyncRepo,
 	wsID string,
 	bundled map[string]SystemSkillSpec,
-) (inserted, updated, removed []string, err error) {
+	log *logger.Logger,
+) (workspaceSyncResult, error) {
+	mu := lockForWorkspace(wsID)
+	mu.Lock()
+	defer mu.Unlock()
+
+	var result workspaceSyncResult
+
 	existing, err := repo.ListSystemSkills(ctx, wsID)
 	if err != nil {
-		return nil, nil, nil, err
+		return result, err
 	}
 	existingBySlug := make(map[string]*models.Skill, len(existing))
 	for _, s := range existing {
 		existingBySlug[s.Slug] = s
 	}
 
+	nonSystem, err := repo.ListNonSystemSkills(ctx, wsID)
+	if err != nil {
+		return result, fmt.Errorf("list non-system skills: %w", err)
+	}
+	nonSystemBySlug := make(map[string]*models.Skill, len(nonSystem))
+	for _, s := range nonSystem {
+		nonSystemBySlug[s.Slug] = s
+	}
+
+	if err := reconcileBundledSkills(ctx, repo, wsID, bundled, existingBySlug, nonSystemBySlug, &result); err != nil {
+		return result, err
+	}
+	if err := retireOrphanedSystemSkills(ctx, repo, wsID, bundled, existingBySlug, &result); err != nil {
+		return result, err
+	}
+	if err := normalizeUserSkillSlugs(ctx, repo, wsID, nonSystem, existingBySlug, nonSystemBySlug, &result, log); err != nil {
+		return result, err
+	}
+
+	sort.Strings(result.Inserted)
+	sort.Strings(result.Updated)
+	sort.Strings(result.Removed)
+	sort.Strings(result.Blocked)
+	sort.Strings(result.Normalized)
+	sort.Strings(result.Conflicted)
+	return result, nil
+}
+
+// reconcileBundledSkills walks the bundled spec set in sorted order,
+// inserting missing rows and updating drifted ones. A bundled slug
+// not yet present as a system row but already held by a non-system
+// row is skipped and recorded as a conflict rather than inserted
+// (AC-003.3); the CreateSkill unique-constraint fallback below covers
+// the same outcome for the race between this pre-check and the
+// insert.
+func reconcileBundledSkills(
+	ctx context.Context,
+	repo SystemSyncRepo,
+	wsID string,
+	bundled map[string]SystemSkillSpec,
+	existingBySlug map[string]*models.Skill,
+	nonSystemBySlug map[string]*models.Skill,
+	result *workspaceSyncResult,
+) error {
 	// Walk bundled slugs in sorted order so log output is stable.
 	slugs := make([]string, 0, len(bundled))
 	for slug := range bundled {
@@ -162,12 +291,20 @@ func syncWorkspace(
 		spec := bundled[slug]
 		cur, ok := existingBySlug[slug]
 		if !ok {
+			if _, taken := nonSystemBySlug[slug]; taken {
+				result.Conflicted = append(result.Conflicted, slug)
+				continue
+			}
 			row := newSystemSkillRow(wsID, spec)
 			if err := repo.CreateSkill(ctx, row); err != nil {
-				return inserted, updated, removed, fmt.Errorf("insert %s: %w", slug, err)
+				if isSlugUniqueConstraintErr(err) {
+					result.Conflicted = append(result.Conflicted, slug)
+					continue
+				}
+				return fmt.Errorf("insert %s: %w", slug, err)
 			}
 			existingBySlug[slug] = row
-			inserted = append(inserted, slug)
+			result.Inserted = append(result.Inserted, slug)
 			continue
 		}
 		if systemSkillUpToDate(cur, spec) {
@@ -175,37 +312,162 @@ func syncWorkspace(
 		}
 		applySystemSkillUpdate(cur, spec)
 		if err := repo.UpdateSkill(ctx, cur); err != nil {
-			return inserted, updated, removed, fmt.Errorf("update %s: %w", slug, err)
+			return fmt.Errorf("update %s: %w", slug, err)
 		}
-		updated = append(updated, slug)
+		result.Updated = append(result.Updated, slug)
 	}
+	return nil
+}
 
-	for slug, cur := range existingBySlug {
+// retireOrphanedSystemSkills deletes system rows whose slug is no
+// longer in the bundle. A retirement with a configured replacement
+// (retiredDefaultSkillReplacements) only proceeds once that
+// replacement row actually exists in this workspace: deleting the
+// retired row first would strand every agent that referenced it with
+// no rewritten reference (AC-003.1). When the replacement is missing
+// this pass (e.g. its insert was withheld by a slug conflict above),
+// the retired row is left in place and reported as blocked so a later
+// pass can retry once the conflict resolves.
+//
+// A retired row is removed from existingBySlug along with the DB row:
+// normalizeUserSkillSlugs runs after this pass and treats existingBySlug
+// as the set of slugs currently taken, so a stale entry for a slug this
+// same pass just freed would report a false conflict for a user skill
+// that could otherwise normalize onto it.
+func retireOrphanedSystemSkills(
+	ctx context.Context,
+	repo SystemSyncRepo,
+	wsID string,
+	bundled map[string]SystemSkillSpec,
+	existingBySlug map[string]*models.Skill,
+	result *workspaceSyncResult,
+) error {
+	// The map is a snapshot, but the order of retirement still matters when
+	// one retired slug names another as its replacement. Walk it in a stable
+	// order so the outcome does not depend on Go's map iteration seed.
+	slugs := make([]string, 0, len(existingBySlug))
+	for slug := range existingBySlug {
+		slugs = append(slugs, slug)
+	}
+	sort.Strings(slugs)
+	for _, slug := range slugs {
+		cur, present := existingBySlug[slug]
+		if !present {
+			continue
+		}
 		if _, kept := bundled[slug]; kept {
 			continue
 		}
-		if replacement := replacementSystemSkill(existingBySlug, slug); replacement != nil {
+		replacementSlug, hasReplacement := retiredDefaultSkillReplacements[slug]
+		if hasReplacement {
+			if _, bundledReplacement := bundled[replacementSlug]; !bundledReplacement {
+				result.Blocked = append(result.Blocked, slug)
+				continue
+			}
+			replacement, found := existingBySlug[replacementSlug]
+			if !found {
+				result.Blocked = append(result.Blocked, slug)
+				continue
+			}
 			if err := replaceSkillOnAgents(ctx, repo, wsID, cur, replacement); err != nil {
-				return inserted, updated, removed, fmt.Errorf("replace %s: %w", slug, err)
+				return fmt.Errorf("replace %s: %w", slug, err)
 			}
 		}
 		if err := repo.DeleteSkill(ctx, cur.ID); err != nil {
-			return inserted, updated, removed, fmt.Errorf("delete %s: %w", slug, err)
+			return fmt.Errorf("delete %s: %w", slug, err)
 		}
 		if err := detachSkillFromAgents(ctx, repo, wsID, cur.ID); err != nil {
-			return inserted, updated, removed, fmt.Errorf("detach %s: %w", slug, err)
+			return fmt.Errorf("detach %s: %w", slug, err)
 		}
-		removed = append(removed, slug)
+		delete(existingBySlug, slug)
+		result.Removed = append(result.Removed, slug)
 	}
-	return inserted, updated, removed, nil
+	return nil
 }
 
-func replacementSystemSkill(skills map[string]*models.Skill, retiredSlug string) *models.Skill {
-	replacementSlug, ok := retiredDefaultSkillReplacements[retiredSlug]
-	if !ok {
-		return nil
+// normalizeUserSkillSlugs rewrites a well-formed but non-canonical
+// user/provider skill slug to its canonical kandev-prefixed form
+// (AC-003.8). The row ID is preserved, so skill_ids needs no
+// rewrite — only slug-keyed desired_skills references are updated. A
+// not-well-formed slug (a legacy artifact predating write-time
+// validation) is left untouched and logged, never normalized
+// (AC-003.9). A normalized value already held by another row is left
+// untouched on both sides and logged as a conflict.
+//
+// The repository performs the row rename and agent-reference rewrite in
+// one transaction. This prevents a concurrent writer from claiming the
+// canonical slug after the snapshot and leaving references pointed at a
+// different row, while also preserving retryability after a failed write.
+func normalizeUserSkillSlugs(
+	ctx context.Context,
+	repo SystemSyncRepo,
+	wsID string,
+	nonSystem []*models.Skill,
+	existingBySlug map[string]*models.Skill,
+	nonSystemBySlug map[string]*models.Skill,
+	result *workspaceSyncResult,
+	log *logger.Logger,
+) error {
+	taken := make(map[string]bool, len(existingBySlug)+len(nonSystemBySlug))
+	for slug := range existingBySlug {
+		taken[slug] = true
 	}
-	return skills[replacementSlug]
+	for slug := range nonSystemBySlug {
+		taken[slug] = true
+	}
+
+	for _, row := range nonSystem {
+		if skillslug.Canonical(row.Slug) {
+			continue
+		}
+		if !skillslug.WellFormed(row.Slug) {
+			log.Warn("system skill sync: leaving not-well-formed user skill slug untouched",
+				zap.String("workspace_id", wsID), zap.String("skill_id", row.ID), zap.String("slug", row.Slug))
+			continue
+		}
+		normalized := skillslug.Normalize(row.Slug)
+		if taken[normalized] {
+			log.Warn("system skill sync: leaving conflicting user skill slug unnormalized",
+				zap.String("workspace_id", wsID), zap.String("skill_id", row.ID),
+				zap.String("slug", row.Slug), zap.String("normalized_slug", normalized))
+			result.Conflicted = append(result.Conflicted, row.Slug)
+			continue
+		}
+		oldSlug := row.Slug
+		changed, err := repo.NormalizeSkillSlug(ctx, wsID, row.ID, oldSlug, normalized)
+		if err != nil {
+			if isSlugUniqueConstraintErr(err) {
+				result.Conflicted = append(result.Conflicted, oldSlug)
+				continue
+			}
+			return fmt.Errorf("normalize slug %s: %w", oldSlug, err)
+		}
+		if !changed {
+			continue
+		}
+		taken[normalized] = true
+		result.Normalized = append(result.Normalized, oldSlug+"->"+normalized)
+	}
+	return nil
+}
+
+// isSlugUniqueConstraintErr reports whether err is a UNIQUE constraint
+// violation for a skill slug. SQLite's driver requires string matching,
+// while pgx exposes a typed SQLSTATE 23505 error. The PostgreSQL
+// constraint-name check avoids treating an unrelated duplicate (for
+// example, a primary-key collision) as a slug conflict.
+func isSlugUniqueConstraintErr(err error) bool {
+	if err == nil {
+		return false
+	}
+	var pgErr *pgconn.PgError
+	if errors.As(err, &pgErr) {
+		return pgErr.Code == "23505" &&
+			(pgErr.ConstraintName == "" || pgErr.ConstraintName == "office_skills_workspace_id_slug_key")
+	}
+	msg := err.Error()
+	return strings.Contains(msg, "UNIQUE constraint failed") ||
+		strings.Contains(msg, "constraint failed: UNIQUE")
 }
 
 func replaceSkillOnAgents(
@@ -273,13 +535,22 @@ func systemSkillUpToDate(cur *models.Skill, spec SystemSkillSpec) bool {
 		cur.SystemVersion == spec.Version
 }
 
+// replaceJSONArrayValue rewrites every occurrence of oldValue to
+// newValue in a JSON-array-encoded agent_profiles column, also
+// de-duplicating and dropping empties. raw may also be a legacy
+// comma-separated value (see ParseDesiredSlugs in injection.go, which
+// reads both formats); a legacy value is parsed the same way and, if
+// changed, re-encoded as the canonical JSON-array form — this is how
+// a legacy desired_skills column migrates format the next time a
+// rename or replacement touches one of its entries. An unchanged
+// legacy value is left exactly as found (no unnecessary rewrite).
 func replaceJSONArrayValue(raw, oldValue, newValue string) (string, bool) {
 	raw = strings.TrimSpace(raw)
 	if raw == "" || raw == "null" {
 		return raw, false
 	}
-	var values []string
-	if err := json.Unmarshal([]byte(raw), &values); err != nil {
+	values, ok := parseJSONOrLegacyCSVArray(raw)
+	if !ok {
 		return raw, false
 	}
 	out := make([]string, 0, len(values))
@@ -309,6 +580,28 @@ func replaceJSONArrayValue(raw, oldValue, newValue string) (string, bool) {
 	return string(encoded), true
 }
 
+// parseJSONOrLegacyCSVArray parses raw as a JSON string array, falling
+// back to splitting it as a legacy comma-separated value when raw
+// isn't a JSON array. Mirrors ParseDesiredSlugs's format detection
+// (injection.go) so both readers of these columns agree on what
+// counts as "legacy". Returns ok=false only for malformed JSON input
+// (a corrupt column stays a safe no-op for the caller).
+func parseJSONOrLegacyCSVArray(raw string) ([]string, bool) {
+	if strings.HasPrefix(raw, "[") {
+		var values []string
+		if err := json.Unmarshal([]byte(raw), &values); err != nil {
+			return nil, false
+		}
+		return values, true
+	}
+	parts := strings.Split(raw, ",")
+	out := make([]string, len(parts))
+	for i, p := range parts {
+		out[i] = strings.TrimSpace(p)
+	}
+	return out, true
+}
+
 // removeIDFromJSONArray parses a JSON-array string, removes every
 // occurrence of `id`, and returns the re-encoded array along with a
 // flag indicating whether anything was removed. Malformed input is

apps/backend/internal/office/skills/system_sync_migration_test.go

@@ -0,0 +1,574 @@
+package skills_test
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
+	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/office/models"
+	"github.com/kandev/kandev/internal/office/skills"
+)
+
+// TestSyncSystemSkills_ReplacesRetiredMemorySkillReference pins the
+// concrete REQ-003 migration case this feature adds: once the bundled
+// `memory` skill is renamed to `kandev-memory` for naming-scheme
+// consistency (pending, tracked separately), a workspace with the old
+// row must retire it and end up with the new canonical slug in its
+// place. This test injects a synthetic bundled spec to exercise that
+// path ahead of the real rename.
+func TestSyncSystemSkills_ReplacesRetiredMemorySkillReference(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"memory": {
+			ID:          "skill-old-memory",
+			WorkspaceID: "ws-1",
+			Slug:        "memory",
+			Name:        "Memory",
+			IsSystem:    true,
+			SourceType:  skills.SourceTypeSystem,
+		},
+	}
+
+	bundled := []skills.SystemSkillSpec{{
+		Slug:        "kandev-memory",
+		Name:        "Memory",
+		Description: "Memory guidance",
+		Version:     "1.0.0",
+		Content:     "---\nname: kandev-memory\ndescription: Memory guidance\n---\n# Memory\n",
+		ContentHash: "hash-kandev-memory",
+	}}
+
+	report, err := skills.SyncSystemSkills(context.Background(), repo, []string{"ws-1"}, bundled, log)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Removed) != 1 || !strings.HasSuffix(report.Removed[0], "memory") {
+		t.Fatalf("expected retirement of memory, got removed=%v", report.Removed)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "memory"); err == nil {
+		t.Error("retired memory row still present after sync")
+	}
+	got, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-memory")
+	if err != nil {
+		t.Fatalf("replacement kandev-memory missing: %v", err)
+	}
+	if !got.IsSystem {
+		t.Error("kandev-memory replacement missing is_system flag")
+	}
+}
+
+// TestSyncSystemSkills_RetirementBlockedWhenReplacementMissing pins the
+// F19-round fix: a retired skill with a configured replacement must
+// not be deleted (and its agent references detached) until the
+// replacement row actually exists in the workspace. Deleting first
+// would strand every agent that referenced it with no rewritten
+// reference. When the replacement did not land this pass, the row
+// stays in place and is reported as blocked for a later pass to
+// retry.
+func TestSyncSystemSkills_RetirementBlockedWhenReplacementMissing(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	const oldTasksID = "skill-old-tasks"
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"kandev-tasks": {
+			ID:          oldTasksID,
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-tasks",
+			Name:        "Tasks",
+			IsSystem:    true,
+			SourceType:  skills.SourceTypeSystem,
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			SkillIDs:      mustJSONArray(t, []string{oldTasksID}),
+			DesiredSkills: mustJSONArray(t, []string{"kandev-tasks"}),
+		},
+	}
+
+	// kandev-task-ops (the configured replacement for kandev-tasks) is
+	// neither an existing row nor part of this pass's bundle, so it
+	// cannot be created this pass either.
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Removed) != 0 {
+		t.Fatalf("expected no removal while replacement is missing, got %v", report.Removed)
+	}
+	if len(report.Blocked) != 1 || !strings.HasSuffix(report.Blocked[0], "kandev-tasks") {
+		t.Fatalf("expected kandev-tasks reported as blocked, got %v", report.Blocked)
+	}
+
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-tasks"); err != nil {
+		t.Error("blocked retirement must leave the office_skills row in place")
+	}
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.SkillIDs); len(got) != 1 || got[0] != oldTasksID {
+		t.Errorf("blocked retirement must leave agent skill_ids untouched, got %v", got)
+	}
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != "kandev-tasks" {
+		t.Errorf("blocked retirement must leave agent desired_skills untouched, got %v", got)
+	}
+}
+
+// TestSyncSystemSkills_RetirementBlockedWhenReplacementIsNotBundled ensures
+// an old system row is not retired merely because a replacement row happens
+// to exist in the database. The replacement must be part of the current
+// bundle; otherwise this pass can delete both rows and strand references.
+func TestSyncSystemSkills_RetirementBlockedWhenReplacementIsNotBundled(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	const oldTasksID = "skill-old-tasks"
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"kandev-tasks": {
+			ID:          oldTasksID,
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-tasks",
+			Name:        "Tasks",
+			IsSystem:    true,
+			SourceType:  skills.SourceTypeSystem,
+		},
+		"kandev-task-ops": {
+			ID:          "skill-task-ops",
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-task-ops",
+			Name:        "Task operations",
+			IsSystem:    true,
+			SourceType:  skills.SourceTypeSystem,
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			SkillIDs:      mustJSONArray(t, []string{oldTasksID}),
+			DesiredSkills: mustJSONArray(t, []string{"kandev-tasks"}),
+		},
+	}
+
+	// Neither existing row is present in this pass's bundle. The configured
+	// replacement is therefore not a valid destination for retirement.
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Blocked) != 1 || !strings.HasSuffix(report.Blocked[0], "kandev-tasks") {
+		t.Fatalf("expected kandev-tasks reported as blocked, got %v", report.Blocked)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-tasks"); err != nil {
+		t.Fatalf("blocked retired row must remain: %v", err)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-task-ops"); err == nil {
+		t.Fatal("unbundled replacement row should be retired independently")
+	}
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.SkillIDs); len(got) != 1 || got[0] != oldTasksID {
+		t.Errorf("blocked retirement must leave agent skill_ids untouched, got %v", got)
+	}
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != "kandev-tasks" {
+		t.Errorf("blocked retirement must leave agent desired_skills untouched, got %v", got)
+	}
+}
+
+// TestSyncSystemSkills_BundledInsertConflictsWithExistingUserSkill pins
+// AC-003.3: a bundled slug not yet backed by a system row, but already
+// held by a non-system (user or provider-imported) row, must not be
+// inserted over that row. It is skipped and recorded as a conflict so
+// the mismatch is visible without silently overwriting user data.
+func TestSyncSystemSkills_BundledInsertConflictsWithExistingUserSkill(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"kandev-custom-thing": {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-custom-thing",
+			Name:        "My Custom Thing",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+
+	bundled := []skills.SystemSkillSpec{{
+		Slug:        "kandev-custom-thing",
+		Name:        "Custom Thing",
+		Description: "Bundled version",
+		Version:     "1.0.0",
+		Content:     "---\nname: kandev-custom-thing\ndescription: Bundled version\n---\n# Custom Thing\n",
+		ContentHash: "hash-custom-thing",
+	}}
+
+	report, err := skills.SyncSystemSkills(context.Background(), repo, []string{"ws-1"}, bundled, log)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Inserted) != 0 {
+		t.Fatalf("expected no insert over an existing user skill, got %v", report.Inserted)
+	}
+	if len(report.Conflicted) != 1 || !strings.HasSuffix(report.Conflicted[0], "kandev-custom-thing") {
+		t.Fatalf("expected kandev-custom-thing reported as conflicted, got %v", report.Conflicted)
+	}
+	got, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-custom-thing")
+	if err != nil {
+		t.Fatalf("existing user row missing after sync: %v", err)
+	}
+	if got.IsSystem {
+		t.Error("existing user row must not become a system row")
+	}
+	if got.ID != "user-skill-1" {
+		t.Errorf("existing user row ID changed: now %s", got.ID)
+	}
+}
+
+// TestSyncSystemSkills_NormalizesWellFormedUserSlugToCanonical pins
+// AC-003.8: a well-formed but non-canonical user/provider skill slug
+// is rewritten to its canonical kandev- prefixed form in place,
+// preserving the row ID, and every agent's desired_skills reference
+// is rewritten to match.
+func TestSyncSystemSkills_NormalizesWellFormedUserSlugToCanonical(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"my-custom-skill": {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        "my-custom-skill",
+			Name:        "My Custom Skill",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			DesiredSkills: mustJSONArray(t, []string{"my-custom-skill"}),
+		},
+	}
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 1 || !strings.HasSuffix(report.Normalized[0], "my-custom-skill->kandev-my-custom-skill") {
+		t.Fatalf("expected my-custom-skill normalized, got %v", report.Normalized)
+	}
+
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "my-custom-skill"); err == nil {
+		t.Error("old slug should no longer resolve after normalization")
+	}
+	got, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-my-custom-skill")
+	if err != nil {
+		t.Fatalf("normalized row missing: %v", err)
+	}
+	if got.ID != "user-skill-1" {
+		t.Errorf("normalization must preserve row ID, got %s", got.ID)
+	}
+
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != "kandev-my-custom-skill" {
+		t.Errorf("agent-1.desired_skills = %v, want [kandev-my-custom-skill]", got)
+	}
+}
+
+// TestSyncSystemSkills_LeavesNotWellFormedUserSlugUntouched pins
+// AC-003.9: a legacy row whose slug predates write-time validation
+// and is not well-formed (contains characters outside the safe path
+// component set) must never be normalized — there is no safe
+// canonical rewrite for it, so the sync leaves it exactly as it was
+// found.
+func TestSyncSystemSkills_LeavesNotWellFormedUserSlugUntouched(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	const badSlug = "legacy slug!"
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		badSlug: {
+			ID:          "user-skill-legacy",
+			WorkspaceID: "ws-1",
+			Slug:        badSlug,
+			Name:        "Legacy",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 0 {
+		t.Fatalf("not-well-formed slug must never be normalized, got %v", report.Normalized)
+	}
+	if len(report.Conflicted) != 0 {
+		t.Fatalf("not-well-formed slug is not a conflict, got %v", report.Conflicted)
+	}
+	got, err := repo.GetSkillBySlug(context.Background(), "ws-1", badSlug)
+	if err != nil {
+		t.Fatalf("legacy row missing after sync: %v", err)
+	}
+	if got.Slug != badSlug {
+		t.Errorf("legacy row slug changed: now %q", got.Slug)
+	}
+}
+
+// TestSyncSystemSkills_LeavesConflictingNormalizedUserSlugUntouched
+// pins the collision branch of AC-003.8: when the canonical form a
+// well-formed slug would normalize to is already held by another row,
+// neither row is touched and the collision is reported so an operator
+// can resolve it manually.
+func TestSyncSystemSkills_LeavesConflictingNormalizedUserSlugUntouched(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"my-thing": {
+			ID:          "user-skill-a",
+			WorkspaceID: "ws-1",
+			Slug:        "my-thing",
+			Name:        "My Thing (legacy)",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+		"kandev-my-thing": {
+			ID:          "user-skill-b",
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-my-thing",
+			Name:        "My Thing (canonical)",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 0 {
+		t.Fatalf("colliding slug must not be normalized, got %v", report.Normalized)
+	}
+	if len(report.Conflicted) != 1 || !strings.HasSuffix(report.Conflicted[0], "my-thing") {
+		t.Fatalf("expected my-thing reported as conflicted, got %v", report.Conflicted)
+	}
+
+	a, err := repo.GetSkillBySlug(context.Background(), "ws-1", "my-thing")
+	if err != nil || a.ID != "user-skill-a" {
+		t.Errorf("legacy row must stay at its original slug, got %+v err=%v", a, err)
+	}
+	b, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-my-thing")
+	if err != nil || b.ID != "user-skill-b" {
+		t.Errorf("canonical row must be untouched, got %+v err=%v", b, err)
+	}
+}
+
+// TestSyncSystemSkills_NormalizationRetriesAfterFailedReferenceRewrite
+// pins the recovery contract in
+// docs/specs/agents/system-design/injected-skill-naming-migration.md
+// ("## Failure and recovery"): a normalization interrupted between the
+// row rename and the agent-reference rewrite must be fully retried on
+// the next pass, not silently left half-done. If the row were renamed
+// to its canonical slug before the agent-reference rewrite is
+// confirmed, a later pass would see the row as already canonical and
+// skip it forever, stranding the stale agent reference.
+func TestSyncSystemSkills_NormalizationRetriesAfterFailedReferenceRewrite(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"my-custom-skill": {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        "my-custom-skill",
+			Name:        "My Custom Skill",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			DesiredSkills: mustJSONArray(t, []string{"my-custom-skill"}),
+		},
+	}
+	repo.failUpdateAgentFor["agent-1"] = true
+
+	// SyncSystemSkills logs and continues past a per-workspace failure
+	// (so one bad workspace doesn't block the rest) rather than
+	// returning it, so the top-level call succeeds; the failure is
+	// checked via the row/agent state left behind instead.
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 0 {
+		t.Fatalf("normalization must not be reported while the reference rewrite failed, got %v", report.Normalized)
+	}
+
+	// The row must NOT have been committed to its canonical slug: a
+	// half-done normalization (row renamed, reference not rewritten)
+	// is exactly the state the recovery contract forbids, because the
+	// canonical-slug gate would then skip this row on every later pass.
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "my-custom-skill"); err != nil {
+		t.Fatalf("row must stay at its original slug after a failed rewrite, got err=%v", err)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-my-custom-skill"); err == nil {
+		t.Fatal("row must not be committed to its canonical slug before the reference rewrite succeeds")
+	}
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != "my-custom-skill" {
+		t.Errorf("agent-1.desired_skills must stay untouched after a failed rewrite, got %v", got)
+	}
+
+	// A later pass, with the transient failure gone, must fully
+	// converge: this is the "next pass repeats the reference rewrite"
+	// half of the recovery contract.
+	repo.failUpdateAgentFor["agent-1"] = false
+	report, err = skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("retry sync: %v", err)
+	}
+	if len(report.Normalized) != 1 || !strings.HasSuffix(report.Normalized[0], "my-custom-skill->kandev-my-custom-skill") {
+		t.Fatalf("expected my-custom-skill normalized on retry, got %v", report.Normalized)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-my-custom-skill"); err != nil {
+		t.Fatalf("row must be canonical after the retry succeeds: %v", err)
+	}
+	agent1 = repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != "kandev-my-custom-skill" {
+		t.Errorf("agent-1.desired_skills = %v, want [kandev-my-custom-skill] after retry", got)
+	}
+}
+
+// TestSyncSystemSkills_RetiredSystemSlugFreedForSamePassNormalization
+// pins a sequencing edge in syncWorkspace: retireOrphanedSystemSkills
+// runs before normalizeUserSkillSlugs, and a system row it retires in
+// this same pass must not block a user skill from normalizing onto
+// that now-freed slug. Treating the just-deleted row as still "taken"
+// would report a false conflict for a slug that is actually free.
+func TestSyncSystemSkills_RetiredSystemSlugFreedForSamePassNormalization(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		// A stale system row for a slug no longer in the bundle, with
+		// no configured replacement, so it retires outright this pass.
+		"kandev-legacy-widget": {
+			ID:          "system-skill-legacy",
+			WorkspaceID: "ws-1",
+			Slug:        "kandev-legacy-widget",
+			Name:        "Legacy Widget",
+			IsSystem:    true,
+			SourceType:  skills.SourceTypeSystem,
+		},
+		// A user skill whose canonical target is exactly the slug the
+		// system row above is retiring in this same pass.
+		"legacy-widget": {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        "legacy-widget",
+			Name:        "My Legacy Widget",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Removed) != 1 || !strings.HasSuffix(report.Removed[0], "kandev-legacy-widget") {
+		t.Fatalf("expected kandev-legacy-widget retired, got removed=%v", report.Removed)
+	}
+	if len(report.Conflicted) != 0 {
+		t.Fatalf("slug freed by same-pass retirement must not be reported as conflicted, got %v", report.Conflicted)
+	}
+	if len(report.Normalized) != 1 || !strings.HasSuffix(report.Normalized[0], "legacy-widget->kandev-legacy-widget") {
+		t.Fatalf("expected legacy-widget normalized onto the freed slug, got %v", report.Normalized)
+	}
+	got, err := repo.GetSkillBySlug(context.Background(), "ws-1", "kandev-legacy-widget")
+	if err != nil {
+		t.Fatalf("normalized row missing: %v", err)
+	}
+	if got.ID != "user-skill-1" {
+		t.Errorf("normalization must preserve the user row's ID, got %s", got.ID)
+	}
+	if got.IsSystem {
+		t.Error("normalized user row must not become a system row")
+	}
+}
+
+// TestSyncSystemSkills_RenameRewritesLegacyCommaSeparatedDesiredSkills
+// pins a format gap in the agent-reference rewrite: desired_skills
+// predates the JSON-array persistence format on some rows and is
+// still a legacy comma-separated string (ParseDesiredSlugs reads
+// both; see injection.go). A slug rename must rewrite that legacy
+// value too — otherwise the agent is left holding a reference to a
+// slug that no longer exists once the row is renamed to its
+// canonical form.
+func TestSyncSystemSkills_RenameRewritesLegacyCommaSeparatedDesiredSkills(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		"my-custom-skill": {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        "my-custom-skill",
+			Name:        "My Custom Skill",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			DesiredSkills: "my-custom-skill,other-skill",
+		},
+	}
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 1 || !strings.HasSuffix(report.Normalized[0], "my-custom-skill->kandev-my-custom-skill") {
+		t.Fatalf("expected my-custom-skill normalized, got %v", report.Normalized)
+	}
+
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	got := decodeIDs(t, agent1.DesiredSkills)
+	want := []string{"kandev-my-custom-skill", "other-skill"}
+	if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] {
+		t.Errorf("agent-1.desired_skills = %q (parsed %v), want %v", agent1.DesiredSkills, got, want)
+	}
+}

apps/backend/internal/office/skills/system_sync_race_test.go

@@ -0,0 +1,217 @@
+package skills_test
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/jackc/pgx/v5/pgconn"
+	settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
+	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/office/models"
+	"github.com/kandev/kandev/internal/office/skills"
+)
+
+var syncTestHooks = struct {
+	sync.Mutex
+	createSkillErr       map[*stubSyncRepo]map[string]error
+	normalizeConflictFor map[*stubSyncRepo]map[string]bool
+}{
+	createSkillErr:       map[*stubSyncRepo]map[string]error{},
+	normalizeConflictFor: map[*stubSyncRepo]map[string]bool{},
+}
+
+func syncTestCreateSkillError(repo *stubSyncRepo, slug string) error {
+	syncTestHooks.Lock()
+	defer syncTestHooks.Unlock()
+	return syncTestHooks.createSkillErr[repo][slug]
+}
+
+func setSyncTestCreateSkillError(repo *stubSyncRepo, slug string, err error) {
+	syncTestHooks.Lock()
+	defer syncTestHooks.Unlock()
+	if syncTestHooks.createSkillErr[repo] == nil {
+		syncTestHooks.createSkillErr[repo] = map[string]error{}
+	}
+	syncTestHooks.createSkillErr[repo][slug] = err
+}
+
+func setSyncTestNormalizeConflict(repo *stubSyncRepo, slug string) {
+	syncTestHooks.Lock()
+	defer syncTestHooks.Unlock()
+	if syncTestHooks.normalizeConflictFor[repo] == nil {
+		syncTestHooks.normalizeConflictFor[repo] = map[string]bool{}
+	}
+	syncTestHooks.normalizeConflictFor[repo][slug] = true
+}
+
+func syncTestNormalizeConflict(repo *stubSyncRepo, slug string) bool {
+	syncTestHooks.Lock()
+	defer syncTestHooks.Unlock()
+	return syncTestHooks.normalizeConflictFor[repo][slug]
+}
+
+// NormalizeSkillSlug mirrors the repository contract: the slug row and
+// every affected desired_skills profile are committed as one operation.
+// The in-memory implementation applies all changes only after it has
+// confirmed that every profile update can succeed.
+func (s *stubSyncRepo) NormalizeSkillSlug(
+	_ context.Context, workspaceID, skillID, oldSlug, newSlug string,
+) (bool, error) {
+	ws := s.rows[workspaceID]
+	row, ok := ws[oldSlug]
+	if !ok || row.ID != skillID || row.IsSystem {
+		return false, nil
+	}
+	if syncTestNormalizeConflict(s, oldSlug) {
+		ws[newSlug] = &models.Skill{
+			ID:          "concurrent-" + newSlug,
+			WorkspaceID: workspaceID,
+			Slug:        newSlug,
+			IsSystem:    false,
+		}
+		return false, &pgconn.PgError{
+			Code:           "23505",
+			ConstraintName: "office_skills_workspace_id_slug_key",
+		}
+	}
+	if _, exists := ws[newSlug]; exists {
+		return false, &pgconn.PgError{
+			Code:           "23505",
+			ConstraintName: "office_skills_workspace_id_slug_key",
+		}
+	}
+
+	type agentUpdate struct {
+		id      string
+		desired string
+	}
+	updates := make([]agentUpdate, 0)
+	for id, agent := range s.agents[workspaceID] {
+		desired, changed := replaceStubJSONArrayValue(agent.DesiredSkills, oldSlug, newSlug)
+		if !changed {
+			continue
+		}
+		if s.failUpdateAgentFor[id] {
+			return false, errors.New("injected failure: agent update unavailable")
+		}
+		updates = append(updates, agentUpdate{id: id, desired: desired})
+	}
+
+	delete(ws, oldSlug)
+	copyRow := *row
+	copyRow.Slug = newSlug
+	ws[newSlug] = &copyRow
+	for _, update := range updates {
+		copyAgent := *s.agents[workspaceID][update.id]
+		copyAgent.DesiredSkills = update.desired
+		s.agents[workspaceID][update.id] = &copyAgent
+	}
+	return true, nil
+}
+
+func replaceStubJSONArrayValue(raw, oldValue, newValue string) (string, bool) {
+	values := skills.ParseDesiredSlugs(raw)
+	if len(values) == 0 {
+		return raw, false
+	}
+	out := make([]string, 0, len(values))
+	seen := make(map[string]bool, len(values))
+	changed := false
+	for _, value := range values {
+		if value == oldValue {
+			value = newValue
+			changed = true
+		}
+		if value == "" || seen[value] {
+			if value != "" {
+				changed = true
+			}
+			continue
+		}
+		seen[value] = true
+		out = append(out, value)
+	}
+	if !changed {
+		return raw, false
+	}
+	encoded, err := json.Marshal(out)
+	if err != nil {
+		return raw, false
+	}
+	return string(encoded), true
+}
+
+func TestSyncSystemSkills_NormalizationConflictLeavesReferencesUntouched(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+
+	const oldSlug = "my-custom-skill"
+	repo.rows["ws-1"] = map[string]*models.Skill{
+		oldSlug: {
+			ID:          "user-skill-1",
+			WorkspaceID: "ws-1",
+			Slug:        oldSlug,
+			Name:        "My Custom Skill",
+			IsSystem:    false,
+			SourceType:  "inline",
+		},
+	}
+	repo.agents["ws-1"] = map[string]*settingsmodels.AgentProfile{
+		"agent-1": {
+			ID:            "agent-1",
+			WorkspaceID:   "ws-1",
+			DesiredSkills: mustJSONArray(t, []string{oldSlug}),
+		},
+	}
+	setSyncTestNormalizeConflict(repo, oldSlug)
+
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, []skills.SystemSkillSpec{}, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Normalized) != 0 {
+		t.Fatalf("conflicting normalization must not be reported as complete: %v", report.Normalized)
+	}
+	if len(report.Conflicted) != 1 || !strings.HasSuffix(report.Conflicted[0], oldSlug) {
+		t.Fatalf("expected %s reported as conflicted, got %v", oldSlug, report.Conflicted)
+	}
+	if _, err := repo.GetSkillBySlug(context.Background(), "ws-1", oldSlug); err != nil {
+		t.Fatalf("original user row must remain after conflict: %v", err)
+	}
+	agent1 := repo.agents["ws-1"]["agent-1"]
+	if got := decodeIDs(t, agent1.DesiredSkills); len(got) != 1 || got[0] != oldSlug {
+		t.Errorf("agent desired_skills changed despite atomic conflict: %v", got)
+	}
+}
+
+func TestSyncSystemSkills_ContinuesAfterPostgresSlugConflict(t *testing.T) {
+	repo := newStubSyncRepo()
+	log := logger.Default()
+	setSyncTestCreateSkillError(repo, "kandev-conflicting", &pgconn.PgError{
+		Code:           "23505",
+		ConstraintName: "office_skills_workspace_id_slug_key",
+	})
+
+	bundled := []skills.SystemSkillSpec{
+		{Slug: "kandev-conflicting", Name: "Conflict", ContentHash: "hash-conflict"},
+		{Slug: "kandev-next", Name: "Next", ContentHash: "hash-next"},
+	}
+	report, err := skills.SyncSystemSkills(
+		context.Background(), repo, []string{"ws-1"}, bundled, log,
+	)
+	if err != nil {
+		t.Fatalf("sync: %v", err)
+	}
+	if len(report.Conflicted) != 1 || !strings.HasSuffix(report.Conflicted[0], "kandev-conflicting") {
+		t.Fatalf("expected PostgreSQL slug conflict to be reported, got %v", report.Conflicted)
+	}
+	if len(report.Inserted) != 1 || !strings.HasSuffix(report.Inserted[0], "kandev-next") {
+		t.Fatalf("sync should continue after PostgreSQL conflict, inserted=%v", report.Inserted)
+	}
+}

apps/backend/internal/office/skills/system_sync_test.go

@@ -21,12 +21,19 @@ import (
 type stubSyncRepo struct {
 	rows   map[string]map[string]*models.Skill                // workspaceID → slug → row
 	agents map[string]map[string]*settingsmodels.AgentProfile // workspaceID → agentID → profile
+
+	// failUpdateAgentFor injects an error from UpdateAgentInstance for
+	// the named agent ID, so a test can simulate a mid-sequence failure
+	// (e.g. the agent-reference rewrite half of a slug normalization)
+	// without it actually persisting.
+	failUpdateAgentFor map[string]bool
 }
 
 func newStubSyncRepo() *stubSyncRepo {
 	return &stubSyncRepo{
-		rows:   map[string]map[string]*models.Skill{},
-		agents: map[string]map[string]*settingsmodels.AgentProfile{},
+		rows:               map[string]map[string]*models.Skill{},
+		agents:             map[string]map[string]*settingsmodels.AgentProfile{},
+		failUpdateAgentFor: map[string]bool{},
 	}
 }
 
@@ -44,6 +51,20 @@ func (s *stubSyncRepo) ListSystemSkills(
 	return out, nil
 }
 
+func (s *stubSyncRepo) ListNonSystemSkills(
+	_ context.Context, workspaceID string,
+) ([]*models.Skill, error) {
+	ws := s.rows[workspaceID]
+	out := make([]*models.Skill, 0, len(ws))
+	for _, sk := range ws {
+		if !sk.IsSystem {
+			out = append(out, sk)
+		}
+	}
+	sort.Slice(out, func(i, j int) bool { return out[i].Slug < out[j].Slug })
+	return out, nil
+}
+
 func (s *stubSyncRepo) GetSkillBySlug(
 	_ context.Context, workspaceID, slug string,
 ) (*models.Skill, error) {
@@ -56,6 +77,9 @@ func (s *stubSyncRepo) GetSkillBySlug(
 }
 
 func (s *stubSyncRepo) CreateSkill(_ context.Context, skill *models.Skill) error {
+	if err := syncTestCreateSkillError(s, skill.Slug); err != nil {
+		return err
+	}
 	if _, ok := s.rows[skill.WorkspaceID]; !ok {
 		s.rows[skill.WorkspaceID] = map[string]*models.Skill{}
 	}
@@ -67,9 +91,19 @@ func (s *stubSyncRepo) CreateSkill(_ context.Context, skill *models.Skill) error
 	return nil
 }
 
+// UpdateSkill mirrors the real repository's update-by-ID semantics
+// (sqlite.Repository.UpdateSkill writes `WHERE id = ?`): it locates
+// the row by ID, not by the caller-supplied slug, so a slug rewrite
+// (e.g. normalizeUserSkillSlugs) re-keys the map instead of failing
+// to find a row under its now-stale old slug.
 func (s *stubSyncRepo) UpdateSkill(_ context.Context, skill *models.Skill) error {
-	if ws, ok := s.rows[skill.WorkspaceID]; ok {
-		if _, ok := ws[skill.Slug]; ok {
+	ws, ok := s.rows[skill.WorkspaceID]
+	if !ok {
+		return errors.New("not found for update")
+	}
+	for slug, sk := range ws {
+		if sk.ID == skill.ID {
+			delete(ws, slug)
 			copy := *skill
 			ws[skill.Slug] = &copy
 			return nil
@@ -90,13 +124,18 @@ func (s *stubSyncRepo) DeleteSkill(_ context.Context, id string) error {
 	return errors.New("not found for delete")
 }
 
+// ListAgentInstances returns copies, matching the real repository's
+// read-from-DB semantics: a caller mutating a returned profile (as
+// renameSkillSlugOnAgents does before its UpdateAgentInstance call)
+// must not silently affect stored state until that write succeeds.
 func (s *stubSyncRepo) ListAgentInstances(
 	_ context.Context, workspaceID string,
 ) ([]*settingsmodels.AgentProfile, error) {
 	ws := s.agents[workspaceID]
 	out := make([]*settingsmodels.AgentProfile, 0, len(ws))
 	for _, a := range ws {
-		out = append(out, a)
+		copy := *a
+		out = append(out, &copy)
 	}
 	sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID })
 	return out, nil
@@ -105,6 +144,9 @@ func (s *stubSyncRepo) ListAgentInstances(
 func (s *stubSyncRepo) UpdateAgentInstance(
 	_ context.Context, agent *settingsmodels.AgentProfile,
 ) error {
+	if s.failUpdateAgentFor[agent.ID] {
+		return errors.New("injected failure: agent update unavailable")
+	}
 	ws, ok := s.agents[agent.WorkspaceID]
 	if !ok {
 		return errors.New("workspace not found")

apps/web/e2e/tests/office/skill-injection.spec.ts

@@ -30,11 +30,15 @@ test("skill injection writes assigned skill to session worktree on launch", asyn
     slug,
     content,
   })) as { id: string; slug: string };
-  expect(skill.slug).toBe(slug);
+  // A caller-supplied slug is normalized to its canonical kandev-prefixed
+  // form on write (AC-001.12); the persisted identity downstream is this
+  // canonical slug, not the one the request supplied.
+  const canonicalSlug = `kandev-${slug}`;
+  expect(skill.slug).toBe(canonicalSlug);
 
   // 2. Assign the skill to the seed agent profile via the test harness.
   //    desired_skills is read from agent_profiles at launch time.
-  await apiClient.setProfileDesiredSkills(seedData.agentProfileId, [slug]);
+  await apiClient.setProfileDesiredSkills(seedData.agentProfileId, [canonicalSlug]);
 
   try {
     // 3. Launch a session via createTaskWithAgent (start_agent: true).
@@ -65,12 +69,14 @@ test("skill injection writes assigned skill to session worktree on launch", asyn
       .not.toBe("");
 
     // 5. Skill landed at the spec-defined location with the right content.
-    const skillFile = path.join(worktreePath, PROJECT_SKILL_DIR, `kandev-${slug}`, "SKILL.md");
+    const skillFile = path.join(worktreePath, PROJECT_SKILL_DIR, canonicalSlug, "SKILL.md");
     await expect
       .poll(() => fs.existsSync(skillFile), { timeout: 15_000, message: skillFile })
       .toBe(true);
     const skillFileContent = fs.readFileSync(skillFile, "utf8");
-    expect(skillFileContent).toContain(`---\nname: ${slug}\ndescription: ${slug}\n---\n`);
+    expect(skillFileContent).toContain(
+      `---\nname: ${canonicalSlug}\ndescription: ${canonicalSlug}\n---\n`,
+    );
     expect(skillFileContent).toContain(`Marker: ${slug}-content`);
 
     // 6. .git/info/exclude (resolved through .git file for linked worktrees)

apps/web/e2e/tests/office/skills-readonly.spec.ts

@@ -49,7 +49,9 @@ test.describe("Office skills - Monaco read-only mode", () => {
       slug: unique,
       content: "# Editable\n\nWorkspace-scope skill content.\n",
     })) as { id?: string; slug?: string };
-    expect(created.slug).toBe(unique);
+    // A caller-supplied slug is normalized to its canonical kandev-prefixed
+    // form on write (AC-001.12).
+    expect(created.slug).toBe(`kandev-${unique}`);
 
     await testPage.goto("/office/workspace/skills");
 

apps/web/e2e/tests/office/skills.spec.ts

@@ -10,7 +10,9 @@ test.describe("Skills", () => {
       slug: "test-skill",
       content: "# Test\n\nThis is a test skill.",
     });
-    expect((skill as Record<string, unknown>).slug).toBe("test-skill");
+    // A caller-supplied slug is normalized to its canonical kandev-prefixed
+    // form on write (AC-001.12).
+    expect((skill as Record<string, unknown>).slug).toBe("kandev-test-skill");
 
     const skills = await officeApi.listSkills(officeSeed.workspaceId);
     const list =
@@ -18,7 +20,7 @@ test.describe("Skills", () => {
       (skills as unknown as Record<string, unknown>[]);
     expect(
       Array.isArray(list)
-        ? list.some((s) => (s as Record<string, unknown>).slug === "test-skill")
+        ? list.some((s) => (s as Record<string, unknown>).slug === "kandev-test-skill")
         : false,
     ).toBe(true);
   });
@@ -32,7 +34,7 @@ test.describe("Skills", () => {
     const id = (created as Record<string, unknown>).id as string;
     const fetched = await officeApi.getSkill(id);
     expect((fetched as Record<string, unknown>).id).toBe(id);
-    expect((fetched as Record<string, unknown>).slug).toBe("get-skill");
+    expect((fetched as Record<string, unknown>).slug).toBe("kandev-get-skill");
   });
 
   test("delete skill removes it from list", async ({ officeApi, officeSeed }) => {
@@ -84,7 +86,9 @@ test.describe("Skills", () => {
 
     const imported = await officeApi.importUserHomeSkill(officeSeed.workspaceId, provider, key);
     expect(imported.name).toBe(name);
-    expect(imported.slug).toBe(key);
+    // A discovered provider-skill key is a well-formed slug and gets the
+    // same write-time canonicalization as any other slug (AC-001.12).
+    expect(imported.slug).toBe(`kandev-${key}`);
     expect(imported.source_type).toBe("user_home");
     expect(imported.source_locator).toBe(`${provider}:${key}`);