fix(office): reject empty slug on skill update ↗

Handler passes slugRequested flag

The handler tells the service if the client sent slug, so empty can be rejected and omitted can be healed.

	}
	applySkillUpdates(skill, &req)
	if err := h.svc.ValidateSkillUpdate(ctx, skill); err != nil {
	if err := h.svc.ValidateSkillUpdate(ctx, skill, req.Slug != nil); err != nil {
		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
		return
	}

Service rejects empty slug and heals legacy rows

The service rejects an empty slug when requested and heals an empty stored slug when not requested.

// 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 == "" {
// handling mirrors ValidateAndPrepareSkill: an empty slug is not
// well-formed and is rejected like any other not-well-formed slug when the
// caller supplies one; the caller omits the field entirely (slugRequested
// false) to leave the slug unchanged. A well-formed slug is normalized to
// canonical form before the uniqueness check runs.
//
// A request that never mentions slug must never fail on slug grounds. The
// config-import path can create a row with an empty stored slug outside
// this service's own validation, so an unrelated update to such a row
// heals it to a free name-derived slug rather than rejecting the caller's
// edit; if every candidate collides, the stored empty value is left as-is.
func (s *SkillService) ValidateSkillUpdate(ctx context.Context, skill *models.Skill, slugRequested bool) error {
	if !slugRequested && skill.Slug == "" {
		s.healEmptySlug(ctx, skill)
		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)
}

// healEmptySlug assigns skill a name-derived slug when one is free in its
// workspace, and leaves its stored empty slug untouched otherwise.
func (s *SkillService) healEmptySlug(ctx context.Context, skill *models.Skill) {
	candidate := skillslug.Normalize(GenerateSlug(skill.Name))
	if s.validateSlugUnique(ctx, skill.WorkspaceID, candidate, skill.ID) == nil {
		skill.Slug = candidate
	}
}

New handler tests for PATCH edge cases

The tests lock the HTTP contract: empty and malformed slugs return 400, content-only PATCH heals or preserves slug.

func TestUpdateSkillHandler_RejectsEmptySlug(t *testing.T) {
	router, svc := newTestSkillRouter(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)
	}
	rec := doSkillRequest(t, router, http.MethodPatch, "/api/v1/skills/"+skill.ID, `{"slug":""}`)
	if rec.Code != http.StatusBadRequest {
		t.Fatalf("status: got %d, want %d (body: %s)", rec.Code, http.StatusBadRequest, rec.Body.String())
	}
	reloaded, _ := svc.GetSkillFromConfig(ctx, skill.ID)
	if reloaded.Slug != "kandev-existing" {
		t.Errorf("stored slug = %q, want unchanged", reloaded.Slug)
	}
}

func TestUpdateSkillHandler_ContentOnlyPatchHealsStoredEmptySlug(t *testing.T) {
	router, svc := newTestSkillRouter(t)
	ctx := context.Background()
	skill := &models.Skill{WorkspaceID: "ws-1", Name: "Existing Skill", Slug: "", SourceType: "inline"}
	if err := svc.CreateSkill(ctx, skill); err != nil {
		t.Fatalf("create: %v", err)
	}
	rec := doSkillRequest(t, router, http.MethodPatch, "/api/v1/skills/"+skill.ID, `{"content":"new content"}`)
	if rec.Code != http.StatusOK {
		t.Fatalf("status: got %d, want %d", rec.Code, http.StatusOK)
	}
	var resp skills.SkillResponse
	json.Unmarshal(rec.Body.Bytes(), &resp)
	if resp.Skill.Slug == "" {
		t.Errorf("response slug still empty after content-only PATCH")
	}
}

Service unit tests for slug update

The service tests verify the same rules without HTTP: empty when requested fails, heal when not requested succeeds.

func TestValidateSkillUpdate_RejectsEmptySlug(t *testing.T) {
	svc := newTestSkillService(t)
	ctx := context.Background()
	skill := &models.Skill{WorkspaceID: "ws-1", Name: "Existing", Slug: "kandev-existing", SourceType: "inline"}
	svc.ValidateAndPrepareSkill(ctx, skill)
	svc.CreateSkill(ctx, skill)
	skill.Slug = ""
	if err := svc.ValidateSkillUpdate(ctx, skill, true); err == nil {
		t.Fatal("expected error for empty slug")
	}
}

func TestValidateSkillUpdate_HealsEmptyStoredSlugWhenNotRequested(t *testing.T) {
	svc := newTestSkillService(t)
	ctx := context.Background()
	skill := &models.Skill{WorkspaceID: "ws-1", Name: "Existing Skill", SourceType: "inline"}
	svc.CreateSkill(ctx, skill)
	if err := svc.ValidateSkillUpdate(ctx, skill, false); err != nil {
		t.Fatalf("update: %v", err)
	}
	if skill.Slug == "" {
		t.Error("slug still empty after healing")
	}
}

Stabilize grace timer test on Windows

The test disables git polling loops so the grace callback is the only poll mode change under test.

func TestPollModeGrace_StopJoinsFinalScan(t *testing.T) {
	wt := newGraceTestTracker(t, graceFiresQuickly)
	// The test covers the grace callback's cancellation and wait-group
	// ownership. Disable the independent polling loops so their real Git work
	// cannot change poll mode while this lifecycle boundary is under test (the
	// Windows race runner can otherwise report an unrelated paused transition).
	wt.gitIndexPath = ""
	finalScanStarted := make(chan struct{})
	finalScanFinished := make(chan struct{})
	wt.gitStatusObserver = func(ctx context.Context) (types.GitStatusUpdate, error) {