diff --git a/.doc/doki/doc/custom-status-type.md b/.doc/doki/doc/custom-status-type.md new file mode 100644 index 0000000..24380e6 --- /dev/null +++ b/.doc/doki/doc/custom-status-type.md @@ -0,0 +1,125 @@ +# Custom Statuses and Types + +Statuses and types are user-configurable via `workflow.yaml`. +Both follow the same structural rules with a few differences noted below. + +## Configuration + +### YAML Shape + +```yaml +statuses: + - key: backlog + label: Backlog + emoji: "📥" + default: true + - key: inProgress + label: "In Progress" + emoji: "⚙️" + active: true + - key: done + label: Done + emoji: "✅" + done: true + +types: + - key: story + label: Story + emoji: "🌀" + - key: bug + label: Bug + emoji: "💥" +``` + +### Shared Rules + +These rules apply identically to both `statuses:` and `types:`: + +| Rule | Detail | +|---|---| +| Canonical keys | Keys must already be in canonical form. Non-canonical keys are rejected with a suggested canonical form. | +| Label defaults to key | When `label` is omitted, the key is used as the label. | +| Empty labels rejected | Explicitly empty or whitespace-only labels are invalid. | +| Emoji trimmed | Leading/trailing whitespace is stripped from emoji values. | +| Unique display strings | Each entry must produce a unique `"Label Emoji"` display. Duplicates are rejected. | +| At least one entry | An empty list is invalid. | +| Duplicate keys rejected | Two entries with the same canonical key are invalid. | +| Unknown keys rejected | Only documented keys are allowed in each entry. | + +### Status-Only Keys + +Statuses support additional boolean flags that types do not: + +| Key | Required | Description | +|---|---|---| +| `active` | no | Marks a status as active (in-progress work). | +| `default` | exactly one | The status assigned to newly created tasks. | +| `done` | exactly one | The terminal status representing completion. | + +Valid keys in a status entry: `key`, `label`, `emoji`, `active`, `default`, `done`. + +### Type-Only Behavior + +Types have no boolean flags. The first configured type is used as the creation default. + +Valid keys in a type entry: `key`, `label`, `emoji`. + +### Key Normalization + +Status and type keys use different normalization rules: + +- **Status keys** use camelCase. Splits on `_`, `-`, ` `, and camelCase boundaries, then reassembles as camelCase. + Examples: `"in_progress"` -> `"inProgress"`, `"In Progress"` -> `"inProgress"`. + +- **Type keys** are lowercased with all separators stripped. + Examples: `"My-Type"` -> `"mytype"`, `"some_thing"` -> `"something"`. + +Keys in `workflow.yaml` must already be in their canonical form. Input normalization (from user queries, ruki expressions, etc.) still applies at lookup time. + +### Inheritance and Override + +- A section (`statuses:` or `types:`) absent from a workflow file means "no opinion" -- it does not override the inherited value. +- A non-empty section fully replaces inherited/built-in entries. No merging across files. +- The last file (most specific location) with the section present wins. +- If no file defines `types:`, built-in defaults are used (`story`, `bug`, `spike`, `epic`). +- Statuses have no built-in fallback -- at least one workflow file must define `statuses:`. + +## Failure Behavior + +### Invalid Configuration + +| Scenario | Behavior | +|---|---| +| Empty list | Error | +| Non-canonical key | Error with suggested canonical form | +| Empty/whitespace label | Error | +| Duplicate display string | Error | +| Unknown key in entry | Error | +| Missing `default: true` (statuses) | Error | +| Missing `done: true` (statuses) | Error | +| Multiple `default: true` (statuses) | Error | +| Multiple `done: true` (statuses) | Error | + +### Invalid Saved Tasks + +- A tiki with a missing or unknown `type` fails to load and is skipped. +- On single-task reload (`ReloadTask`), an invalid file causes the task to be removed from memory. + +### Invalid Templates + +- Missing `type` in a template defaults to the first configured type. +- Invalid non-empty `type` in a template is a hard error; creation is aborted. + +### Sample Tasks at Init + +- Each embedded sample is validated against the active registries before writing. +- Incompatible samples are silently skipped. +- `tiki init` offers a "Create sample tasks" checkbox (default: enabled). + +### Cross-Reference Errors + +If a `types:` override removes type keys still referenced by inherited views, actions, or triggers, startup fails with a configuration error. There is no silent view-skipping or automatic remapping. + +## Pre-Init Rules + +Calling type or status helpers (`task.ParseType()`, `task.AllTypes()`, `task.DefaultType()`, `task.ParseStatus()`, etc.) before `config.LoadWorkflowRegistries()` is a programmer error and panics. diff --git a/.doc/doki/doc/customization.md b/.doc/doki/doc/customization.md index 05ef4ff..369f2d9 100644 --- a/.doc/doki/doc/customization.md +++ b/.doc/doki/doc/customization.md @@ -7,7 +7,7 @@ while plugins control what you see and how you interact with your work. This sec ## Statuses Workflow statuses are defined in `workflow.yaml` under the `statuses:` key. Every tiki project must define -its statuses here — there is no hardcoded fallback. The default `workflow.yaml` ships with: +its statuses here — there is no hardcoded fallback. See [Custom statuses and types](custom-status-type.md). The default `workflow.yaml` ships with: ```yaml statuses: @@ -19,7 +19,7 @@ statuses: label: Ready emoji: "📋" active: true - - key: in_progress + - key: inProgress label: "In Progress" emoji: "⚙️" active: true @@ -34,15 +34,43 @@ statuses: ``` Each status has: -- `key` — canonical identifier (lowercase, underscores). Used in filters, actions, and frontmatter. -- `label` — display name shown in the UI +- `key` — canonical camelCase identifier. Used in filters, actions, and frontmatter. +- `label` — display name shown in the UI (defaults to key when omitted) - `emoji` — emoji shown alongside the label - `active` — marks the status as "active work" (used for activity tracking) -- `default` — the status assigned to new tikis (exactly one status should have this) -- `done` — marks the status as "completed" (used for completion tracking) +- `default` — the status assigned to new tikis (exactly one required) +- `done` — marks the status as "completed" (exactly one required) You can customize these to match your team's workflow. All filters and actions in view definitions (see below) must reference valid status keys. +## Types + +Task types are defined in `workflow.yaml` under the `types:` key. If omitted, built-in defaults are used. +See [Custom statuses and types](custom-status-type.md) for the full validation and inheritance rules. The default `workflow.yaml` ships with: + +```yaml +types: + - key: story + label: Story + emoji: "🌀" + - key: bug + label: Bug + emoji: "💥" + - key: spike + label: Spike + emoji: "🔍" + - key: epic + label: Epic + emoji: "🗂️" +``` + +Each type has: +- `key` — canonical lowercase identifier. Used in filters, actions, and frontmatter. +- `label` — display name shown in the UI (defaults to key when omitted) +- `emoji` — emoji shown alongside the label + +The first configured type is used as the default for new tikis. + ## Task Template When you create a new tiki — whether in the TUI or command line — field defaults come from a template file. @@ -53,8 +81,7 @@ The file uses YAML frontmatter for field defaults ```markdown --- -type: story -status: backlog +title: points: 1 priority: 3 tags: @@ -62,6 +89,8 @@ tags: --- ``` +Type and status are omitted but can be added, otherwise they default to the first configured type and the status marked `default: true`. + ## Plugins tiki TUI app is much like a lego - everything is a customizable view. Here is, for example, @@ -240,7 +269,7 @@ update where id = id() set assignee=user() - `id` - task identifier (e.g., "TIKI-M7N2XK") - `title` - task title text -- `type` - task type: "story", "bug", "spike", or "epic" +- `type` - task type (must match a key defined in `workflow.yaml` types) - `status` - workflow status (must match a key defined in `workflow.yaml` statuses) - `assignee` - assigned user - `priority` - numeric priority value (1-5) @@ -273,4 +302,4 @@ update where id = id() set assignee=user() - `id()` — currently selected tiki (in plugin context) - `count(select where ...)` — count matching tikis -For the full language reference, see the [ruki documentation](ruki/index.md). +For the full language reference, see the [ruki documentation](ruki/index.md). \ No newline at end of file diff --git a/.doc/doki/doc/index.md b/.doc/doki/doc/index.md index 0348894..a660fac 100644 --- a/.doc/doki/doc/index.md +++ b/.doc/doki/doc/index.md @@ -6,6 +6,7 @@ - [Markdown viewer](markdown-viewer.md) - [Image support](image-requirements.md) - [Custom fields](custom-fields.md) +- [Custom statuses and types](custom-status-type.md) - [Customization](customization.md) - [Themes](themes.md) - [ruki](ruki/index.md) diff --git a/.doc/doki/doc/ruki/types-and-values.md b/.doc/doki/doc/ruki/types-and-values.md index 64cbb92..52ba0f6 100644 --- a/.doc/doki/doc/ruki/types-and-values.md +++ b/.doc/doki/doc/ruki/types-and-values.md @@ -111,10 +111,11 @@ select where due is empty `type` -- normalized through the injected schema +- validated through the injected schema against the `types:` section of `workflow.yaml` - production normalization lowercases, trims, and removes separators - default built-in types are `story`, `bug`, `spike`, and `epic` -- default aliases include `feature` and `task` mapping to `story` +- type keys must be canonical (matching normalized form); aliases are not supported +- unknown type values are rejected — no silent fallback Examples: diff --git a/.doc/doki/doc/tiki-format.md b/.doc/doki/doc/tiki-format.md index f83a010..60b81e0 100644 --- a/.doc/doki/doc/tiki-format.md +++ b/.doc/doki/doc/tiki-format.md @@ -88,10 +88,11 @@ title: Implement user authentication #### type -Optional string. Default: `story`. +Optional string. Defaults to the first type defined in `workflow.yaml`. -Valid values: `story`, `bug`, `spike`, `epic`. Aliases `feature` and `task` resolve to `story`. -In the TUI each type has an icon: Story 🌀, Bug 💥, Spike 🔍, Epic 🗂️. +Valid values are the type keys defined in the `types:` section of `workflow.yaml`. +Default types: `story`, `bug`, `spike`, `epic`. Each type can have a label and emoji +configured in `workflow.yaml`. Aliases are not supported; use the canonical key. ```yaml type: bug diff --git a/config/default_workflow.yaml b/config/default_workflow.yaml index d53d832..011da9c 100644 --- a/config/default_workflow.yaml +++ b/config/default_workflow.yaml @@ -20,6 +20,20 @@ statuses: emoji: "✅" done: true +types: + - key: story + label: Story + emoji: "🌀" + - key: bug + label: Bug + emoji: "💥" + - key: spike + label: Spike + emoji: "🔍" + - key: epic + label: Epic + emoji: "🗂️" + views: actions: - key: "a" diff --git a/config/init.go b/config/init.go index 48d6abf..ff17307 100644 --- a/config/init.go +++ b/config/init.go @@ -26,10 +26,17 @@ func IsProjectInitialized() bool { return info.IsDir() } +// InitOptions holds user choices from the init dialog. +type InitOptions struct { + AITools []string + SampleTasks bool +} + // PromptForProjectInit presents a Huh form for project initialization. -// Returns (selectedAITools, proceed, error) -func PromptForProjectInit() ([]string, bool, error) { - var selectedAITools []string +// Returns (options, proceed, error) +func PromptForProjectInit() (InitOptions, bool, error) { + var opts InitOptions + opts.SampleTasks = true // default enabled // Create custom theme with brighter description and help text theme := huh.ThemeCharm() @@ -68,9 +75,9 @@ AI skills extend your AI assistant with commands to manage tasks and documentati Select AI assistants to install (optional), then press Enter to continue. Press Esc to cancel project initialization.` - options := make([]huh.Option[string], 0, len(AITools())) + aiOptions := make([]huh.Option[string], 0, len(AITools())) for _, t := range AITools() { - options = append(options, huh.NewOption(fmt.Sprintf("%s (%s/)", t.DisplayName, t.SkillDir), t.Key)) + aiOptions = append(aiOptions, huh.NewOption(fmt.Sprintf("%s (%s/)", t.DisplayName, t.SkillDir), t.Key)) } form := huh.NewForm( @@ -78,9 +85,13 @@ Press Esc to cancel project initialization.` huh.NewMultiSelect[string](). Title("Initialize project"). Description(description). - Options(options...). + Options(aiOptions...). Filterable(false). - Value(&selectedAITools), + Value(&opts.AITools), + huh.NewConfirm(). + Title("Create sample tasks"). + Description("Seed project with example tikis (incompatible samples are skipped automatically)"). + Value(&opts.SampleTasks), ), ).WithTheme(theme). WithKeyMap(keymap). @@ -89,12 +100,12 @@ Press Esc to cancel project initialization.` err := form.Run() if err != nil { if errors.Is(err, huh.ErrUserAborted) { - return nil, false, nil + return InitOptions{}, false, nil } - return nil, false, fmt.Errorf("form error: %w", err) + return InitOptions{}, false, fmt.Errorf("form error: %w", err) } - return selectedAITools, true, nil + return opts, true, nil } // EnsureProjectInitialized bootstraps the project if .doc/tiki is missing. @@ -107,7 +118,7 @@ func EnsureProjectInitialized(tikiSkillMdContent, dokiSkillMdContent string) (bo return false, fmt.Errorf("failed to stat task directory: %w", err) } - selectedTools, proceed, err := PromptForProjectInit() + opts, proceed, err := PromptForProjectInit() if err != nil { return false, fmt.Errorf("failed to prompt for project initialization: %w", err) } @@ -115,18 +126,18 @@ func EnsureProjectInitialized(tikiSkillMdContent, dokiSkillMdContent string) (bo return false, nil } - if err := BootstrapSystem(); err != nil { + if err := BootstrapSystem(opts.SampleTasks); err != nil { return false, fmt.Errorf("failed to bootstrap project: %w", err) } // Install selected AI skills - if len(selectedTools) > 0 { - if err := installAISkills(selectedTools, tikiSkillMdContent, dokiSkillMdContent); err != nil { + if len(opts.AITools) > 0 { + if err := installAISkills(opts.AITools, tikiSkillMdContent, dokiSkillMdContent); err != nil { // Non-fatal - log warning but continue slog.Warn("some AI skills failed to install", "error", err) fmt.Println("You can manually copy ai/skills/tiki/SKILL.md and ai/skills/doki/SKILL.md to the appropriate directories.") } else { - fmt.Printf("✓ Installed AI skills for: %s\n", strings.Join(selectedTools, ", ")) + fmt.Printf("✓ Installed AI skills for: %s\n", strings.Join(opts.AITools, ", ")) } } diff --git a/config/loader.go b/config/loader.go index 83d31ae..ba8dde3 100644 --- a/config/loader.go +++ b/config/loader.go @@ -201,6 +201,7 @@ type viewsFileData struct { // all top-level sections must be listed here to survive round-trip serialization. type workflowFileData struct { Statuses []map[string]interface{} `yaml:"statuses,omitempty"` + Types []map[string]interface{} `yaml:"types,omitempty"` Views viewsFileData `yaml:"views,omitempty"` Triggers []map[string]interface{} `yaml:"triggers,omitempty"` Fields []map[string]interface{} `yaml:"fields,omitempty"` diff --git a/config/new.md b/config/new.md index 0c01c3d..4dc7fee 100644 --- a/config/new.md +++ b/config/new.md @@ -1,7 +1,5 @@ --- title: -type: story -status: backlog points: 1 priority: 3 tags: diff --git a/config/statuses.go b/config/statuses.go index 76224b4..25304f3 100644 --- a/config/statuses.go +++ b/config/statuses.go @@ -28,11 +28,9 @@ var ( registryMu sync.RWMutex ) -// LoadStatusRegistry reads the statuses: section from workflow.yaml files. -// Uses FindRegistryWorkflowFiles (no views filtering) so files with empty views: -// still contribute status definitions. -// The last file that contains a non-empty statuses list wins -// (most specific location takes precedence, matching plugin merge behavior). +// LoadStatusRegistry reads statuses: and types: from workflow.yaml files. +// Both registries are loaded into locals first, then published atomically +// so no intermediate state exists where one is updated and the other stale. // Returns an error if no statuses are defined anywhere (no Go fallback). func LoadStatusRegistry() error { files := FindRegistryWorkflowFiles() @@ -40,28 +38,27 @@ func LoadStatusRegistry() error { return fmt.Errorf("no workflow.yaml found; statuses must be defined in workflow.yaml") } - reg, path, err := loadStatusRegistryFromFiles(files) + statusReg, statusPath, err := loadStatusRegistryFromFiles(files) if err != nil { return err } - if reg == nil { + if statusReg == nil { return fmt.Errorf("no statuses defined in workflow.yaml; add a statuses: section") } - registryMu.Lock() - globalStatusRegistry = reg - registryMu.Unlock() - slog.Debug("loaded status registry", "file", path, "count", len(reg.All())) - - // also initialize type registry with defaults - typeReg, err := workflow.NewTypeRegistry(workflow.DefaultTypeDefs()) + typeReg, typePath, err := loadTypeRegistryFromFiles(files) if err != nil { - return fmt.Errorf("initializing type registry: %w", err) + return err } + + // publish both atomically registryMu.Lock() + globalStatusRegistry = statusReg globalTypeRegistry = typeReg registryMu.Unlock() + slog.Debug("loaded status registry", "file", statusPath, "count", len(statusReg.All())) + slog.Debug("loaded type registry", "file", typePath, "count", len(typeReg.All())) return nil } @@ -118,8 +115,8 @@ func MaybeGetTypeRegistry() (*workflow.TypeRegistry, bool) { } // ResetStatusRegistry replaces the global registry with one built from the given defs. -// Also clears custom fields so test helpers don't leak registry state. -// Intended for tests only. +// Also resets types to built-in defaults and clears custom fields so test helpers +// don't leak registry state. Intended for tests only. func ResetStatusRegistry(defs []workflow.StatusDef) { reg, err := workflow.NewStatusRegistry(defs) if err != nil { @@ -137,6 +134,19 @@ func ResetStatusRegistry(defs []workflow.StatusDef) { registriesLoaded.Store(true) } +// ResetTypeRegistry replaces the global type registry with one built from the +// given defs, without touching the status registry. Intended for tests that +// need custom type configurations while keeping existing status setup. +func ResetTypeRegistry(defs []workflow.TypeDef) { + reg, err := workflow.NewTypeRegistry(defs) + if err != nil { + panic(fmt.Sprintf("ResetTypeRegistry: %v", err)) + } + registryMu.Lock() + globalTypeRegistry = reg + registryMu.Unlock() +} + // ClearStatusRegistry removes the global registries and clears custom fields. // Intended for test teardown. func ClearStatusRegistry() { @@ -148,7 +158,7 @@ func ClearStatusRegistry() { registriesLoaded.Store(false) } -// --- internal --- +// --- internal: statuses --- // workflowStatusData is the YAML shape we unmarshal to extract just the statuses key. type workflowStatusData struct { @@ -172,3 +182,99 @@ func loadStatusesFromFile(path string) (*workflow.StatusRegistry, error) { return workflow.NewStatusRegistry(ws.Statuses) } + +// --- internal: types --- + +// validTypeDefKeys is the set of allowed keys inside a types: entry. +var validTypeDefKeys = map[string]bool{ + "key": true, "label": true, "emoji": true, +} + +// loadTypesFromFile loads types from a single workflow.yaml. +// Returns (registry, present, error): +// - (nil, false, nil) when the types: key is absent — file does not override +// - (reg, true, nil) when types: is present and valid +// - (nil, true, err) when types: is present but invalid (empty list, bad entries) +func loadTypesFromFile(path string) (*workflow.TypeRegistry, bool, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, false, fmt.Errorf("reading %s: %w", path, err) + } + + // first pass: check whether the types key exists at all + var raw map[string]interface{} + if err := yaml.Unmarshal(data, &raw); err != nil { + return nil, false, fmt.Errorf("parsing %s: %w", path, err) + } + + rawTypes, exists := raw["types"] + if !exists { + return nil, false, nil // absent — no opinion + } + + // present: validate the raw structure + typesSlice, ok := rawTypes.([]interface{}) + if !ok { + return nil, true, fmt.Errorf("types: must be a list, got %T", rawTypes) + } + if len(typesSlice) == 0 { + return nil, true, fmt.Errorf("types section must define at least one type") + } + + // validate each entry for unknown keys and convert to TypeDef + defs := make([]workflow.TypeDef, 0, len(typesSlice)) + for i, entry := range typesSlice { + entryMap, ok := entry.(map[string]interface{}) + if !ok { + return nil, true, fmt.Errorf("type at index %d: expected mapping, got %T", i, entry) + } + for k := range entryMap { + if !validTypeDefKeys[k] { + return nil, true, fmt.Errorf("type at index %d: unknown key %q (valid keys: key, label, emoji)", i, k) + } + } + + var def workflow.TypeDef + keyRaw, _ := entryMap["key"].(string) + def.Key = workflow.TaskType(keyRaw) + def.Label, _ = entryMap["label"].(string) + def.Emoji, _ = entryMap["emoji"].(string) + defs = append(defs, def) + } + + reg, err := workflow.NewTypeRegistry(defs) + if err != nil { + return nil, true, err + } + return reg, true, nil +} + +// loadTypeRegistryFromFiles iterates workflow files. The last file that has a +// types: key wins. Files without a types: key are skipped (no override). +// If no file defines types:, returns a registry built from DefaultTypeDefs(). +func loadTypeRegistryFromFiles(files []string) (*workflow.TypeRegistry, string, error) { + var lastReg *workflow.TypeRegistry + var lastFile string + + for _, path := range files { + reg, present, err := loadTypesFromFile(path) + if err != nil { + return nil, "", fmt.Errorf("loading types from %s: %w", path, err) + } + if present { + lastReg = reg + lastFile = path + } + } + + if lastReg != nil { + return lastReg, lastFile, nil + } + + // no file defined types: — fall back to built-in defaults + reg, err := workflow.NewTypeRegistry(workflow.DefaultTypeDefs()) + if err != nil { + return nil, "", fmt.Errorf("building default type registry: %w", err) + } + return reg, "", nil +} diff --git a/config/statuses_test.go b/config/statuses_test.go index ae82c25..bf3ac24 100644 --- a/config/statuses_test.go +++ b/config/statuses_test.go @@ -152,19 +152,32 @@ func TestRegistry_Keys(t *testing.T) { } } -func TestRegistry_NormalizesKeys(t *testing.T) { - custom := []workflow.StatusDef{ +func TestRegistry_RejectsNonCanonicalKeys(t *testing.T) { + _, err := workflow.NewStatusRegistry([]workflow.StatusDef{ {Key: "In-Progress", Label: "In Progress", Default: true}, - {Key: " DONE ", Label: "Done", Done: true}, + {Key: "done", Label: "Done", Done: true}, + }) + if err == nil { + t.Fatal("expected error for non-canonical key") + } + if got := err.Error(); got != `status key "In-Progress" is not canonical; use "inProgress"` { + t.Errorf("got error %q", got) + } +} + +func TestRegistry_CanonicalKeysWork(t *testing.T) { + custom := []workflow.StatusDef{ + {Key: "inProgress", Label: "In Progress", Default: true}, + {Key: "done", Label: "Done", Done: true}, } setupTestRegistry(t, custom) reg := GetStatusRegistry() if !reg.IsValid("inProgress") { - t.Error("expected 'inProgress' to be valid after normalization") + t.Error("expected 'inProgress' to be valid") } if !reg.IsValid("done") { - t.Error("expected 'done' to be valid after normalization") + t.Error("expected 'done' to be valid") } } @@ -196,17 +209,87 @@ func TestBuildRegistry_Empty(t *testing.T) { } } -func TestBuildRegistry_DefaultFallsToFirst(t *testing.T) { +func TestBuildRegistry_RequiresExplicitDefault(t *testing.T) { defs := []workflow.StatusDef{ - {Key: "alpha", Label: "Alpha"}, + {Key: "alpha", Label: "Alpha", Done: true}, {Key: "beta", Label: "Beta"}, } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error when no status is marked default") + } +} + +func TestBuildRegistry_RequiresExplicitDone(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Label: "Alpha", Default: true}, + {Key: "beta", Label: "Beta"}, + } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error when no status is marked done") + } +} + +func TestBuildRegistry_RejectsDuplicateDefault(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Label: "Alpha", Default: true}, + {Key: "beta", Label: "Beta", Default: true, Done: true}, + } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for duplicate default") + } +} + +func TestBuildRegistry_RejectsDuplicateDone(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Label: "Alpha", Default: true, Done: true}, + {Key: "beta", Label: "Beta", Done: true}, + } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for duplicate done") + } +} + +func TestBuildRegistry_RejectsDuplicateDisplay(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Label: "Open", Emoji: "🟢", Default: true}, + {Key: "beta", Label: "Open", Emoji: "🟢", Done: true}, + } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for duplicate display") + } +} + +func TestBuildRegistry_LabelDefaultsToKey(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Emoji: "🔵", Default: true}, + {Key: "beta", Emoji: "🔴", Done: true}, + } reg, err := workflow.NewStatusRegistry(defs) if err != nil { t.Fatalf("unexpected error: %v", err) } - if reg.DefaultKey() != "alpha" { - t.Errorf("expected default to fall back to first status 'alpha', got %q", reg.DefaultKey()) + def, ok := reg.Lookup("alpha") + if !ok { + t.Fatal("expected to find alpha") + } + if def.Label != "alpha" { + t.Errorf("expected label to default to key, got %q", def.Label) + } +} + +func TestBuildRegistry_EmptyWhitespaceLabel(t *testing.T) { + defs := []workflow.StatusDef{ + {Key: "alpha", Label: " ", Default: true}, + {Key: "beta", Done: true}, + } + _, err := workflow.NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for whitespace-only label") } } @@ -430,6 +513,9 @@ statuses: - key: alpha label: Alpha default: true + - key: beta + label: Beta + done: true `) f2 := writeTempWorkflow(t, dir2, ` statuses: [[[invalid yaml @@ -615,3 +701,240 @@ func TestLoadStatusRegistryFromFiles_AllFilesEmpty(t *testing.T) { t.Errorf("expected empty path, got %q", path) } } + +// --- type loading tests --- + +func TestLoadTypesFromFile_HappyPath(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +types: + - key: story + label: Story + emoji: "🌀" + - key: bug + label: Bug + emoji: "💥" +`) + reg, present, err := loadTypesFromFile(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !present { + t.Fatal("expected present=true") + } + if reg == nil { + t.Fatal("expected non-nil registry") + } + if !reg.IsValid("story") { + t.Error("expected story to be valid") + } + if !reg.IsValid("bug") { + t.Error("expected bug to be valid") + } +} + +func TestLoadTypesFromFile_Absent(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +statuses: + - key: open + label: Open + default: true + - key: done + label: Done + done: true +`) + reg, present, err := loadTypesFromFile(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if present { + t.Error("expected present=false when types: key is absent") + } + if reg != nil { + t.Error("expected nil registry when absent") + } +} + +func TestLoadTypesFromFile_EmptyList(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, `types: []`) + _, present, err := loadTypesFromFile(f) + if err == nil { + t.Fatal("expected error for types: []") + } + if !present { + t.Error("expected present=true even for empty list") + } +} + +func TestLoadTypesFromFile_NonCanonicalKey(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +types: + - key: Story + label: Story +`) + _, _, err := loadTypesFromFile(f) + if err == nil { + t.Fatal("expected error for non-canonical key") + } +} + +func TestLoadTypesFromFile_UnknownKey(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +types: + - key: story + label: Story + aliases: + - feature +`) + _, _, err := loadTypesFromFile(f) + if err == nil { + t.Fatal("expected error for unknown key 'aliases'") + } +} + +func TestLoadTypesFromFile_MissingLabelDefaultsToKey(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +types: + - key: task + emoji: "📋" +`) + reg, _, err := loadTypesFromFile(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := reg.TypeLabel("task"); got != "task" { + t.Errorf("expected label to default to key, got %q", got) + } +} + +func TestLoadTypesFromFile_InvalidYAML(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, `types: [[[invalid`) + _, _, err := loadTypesFromFile(f) + if err == nil { + t.Fatal("expected error for invalid YAML") + } +} + +func TestLoadTypeRegistryFromFiles_LastFileWithTypesWins(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + + f1 := writeTempWorkflow(t, dir1, ` +types: + - key: alpha + label: Alpha +`) + f2 := writeTempWorkflow(t, dir2, ` +types: + - key: beta + label: Beta + - key: gamma + label: Gamma +`) + + reg, path, err := loadTypeRegistryFromFiles([]string{f1, f2}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != f2 { + t.Errorf("expected path %q, got %q", f2, path) + } + if reg.IsValid("alpha") { + t.Error("expected alpha to NOT be valid (overridden)") + } + if !reg.IsValid("beta") { + t.Error("expected beta to be valid") + } +} + +func TestLoadTypeRegistryFromFiles_SkipsFilesWithoutTypes(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + + f1 := writeTempWorkflow(t, dir1, ` +types: + - key: alpha + label: Alpha +`) + f2 := writeTempWorkflow(t, dir2, ` +statuses: + - key: open + label: Open + default: true + - key: done + label: Done + done: true +`) + + reg, path, err := loadTypeRegistryFromFiles([]string{f1, f2}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != f1 { + t.Errorf("expected path %q (file with types), got %q", f1, path) + } + if !reg.IsValid("alpha") { + t.Error("expected alpha to be valid") + } +} + +func TestLoadTypeRegistryFromFiles_FallbackToBuiltins(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +statuses: + - key: open + label: Open + default: true + - key: done + label: Done + done: true +`) + + reg, path, err := loadTypeRegistryFromFiles([]string{f}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if path != "" { + t.Errorf("expected built-in path, got %q", path) + } + if !reg.IsValid("story") { + t.Error("expected built-in story type") + } +} + +func TestLoadTypeRegistryFromFiles_ParseErrorStops(t *testing.T) { + dir := t.TempDir() + f := writeTempWorkflow(t, dir, ` +types: + - key: Story + label: Story +`) + _, _, err := loadTypeRegistryFromFiles([]string{f}) + if err == nil { + t.Fatal("expected error for non-canonical key") + } +} + +func TestResetTypeRegistry(t *testing.T) { + setupTestRegistry(t, defaultTestStatuses()) + + custom := []workflow.TypeDef{ + {Key: "task", Label: "Task"}, + {Key: "incident", Label: "Incident"}, + } + ResetTypeRegistry(custom) + + reg := GetTypeRegistry() + if !reg.IsValid("task") { + t.Error("expected 'task' to be valid after ResetTypeRegistry") + } + if reg.IsValid("story") { + t.Error("expected 'story' to NOT be valid after ResetTypeRegistry") + } +} diff --git a/config/system.go b/config/system.go index 9c483ad..7cc6ffc 100644 --- a/config/system.go +++ b/config/system.go @@ -3,9 +3,11 @@ package config import ( _ "embed" "fmt" + "log/slog" "os" "os/exec" "path/filepath" + "regexp" "strings" gonanoid "github.com/matoous/go-nanoid/v2" @@ -56,8 +58,35 @@ func GenerateRandomID() string { return id } +// sampleFrontmatterRe extracts type and status values from sample tiki frontmatter. +var sampleFrontmatterRe = regexp.MustCompile(`(?m)^(type|status):\s*(.+)$`) + +// validateSampleTiki checks whether a sample tiki's type and status +// are valid against the current workflow registries. +func validateSampleTiki(template string) bool { + matches := sampleFrontmatterRe.FindAllStringSubmatch(template, -1) + statusReg := GetStatusRegistry() + typeReg := GetTypeRegistry() + for _, m := range matches { + key, val := m[1], strings.TrimSpace(m[2]) + switch key { + case "type": + if _, ok := typeReg.ParseType(val); !ok { + return false + } + case "status": + if !statusReg.IsValid(val) { + return false + } + } + } + return true +} + // BootstrapSystem creates the task storage and seeds the initial tiki. -func BootstrapSystem() error { +// If createSamples is true, embedded sample tikis are validated against +// the active workflow registries and only valid ones are written. +func BootstrapSystem(createSamples bool) error { // Create all necessary directories if err := EnsureDirs(); err != nil { return fmt.Errorf("ensure directories: %w", err) @@ -66,59 +95,50 @@ func BootstrapSystem() error { taskDir := GetTaskDir() var createdFiles []string - // Helper function to create a sample tiki - createSampleTiki := func(template string) (string, error) { - randomID := GenerateRandomID() - taskID := fmt.Sprintf("TIKI-%s", randomID) - taskFilename := fmt.Sprintf("tiki-%s.md", randomID) - taskPath := filepath.Join(taskDir, taskFilename) - - // Replace placeholder in template - taskContent := strings.Replace(template, "TIKI-XXXXXX", taskID, 1) - if err := os.WriteFile(taskPath, []byte(taskContent), 0644); err != nil { - return "", fmt.Errorf("write task: %w", err) + if createSamples { + // ensure workflow registries are loaded before validating samples; + // on first run this may require installing the default workflow first + if err := InstallDefaultWorkflow(); err != nil { + slog.Warn("failed to install default workflow for sample validation", "error", err) + } + if err := LoadWorkflowRegistries(); err != nil { + slog.Warn("failed to load workflow registries for sample validation; skipping samples", "error", err) + createSamples = false } - return taskPath, nil } - // Create board sample (original welcome tiki) - boardPath, err := createSampleTiki(initialTaskTemplate) - if err != nil { - return fmt.Errorf("create board sample: %w", err) - } - createdFiles = append(createdFiles, boardPath) + if createSamples { + type sampleDef struct { + name string + template string + } + samples := []sampleDef{ + {"board", initialTaskTemplate}, + {"backlog 1", backlogSample1}, + {"backlog 2", backlogSample2}, + {"roadmap now", roadmapNowSample}, + {"roadmap next", roadmapNextSample}, + {"roadmap later", roadmapLaterSample}, + } - // Create backlog samples - backlog1Path, err := createSampleTiki(backlogSample1) - if err != nil { - return fmt.Errorf("create backlog sample 1: %w", err) - } - createdFiles = append(createdFiles, backlog1Path) + for _, s := range samples { + if !validateSampleTiki(s.template) { + slog.Info("skipping incompatible sample tiki", "name", s.name) + continue + } - backlog2Path, err := createSampleTiki(backlogSample2) - if err != nil { - return fmt.Errorf("create backlog sample 2: %w", err) - } - createdFiles = append(createdFiles, backlog2Path) + randomID := GenerateRandomID() + taskID := fmt.Sprintf("TIKI-%s", randomID) + taskFilename := fmt.Sprintf("tiki-%s.md", randomID) + taskPath := filepath.Join(taskDir, taskFilename) - // Create roadmap samples - roadmapNowPath, err := createSampleTiki(roadmapNowSample) - if err != nil { - return fmt.Errorf("create roadmap now sample: %w", err) + taskContent := strings.Replace(s.template, "TIKI-XXXXXX", taskID, 1) + if err := os.WriteFile(taskPath, []byte(taskContent), 0644); err != nil { + return fmt.Errorf("create sample %s: %w", s.name, err) + } + createdFiles = append(createdFiles, taskPath) + } } - createdFiles = append(createdFiles, roadmapNowPath) - - roadmapNextPath, err := createSampleTiki(roadmapNextSample) - if err != nil { - return fmt.Errorf("create roadmap next sample: %w", err) - } - createdFiles = append(createdFiles, roadmapNextPath) - - roadmapLaterPath, err := createSampleTiki(roadmapLaterSample) - if err != nil { - return fmt.Errorf("create roadmap later sample: %w", err) - } - createdFiles = append(createdFiles, roadmapLaterPath) // Write doki documentation files dokiDir := GetDokiDir() diff --git a/internal/ruki/runtime/runner_test.go b/internal/ruki/runtime/runner_test.go index 3b586a7..4718887 100644 --- a/internal/ruki/runtime/runner_test.go +++ b/internal/ruki/runtime/runner_test.go @@ -439,9 +439,9 @@ func TestRunQueryCreateTemplateDefaults(t *testing.T) { if len(found.Tags) != 2 || found.Tags[0] != "idea" || found.Tags[1] != "extra" { t.Errorf("tags = %v, want [idea extra]", found.Tags) } - // priority should be template default (7) - if found.Priority != 7 { - t.Errorf("priority = %d, want 7 (template default)", found.Priority) + // priority should be template default (3 = medium) + if found.Priority != 3 { + t.Errorf("priority = %d, want 3 (template default)", found.Priority) } } diff --git a/internal/ruki/runtime/schema_test.go b/internal/ruki/runtime/schema_test.go index da80557..1d3c2a6 100644 --- a/internal/ruki/runtime/schema_test.go +++ b/internal/ruki/runtime/schema_test.go @@ -104,9 +104,9 @@ func TestSchemaNormalizeType(t *testing.T) { }{ {"story", "story", true}, {"bug", "bug", true}, - {"feature", "story", true}, // alias - {"task", "story", true}, // alias - {"unknown_type", "story", false}, // falls back to first type + {"feature", "", false}, // no aliases + {"task", "", false}, // no aliases + {"unknown_type", "", false}, // unknown returns empty } for _, tt := range tests { diff --git a/llms.txt b/llms.txt index 0cc46ba..19afa6b 100644 --- a/llms.txt +++ b/llms.txt @@ -11,6 +11,7 @@ Everything beyond the core is plugin-driven. `workflow.yaml` defines statuses an - [Quick Start](.doc/doki/doc/quick-start.md): getting started — markdown viewer, file management, tiki board, keyboard shortcuts - [Tiki Format](.doc/doki/doc/tiki-format.md): .doc directory structure, tiki Markdown+YAML spec, field types - [Configuration](.doc/doki/doc/config.md): config.yaml, workflow.yaml, config directories, settings reference +- [Custom Statuses and Types](.doc/doki/doc/custom-status-type.md): type and status definition rules, validation, key normalization, inheritance - [Customization](.doc/doki/doc/customization.md): workflow statuses, plugin definitions, lanes, actions, views - [Installation](.doc/doki/doc/install.md): macOS, Linux, Windows, manual install - [Markdown Viewer](.doc/doki/doc/markdown-viewer.md): pager commands, link navigation, image/diagram rendering diff --git a/plugin/loader.go b/plugin/loader.go index cd84139..69e77f2 100644 --- a/plugin/loader.go +++ b/plugin/loader.go @@ -145,7 +145,8 @@ func mergePluginLists(base, overrides []Plugin) []Plugin { // Files are discovered via config.FindWorkflowFiles() which returns user config first, then project config. // Plugins from later files override same-named plugins from earlier files via field merging. // Global actions are merged by key across files (later files override same-keyed globals from earlier files). -// Returns an error when workflow files were found but no valid plugins could be loaded. +// Returns an error when workflow files were found but no valid plugins could be loaded, +// or when type-reference errors indicate an inconsistent merged workflow. func LoadPlugins(schema ruki.Schema) ([]Plugin, error) { files := config.FindWorkflowFiles() if len(files) == 0 { @@ -171,6 +172,13 @@ func LoadPlugins(schema ruki.Schema) ([]Plugin, error) { allGlobalActions = mergeGlobalActions(allGlobalActions, moreGlobals) } + // type-reference errors in views/actions are fatal merged-workflow errors, + // not ordinary per-view parse errors that can be skipped + if typeErrs := filterTypeErrors(allErrors); len(typeErrs) > 0 { + return nil, fmt.Errorf("merged workflow references invalid types:\n %s\n\nIf you redefined types: in a later workflow file, update views/actions/triggers to match", + strings.Join(typeErrs, "\n ")) + } + // merge global actions into each TikiPlugin mergeGlobalActionsIntoPlugins(base, allGlobalActions) @@ -188,6 +196,17 @@ func LoadPlugins(schema ruki.Schema) ([]Plugin, error) { return base, nil } +// filterTypeErrors extracts errors that mention unknown type references. +func filterTypeErrors(errs []string) []string { + var typeErrs []string + for _, e := range errs { + if strings.Contains(e, "unknown type") { + typeErrs = append(typeErrs, e) + } + } + return typeErrs +} + // mergeGlobalActions merges override global actions into base by key (rune). // Overrides with the same rune replace the base action. func mergeGlobalActions(base, overrides []PluginAction) []PluginAction { diff --git a/store/memory_store.go b/store/memory_store.go index 61a2323..3278331 100644 --- a/store/memory_store.go +++ b/store/memory_store.go @@ -248,9 +248,9 @@ func (s *InMemoryStore) NewTaskTemplate() (*task.Task, error) { ID: taskID, Title: "", Description: "", - Type: task.TypeStory, + Type: task.DefaultType(), Status: task.DefaultStatus(), - Priority: 7, // match embedded template default + Priority: 3, Points: 1, Tags: []string{"idea"}, CustomFields: make(map[string]interface{}), diff --git a/store/memory_store_test.go b/store/memory_store_test.go index b8f62fd..c292710 100644 --- a/store/memory_store_test.go +++ b/store/memory_store_test.go @@ -338,8 +338,8 @@ func TestInMemoryStore_NewTaskTemplate(t *testing.T) { if tmpl.ID != strings.ToUpper(tmpl.ID) { t.Errorf("ID = %q, should be uppercased", tmpl.ID) } - if tmpl.Priority != 7 { - t.Errorf("Priority = %d, want 7", tmpl.Priority) + if tmpl.Priority != 3 { + t.Errorf("Priority = %d, want 3", tmpl.Priority) } if tmpl.Points != 1 { t.Errorf("Points = %d, want 1", tmpl.Points) @@ -350,8 +350,8 @@ func TestInMemoryStore_NewTaskTemplate(t *testing.T) { if tmpl.Status != taskpkg.DefaultStatus() { t.Errorf("Status = %q, want %q", tmpl.Status, taskpkg.DefaultStatus()) } - if tmpl.Type != taskpkg.TypeStory { - t.Errorf("Type = %q, want %q", tmpl.Type, taskpkg.TypeStory) + if tmpl.Type != taskpkg.DefaultType() { + t.Errorf("Type = %q, want %q", tmpl.Type, taskpkg.DefaultType()) } } diff --git a/store/tikistore/persistence.go b/store/tikistore/persistence.go index b385c06..b151d14 100644 --- a/store/tikistore/persistence.go +++ b/store/tikistore/persistence.go @@ -121,11 +121,17 @@ func (s *TikiStore) loadTaskFile(path string, authorMap map[string]*git.AuthorIn return nil, err } + // validate type strictly — missing or unknown types are load errors + taskType, typeOK := taskpkg.ParseType(fm.Type) + if !typeOK { + return nil, fmt.Errorf("invalid or missing type %q", fm.Type) + } + task := &taskpkg.Task{ ID: taskID, Title: fm.Title, Description: strings.TrimSpace(body), - Type: taskpkg.NormalizeType(fm.Type), + Type: taskType, Status: taskpkg.MapStatus(fm.Status), Tags: fm.Tags.ToStringSlice(), DependsOn: fm.DependsOn.ToStringSlice(), @@ -259,9 +265,16 @@ func (s *TikiStore) ReloadTask(taskID string) error { } } - // Load the task file + // Load the task file — if it's now invalid (e.g. bad type after external edit), + // remove the stale in-memory copy rather than leaving it pretending the file is valid task, err := s.loadTaskFile(filePath, authorMap, lastCommitMap) if err != nil { + s.mu.Lock() + delete(s.tasks, normalizedID) + s.mu.Unlock() + slog.Warn("removed invalid task from memory after reload failure", + "task_id", normalizedID, "file", filePath, "error", err) + s.notifyListeners() return fmt.Errorf("loading task file %s: %w", filePath, err) } diff --git a/store/tikistore/template.go b/store/tikistore/template.go index 41b021a..658ef9d 100644 --- a/store/tikistore/template.go +++ b/store/tikistore/template.go @@ -96,6 +96,19 @@ func parseTaskTemplate(data []byte) (*taskpkg.Task, error) { } } + // resolve type: missing defaults to first configured type, + // invalid non-empty type is a hard error + var taskType taskpkg.Type + if fm.Type == "" { + taskType = taskpkg.DefaultType() + } else { + var ok bool + taskType, ok = taskpkg.ParseType(fm.Type) + if !ok { + return nil, fmt.Errorf("invalid template type %q", fm.Type) + } + } + // second pass: extract custom fields from frontmatter map var fmMap map[string]interface{} if err := yaml.Unmarshal([]byte(frontmatter), &fmMap); err != nil { @@ -109,7 +122,7 @@ func parseTaskTemplate(data []byte) (*taskpkg.Task, error) { return &taskpkg.Task{ Title: fm.Title, Description: body, - Type: taskpkg.NormalizeType(fm.Type), + Type: taskType, Status: taskpkg.NormalizeStatus(fm.Status), Tags: fm.Tags, DependsOn: fm.DependsOn, @@ -177,9 +190,9 @@ func (s *TikiStore) NewTaskTemplate() (*taskpkg.Task, error) { ID: taskID, Title: "", Description: "", - Status: taskpkg.DefaultStatus(), // default fallback - Type: taskpkg.TypeStory, // default fallback - Priority: 3, // default: medium priority (1-5 scale) + Status: taskpkg.DefaultStatus(), + Type: taskpkg.DefaultType(), + Priority: 3, // default: medium priority (1-5 scale) Points: 0, CreatedAt: time.Now(), } @@ -214,7 +227,7 @@ func (s *TikiStore) NewTaskTemplate() (*taskpkg.Task, error) { // Ensure type has a value (fallback if template didn't provide) if task.Type == "" { - task.Type = taskpkg.TypeStory + task.Type = taskpkg.DefaultType() } // Ensure status has a value diff --git a/task/status_test.go b/task/status_test.go index d590b7f..7fcaf73 100644 --- a/task/status_test.go +++ b/task/status_test.go @@ -9,14 +9,8 @@ import ( func setupStatusTestRegistry(t *testing.T) { t.Helper() - config.ResetStatusRegistry([]workflow.StatusDef{ - {Key: "backlog", Label: "Backlog", Emoji: "📥", Default: true}, - {Key: "ready", Label: "Ready", Emoji: "📋", Active: true}, - {Key: "inProgress", Label: "In Progress", Emoji: "⚙️", Active: true}, - {Key: "review", Label: "Review", Emoji: "👀", Active: true}, - {Key: "done", Label: "Done", Emoji: "✅", Done: true}, - }) - t.Cleanup(func() { config.ClearStatusRegistry() }) + config.ResetStatusRegistry(defaultTestStatusDefs()) + t.Cleanup(func() { config.ResetStatusRegistry(defaultTestStatusDefs()) }) } func TestParseStatus(t *testing.T) { @@ -110,8 +104,9 @@ func TestStatusDisplay(t *testing.T) { func TestStatusDisplay_NoEmoji(t *testing.T) { config.ResetStatusRegistry([]workflow.StatusDef{ {Key: "plain", Label: "Plain", Default: true}, + {Key: "finished", Label: "Finished", Done: true}, }) - t.Cleanup(func() { config.ClearStatusRegistry() }) + t.Cleanup(func() { config.ResetStatusRegistry(defaultTestStatusDefs()) }) if got := StatusDisplay("plain"); got != "Plain" { t.Errorf("StatusDisplay(%q) = %q, want %q (no emoji)", "plain", got, "Plain") diff --git a/task/type.go b/task/type.go index 6101a77..4687a55 100644 --- a/task/type.go +++ b/task/type.go @@ -1,8 +1,6 @@ package task import ( - "fmt" - "github.com/boolean-maybe/tiki/config" "github.com/boolean-maybe/tiki/workflow" ) @@ -19,59 +17,54 @@ const ( TypeEpic = workflow.TypeEpic ) -// defaultTypeRegistry is built once from the built-in type definitions. -// It serves as a fallback when config has not been initialized yet. -var defaultTypeRegistry = func() *workflow.TypeRegistry { - reg, err := workflow.NewTypeRegistry(workflow.DefaultTypeDefs()) - if err != nil { - panic(fmt.Sprintf("task: building default type registry: %v", err)) - } - return reg -}() - -// currentTypeRegistry returns the config-provided type registry when available, -// falling back to the package-level default built from DefaultTypeDefs(). -func currentTypeRegistry() *workflow.TypeRegistry { - if reg, ok := config.MaybeGetTypeRegistry(); ok { - return reg - } - return defaultTypeRegistry +// requireTypeRegistry returns the loaded type registry. +// Panics if workflow registries have not been loaded — this is a programmer +// error, not a user-facing path. +func requireTypeRegistry() *workflow.TypeRegistry { + return config.GetTypeRegistry() } // ParseType parses a raw string into a Type with validation. -// Returns the canonical key and true if recognized (including aliases), -// or (TypeStory, false) for unknown types. +// Returns the canonical key and true if recognized, +// or ("", false) for unknown types. +// Panics if registries are not loaded. func ParseType(t string) (Type, bool) { - return currentTypeRegistry().ParseType(t) -} - -// NormalizeType standardizes a raw type string into a Type. -func NormalizeType(t string) Type { - return currentTypeRegistry().NormalizeType(t) + return requireTypeRegistry().ParseType(t) } // TypeLabel returns a human-readable label for a task type. +// Panics if registries are not loaded. func TypeLabel(taskType Type) string { - return currentTypeRegistry().TypeLabel(taskType) + return requireTypeRegistry().TypeLabel(taskType) } // TypeEmoji returns the emoji for a task type. +// Panics if registries are not loaded. func TypeEmoji(taskType Type) string { - return currentTypeRegistry().TypeEmoji(taskType) + return requireTypeRegistry().TypeEmoji(taskType) } // TypeDisplay returns a formatted display string with label and emoji. +// Panics if registries are not loaded. func TypeDisplay(taskType Type) string { - return currentTypeRegistry().TypeDisplay(taskType) + return requireTypeRegistry().TypeDisplay(taskType) } // ParseDisplay reverses a TypeDisplay() string back to a canonical key. -// Returns (key, true) on match, or (fallback, false) for unrecognized display strings. +// Returns (key, true) on match, or ("", false) for unrecognized display strings. +// Panics if registries are not loaded. func ParseDisplay(display string) (Type, bool) { - return currentTypeRegistry().ParseDisplay(display) + return requireTypeRegistry().ParseDisplay(display) } // AllTypes returns the ordered list of all configured type keys. +// Panics if registries are not loaded. func AllTypes() []Type { - return currentTypeRegistry().Keys() + return requireTypeRegistry().Keys() +} + +// DefaultType returns the first configured type, used as the creation default. +// Panics if registries are not loaded. +func DefaultType() Type { + return requireTypeRegistry().DefaultType() } diff --git a/task/type_test.go b/task/type_test.go index f74257b..3ea81e1 100644 --- a/task/type_test.go +++ b/task/type_test.go @@ -1,40 +1,52 @@ package task import ( + "os" "testing" "github.com/boolean-maybe/tiki/config" ) -func TestNormalizeType(t *testing.T) { +func TestMain(m *testing.M) { + config.ResetStatusRegistry(defaultTestStatusDefs()) + os.Exit(m.Run()) +} + +func defaultTestStatusDefs() []config.StatusDef { + return []config.StatusDef{ + {Key: "backlog", Label: "Backlog", Emoji: "📥", Default: true}, + {Key: "ready", Label: "Ready", Emoji: "📋", Active: true}, + {Key: "inProgress", Label: "In Progress", Emoji: "⚙️", Active: true}, + {Key: "review", Label: "Review", Emoji: "👀", Active: true}, + {Key: "done", Label: "Done", Emoji: "✅", Done: true}, + } +} + +func TestParseType(t *testing.T) { tests := []struct { name string input string - expected Type + wantType Type + wantOK bool }{ - // Valid types - {name: "story", input: "story", expected: TypeStory}, - {name: "bug", input: "bug", expected: TypeBug}, - {name: "spike", input: "spike", expected: TypeSpike}, - {name: "epic", input: "epic", expected: TypeEpic}, - {name: "feature -> story", input: "feature", expected: TypeStory}, - {name: "task -> story", input: "task", expected: TypeStory}, - // Case variations - {name: "Story capitalized", input: "Story", expected: TypeStory}, - {name: "BUG uppercase", input: "BUG", expected: TypeBug}, - {name: "SPIKE uppercase", input: "SPIKE", expected: TypeSpike}, - {name: "EPIC uppercase", input: "EPIC", expected: TypeEpic}, - {name: "FEATURE uppercase", input: "FEATURE", expected: TypeStory}, - // Unknown defaults to story - {name: "unknown type", input: "unknown", expected: TypeStory}, - {name: "empty string", input: "", expected: TypeStory}, + {"valid story", "story", TypeStory, true}, + {"valid bug", "bug", TypeBug, true}, + {"valid spike", "spike", TypeSpike, true}, + {"valid epic", "epic", TypeEpic, true}, + {"case insensitive", "Story", TypeStory, true}, + {"uppercase", "BUG", TypeBug, true}, + {"unknown returns empty", "nonsense", "", false}, + {"empty returns empty", "", "", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := NormalizeType(tt.input) - if result != tt.expected { - t.Errorf("NormalizeType(%q) = %q, want %q", tt.input, result, tt.expected) + got, ok := ParseType(tt.input) + if ok != tt.wantOK { + t.Errorf("ParseType(%q) ok = %v, want %v", tt.input, ok, tt.wantOK) + } + if got != tt.wantType { + t.Errorf("ParseType(%q) = %q, want %q", tt.input, got, tt.wantType) } }) } @@ -109,54 +121,6 @@ func TestTypeDisplay(t *testing.T) { } } -// TestTypeHelpers_FallbackWithoutConfig verifies that all type helpers work -// when the config registry has not been initialized (fallback to defaults). -func TestTypeHelpers_FallbackWithoutConfig(t *testing.T) { - config.ClearStatusRegistry() - t.Cleanup(func() { - // restore for other tests in the package - config.ResetStatusRegistry(testStatusDefs()) - }) - - t.Run("NormalizeType", func(t *testing.T) { - if got := NormalizeType("bug"); got != TypeBug { - t.Errorf("NormalizeType(%q) = %q, want %q", "bug", got, TypeBug) - } - if got := NormalizeType("feature"); got != TypeStory { - t.Errorf("NormalizeType(%q) = %q, want %q (alias)", "feature", got, TypeStory) - } - if got := NormalizeType("unknown"); got != TypeStory { - t.Errorf("NormalizeType(%q) = %q, want %q (fallback)", "unknown", got, TypeStory) - } - }) - - t.Run("ParseType", func(t *testing.T) { - typ, ok := ParseType("epic") - if !ok || typ != TypeEpic { - t.Errorf("ParseType(%q) = (%q, %v), want (%q, true)", "epic", typ, ok, TypeEpic) - } - typ, ok = ParseType("nonsense") - if ok { - t.Errorf("ParseType(%q) returned ok=true for unknown type", "nonsense") - } - if typ != TypeStory { - t.Errorf("ParseType(%q) fallback = %q, want %q", "nonsense", typ, TypeStory) - } - }) - - t.Run("TypeLabel", func(t *testing.T) { - if got := TypeLabel(TypeBug); got != "Bug" { - t.Errorf("TypeLabel(%q) = %q, want %q", TypeBug, got, "Bug") - } - }) - - t.Run("TypeDisplay", func(t *testing.T) { - if got := TypeDisplay(TypeSpike); got != "Spike 🔍" { - t.Errorf("TypeDisplay(%q) = %q, want %q", TypeSpike, got, "Spike 🔍") - } - }) -} - func TestParseDisplay(t *testing.T) { tests := []struct { name string @@ -168,7 +132,7 @@ func TestParseDisplay(t *testing.T) { {"bug display", "Bug 💥", TypeBug, true}, {"spike display", "Spike 🔍", TypeSpike, true}, {"epic display", "Epic 🗂️", TypeEpic, true}, - {"unknown display", "Unknown 🤷", TypeStory, false}, + {"unknown display", "Unknown 🤷", "", false}, } for _, tt := range tests { @@ -189,7 +153,6 @@ func TestAllTypes(t *testing.T) { if len(types) == 0 { t.Fatal("AllTypes() returned empty list") } - // verify well-known types are present found := make(map[Type]bool) for _, tp := range types { found[tp] = true @@ -201,39 +164,32 @@ func TestAllTypes(t *testing.T) { } } -func TestParseType(t *testing.T) { - tests := []struct { - name string - input string - wantType Type - wantOK bool - }{ - {"valid story", "story", TypeStory, true}, - {"valid bug", "bug", TypeBug, true}, - {"alias feature", "feature", TypeStory, true}, - {"unknown", "nonsense", TypeStory, false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, ok := ParseType(tt.input) - if ok != tt.wantOK { - t.Errorf("ParseType(%q) ok = %v, want %v", tt.input, ok, tt.wantOK) - } - if got != tt.wantType { - t.Errorf("ParseType(%q) = %q, want %q", tt.input, got, tt.wantType) - } - }) +func TestDefaultType(t *testing.T) { + if got := DefaultType(); got != TypeStory { + t.Errorf("DefaultType() = %q, want %q", got, TypeStory) } } -// testStatusDefs returns the standard test status definitions. -func testStatusDefs() []config.StatusDef { - return []config.StatusDef{ - {Key: "backlog", Label: "Backlog", Emoji: "📥", Default: true}, - {Key: "ready", Label: "Ready", Emoji: "📋", Active: true}, - {Key: "inProgress", Label: "In Progress", Emoji: "⚙️", Active: true}, - {Key: "review", Label: "Review", Emoji: "👀", Active: true}, - {Key: "done", Label: "Done", Emoji: "✅", Done: true}, +// TestPreInitPanics verifies that type helpers fail before registries are loaded. +func TestPreInitPanics(t *testing.T) { + config.ClearStatusRegistry() + t.Cleanup(func() { + config.ResetStatusRegistry(defaultTestStatusDefs()) + }) + + assertPanics := func(name string, fn func()) { + t.Helper() + defer func() { + if r := recover(); r == nil { + t.Errorf("%s: expected panic before registry init", name) + } + }() + fn() } + + assertPanics("ParseType", func() { ParseType("story") }) + assertPanics("AllTypes", func() { AllTypes() }) + assertPanics("DefaultType", func() { DefaultType() }) + assertPanics("TypeLabel", func() { TypeLabel(TypeStory) }) + assertPanics("ParseDisplay", func() { ParseDisplay("Story 🌀") }) } diff --git a/task/validation.go b/task/validation.go index 984a7d4..c463a07 100644 --- a/task/validation.go +++ b/task/validation.go @@ -38,7 +38,7 @@ func ValidateStatus(t *Task) string { // ValidateType returns an error message if the task type is invalid. func ValidateType(t *Task) string { - if currentTypeRegistry().IsValid(t.Type) { + if requireTypeRegistry().IsValid(t.Type) { return "" } return fmt.Sprintf("invalid type value: %s", t.Type) diff --git a/view/help/custom.md b/view/help/custom.md index 8796340..e8e335e 100644 --- a/view/help/custom.md +++ b/view/help/custom.md @@ -215,7 +215,7 @@ update where id = id() set assignee=user() - `id` - task identifier (e.g., "TIKI-M7N2XK") - `title` - task title text -- `type` - task type: "story", "bug", "spike", or "epic" +- `type` - task type (must match a key defined in `workflow.yaml` types) - `status` - workflow status (must match a key defined in `workflow.yaml` statuses) - `assignee` - assigned user - `priority` - numeric priority value (1-5) diff --git a/view/help/tiki.md b/view/help/tiki.md index b04688c..a82fb2d 100644 --- a/view/help/tiki.md +++ b/view/help/tiki.md @@ -91,10 +91,11 @@ title: Implement user authentication #### type -Optional string. Default: `story`. +Optional string. Defaults to the first type defined in `workflow.yaml`. -Valid values: `story`, `bug`, `spike`, `epic`. Aliases `feature` and `task` resolve to `story`. -In the TUI each type has an icon: Story 🌀, Bug 💥, Spike 🔍, Epic 🗂️. +Valid values are the type keys defined in the `types:` section of `workflow.yaml`. +Default types: `story`, `bug`, `spike`, `epic`. Each type can have a label and emoji +configured in `workflow.yaml`. Aliases are not supported; use the canonical key. ```yaml type: bug diff --git a/workflow/status.go b/workflow/status.go index 96c0c11..3d222f0 100644 --- a/workflow/status.go +++ b/workflow/status.go @@ -2,7 +2,6 @@ package workflow import ( "fmt" - "log/slog" "strings" ) @@ -100,8 +99,24 @@ func splitCamelCase(s string) []string { return words } +// StatusDisplay returns "Label Emoji" for a status. +func StatusDisplay(def StatusDef) string { + label := def.Label + if label == "" { + label = def.Key + } + emoji := strings.TrimSpace(def.Emoji) + if emoji == "" { + return label + } + return label + " " + emoji +} + // NewStatusRegistry constructs a StatusRegistry from the given definitions. -// Returns an error if keys are empty, duplicated, or the list is empty. +// Configured keys must be canonical (matching NormalizeStatusKey output). +// Labels default to key when omitted; explicitly empty labels are rejected. +// Emoji values are trimmed. Requires exactly one default and one done status. +// Duplicate display strings are rejected. func NewStatusRegistry(defs []StatusDef) (*StatusRegistry, error) { if len(defs) == 0 { return nil, fmt.Errorf("statuses list is empty") @@ -111,46 +126,64 @@ func NewStatusRegistry(defs []StatusDef) (*StatusRegistry, error) { statuses: make([]StatusDef, 0, len(defs)), byKey: make(map[StatusKey]StatusDef, len(defs)), } + displaySeen := make(map[string]StatusKey, len(defs)) for i, def := range defs { if def.Key == "" { return nil, fmt.Errorf("status at index %d has empty key", i) } - normalized := NormalizeStatusKey(def.Key) - def.Key = string(normalized) - - if _, exists := reg.byKey[normalized]; exists { - return nil, fmt.Errorf("duplicate status key %q", normalized) + // require canonical key + canonical := NormalizeStatusKey(def.Key) + if def.Key != string(canonical) { + return nil, fmt.Errorf("status key %q is not canonical; use %q", def.Key, canonical) } + def.Key = string(canonical) + + if _, exists := reg.byKey[canonical]; exists { + return nil, fmt.Errorf("duplicate status key %q", canonical) + } + + // label: default to key when omitted, reject explicit empty/whitespace + if def.Label == "" { + def.Label = def.Key + } else if strings.TrimSpace(def.Label) == "" { + return nil, fmt.Errorf("status %q has empty/whitespace label", def.Key) + } + + // emoji: trim whitespace + def.Emoji = strings.TrimSpace(def.Emoji) + + // reject duplicate display strings + display := StatusDisplay(def) + if existingKey, exists := displaySeen[display]; exists { + return nil, fmt.Errorf("duplicate status display %q: statuses %q and %q", display, existingKey, canonical) + } + displaySeen[display] = canonical if def.Default { if reg.defaultKey != "" { - slog.Warn("multiple statuses marked default; using first", "first", reg.defaultKey, "duplicate", normalized) - } else { - reg.defaultKey = normalized + return nil, fmt.Errorf("multiple statuses marked default: %q and %q", reg.defaultKey, canonical) } + reg.defaultKey = canonical } if def.Done { if reg.doneKey != "" { - slog.Warn("multiple statuses marked done; using first", "first", reg.doneKey, "duplicate", normalized) - } else { - reg.doneKey = normalized + return nil, fmt.Errorf("multiple statuses marked done: %q and %q", reg.doneKey, canonical) } + reg.doneKey = canonical } - reg.byKey[normalized] = def + reg.byKey[canonical] = def reg.statuses = append(reg.statuses, def) } - // if no explicit default, use the first status if reg.defaultKey == "" { - reg.defaultKey = StatusKey(reg.statuses[0].Key) - slog.Warn("no status marked default; using first status", "key", reg.defaultKey) + return nil, fmt.Errorf("no status marked default: true; exactly one is required") } if reg.doneKey == "" { - slog.Warn("no status marked done; task completion features may not work correctly") + return nil, fmt.Errorf("no status marked done: true; exactly one is required") } return reg, nil diff --git a/workflow/status_test.go b/workflow/status_test.go index 3d4cecc..4d2c787 100644 --- a/workflow/status_test.go +++ b/workflow/status_test.go @@ -177,18 +177,25 @@ func TestStatusRegistry_Keys(t *testing.T) { } } -func TestStatusRegistry_NormalizesKeys(t *testing.T) { - custom := []StatusDef{ +func TestStatusRegistry_RejectsNonCanonicalKeys(t *testing.T) { + _, err := NewStatusRegistry([]StatusDef{ {Key: "In-Progress", Label: "In Progress", Default: true}, - {Key: " DONE ", Label: "Done", Done: true}, + {Key: "done", Label: "Done", Done: true}, + }) + if err == nil { + t.Fatal("expected error for non-canonical key") } - reg := mustBuildStatusRegistry(t, custom) +} + +func TestStatusRegistry_CanonicalKeysWork(t *testing.T) { + reg := mustBuildStatusRegistry(t, defaultTestStatuses()) if !reg.IsValid("inProgress") { - t.Error("expected 'inProgress' to be valid after normalization") + t.Error("expected 'inProgress' to be valid") } - if !reg.IsValid("done") { - t.Error("expected 'done' to be valid after normalization") + // lookup normalizes input — "In-Progress" splits on separator to get "inProgress" + if !reg.IsValid("In-Progress") { + t.Error("expected 'In-Progress' to be valid via normalization") } } @@ -220,17 +227,25 @@ func TestNewStatusRegistry_Empty(t *testing.T) { } } -func TestNewStatusRegistry_DefaultFallsToFirst(t *testing.T) { +func TestNewStatusRegistry_RequiresDefault(t *testing.T) { defs := []StatusDef{ - {Key: "alpha", Label: "Alpha"}, + {Key: "alpha", Label: "Alpha", Done: true}, {Key: "beta", Label: "Beta"}, } - reg, err := NewStatusRegistry(defs) - if err != nil { - t.Fatalf("unexpected error: %v", err) + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error when no status is marked default") } - if reg.DefaultKey() != "alpha" { - t.Errorf("expected default to fall back to first status 'alpha', got %q", reg.DefaultKey()) +} + +func TestNewStatusRegistry_RequiresDone(t *testing.T) { + defs := []StatusDef{ + {Key: "alpha", Label: "Alpha", Default: true}, + {Key: "beta", Label: "Beta"}, + } + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error when no status is marked done") } } @@ -247,51 +262,62 @@ func TestStatusRegistry_AllReturnsCopy(t *testing.T) { } } -func TestNewStatusRegistry_MultipleDoneWarns(t *testing.T) { +func TestNewStatusRegistry_MultipleDoneErrors(t *testing.T) { defs := []StatusDef{ {Key: "alpha", Label: "Alpha", Default: true, Done: true}, {Key: "beta", Label: "Beta", Done: true}, } - reg, err := NewStatusRegistry(defs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - // first done wins - if reg.DoneKey() != "alpha" { - t.Errorf("expected done key 'alpha', got %q", reg.DoneKey()) + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for multiple done statuses") } } -func TestNewStatusRegistry_MultipleDefaultWarns(t *testing.T) { +func TestNewStatusRegistry_MultipleDefaultErrors(t *testing.T) { defs := []StatusDef{ {Key: "alpha", Label: "Alpha", Default: true}, - {Key: "beta", Label: "Beta", Default: true}, + {Key: "beta", Label: "Beta", Default: true, Done: true}, } - reg, err := NewStatusRegistry(defs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - // first default wins - if reg.DefaultKey() != "alpha" { - t.Errorf("expected default key 'alpha', got %q", reg.DefaultKey()) + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for multiple default statuses") } } -func TestNewStatusRegistry_NoDoneKey(t *testing.T) { +func TestNewStatusRegistry_DuplicateDisplay(t *testing.T) { defs := []StatusDef{ - {Key: "alpha", Label: "Alpha", Default: true}, - {Key: "beta", Label: "Beta"}, + {Key: "alpha", Label: "Open", Emoji: "🟢", Default: true}, + {Key: "beta", Label: "Open", Emoji: "🟢", Done: true}, + } + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for duplicate display") + } +} + +func TestNewStatusRegistry_LabelDefaultsToKey(t *testing.T) { + defs := []StatusDef{ + {Key: "alpha", Emoji: "🔵", Default: true}, + {Key: "beta", Emoji: "🔴", Done: true}, } reg, err := NewStatusRegistry(defs) if err != nil { t.Fatalf("unexpected error: %v", err) } - if reg.DoneKey() != "" { - t.Errorf("expected empty done key, got %q", reg.DoneKey()) + def, _ := reg.Lookup("alpha") + if def.Label != "alpha" { + t.Errorf("expected label to default to key, got %q", def.Label) } - // IsDone should return false for all statuses - if reg.IsDone("alpha") { - t.Error("expected alpha to not be done") +} + +func TestNewStatusRegistry_EmptyWhitespaceLabel(t *testing.T) { + defs := []StatusDef{ + {Key: "alpha", Label: " ", Default: true}, + {Key: "beta", Done: true}, + } + _, err := NewStatusRegistry(defs) + if err == nil { + t.Fatal("expected error for whitespace-only label") } } diff --git a/workflow/tasktype.go b/workflow/tasktype.go index db8fe36..f0000a1 100644 --- a/workflow/tasktype.go +++ b/workflow/tasktype.go @@ -16,18 +16,18 @@ const ( TypeEpic TaskType = "epic" ) -// TypeDef defines a single task type with metadata and aliases. +// TypeDef defines a single task type with display metadata. +// Keys must be canonical (matching NormalizeTypeKey output). type TypeDef struct { - Key TaskType - Label string - Emoji string - Aliases []string // e.g. "feature" and "task" → story + Key TaskType `yaml:"key"` + Label string `yaml:"label,omitempty"` + Emoji string `yaml:"emoji,omitempty"` } // DefaultTypeDefs returns the built-in type definitions. func DefaultTypeDefs() []TypeDef { return []TypeDef{ - {Key: TypeStory, Label: "Story", Emoji: "🌀", Aliases: []string{"feature", "task"}}, + {Key: TypeStory, Label: "Story", Emoji: "🌀"}, {Key: TypeBug, Label: "Bug", Emoji: "💥"}, {Key: TypeSpike, Label: "Spike", Emoji: "🔍"}, {Key: TypeEpic, Label: "Epic", Emoji: "🗂️"}, @@ -35,16 +35,15 @@ func DefaultTypeDefs() []TypeDef { } // TypeRegistry is an ordered collection of valid task types. -// It is constructed from a list of TypeDef and provides lookup and normalization. +// Unknown input is never silently coerced — ParseType returns ("", false). type TypeRegistry struct { - types []TypeDef - byKey map[TaskType]TypeDef - byAlias map[string]TaskType // normalized alias → canonical key - fallback TaskType // returned for unknown types + types []TypeDef + byKey map[TaskType]TypeDef + byDisplay map[string]TaskType // display string → canonical key } // NormalizeTypeKey lowercases, trims, and strips all separators ("-", "_", " "). -// Built-in type keys are single words, so stripping is lossless. +// Used to compute the canonical form of a type key for validation. func NormalizeTypeKey(s string) TaskType { s = strings.ToLower(strings.TrimSpace(s)) s = strings.ReplaceAll(s, "_", "") @@ -54,54 +53,68 @@ func NormalizeTypeKey(s string) TaskType { } // NewTypeRegistry constructs a TypeRegistry from the given definitions. -// The first definition's key is used as the fallback for unknown types. +// Configured keys must already be canonical (matching NormalizeTypeKey output). +// Labels default to the key when omitted; explicitly empty labels are rejected. +// Emoji values are trimmed; duplicate display strings are rejected. func NewTypeRegistry(defs []TypeDef) (*TypeRegistry, error) { if len(defs) == 0 { return nil, fmt.Errorf("type definitions list is empty") } reg := &TypeRegistry{ - types: make([]TypeDef, 0, len(defs)), - byKey: make(map[TaskType]TypeDef, len(defs)), - byAlias: make(map[string]TaskType), - fallback: NormalizeTypeKey(string(defs[0].Key)), + types: make([]TypeDef, 0, len(defs)), + byKey: make(map[TaskType]TypeDef, len(defs)), + byDisplay: make(map[string]TaskType, len(defs)), } - // first pass: register all primary keys for i, def := range defs { if def.Key == "" { return nil, fmt.Errorf("type at index %d has empty key", i) } - normalized := NormalizeTypeKey(string(def.Key)) - def.Key = normalized - defs[i] = def + // require canonical key + canonical := NormalizeTypeKey(string(def.Key)) + if def.Key != canonical { + return nil, fmt.Errorf("type key %q is not canonical; use %q", def.Key, canonical) + } + def.Key = canonical - if _, exists := reg.byKey[normalized]; exists { - return nil, fmt.Errorf("duplicate type key %q", normalized) + if _, exists := reg.byKey[canonical]; exists { + return nil, fmt.Errorf("duplicate type key %q", canonical) } - reg.byKey[normalized] = def + // label: default to key when omitted, reject explicit empty/whitespace + if def.Label == "" { + def.Label = string(def.Key) + } else if strings.TrimSpace(def.Label) == "" { + return nil, fmt.Errorf("type %q has empty/whitespace label", def.Key) + } + + // emoji: trim whitespace + def.Emoji = strings.TrimSpace(def.Emoji) + + // compute display and reject duplicates + display := typeDisplay(def.Label, def.Emoji) + if existingKey, exists := reg.byDisplay[display]; exists { + return nil, fmt.Errorf("duplicate type display %q: types %q and %q", display, existingKey, def.Key) + } + reg.byDisplay[display] = def.Key + + reg.byKey[canonical] = def reg.types = append(reg.types, def) } - // second pass: register aliases against the complete key set - for _, def := range defs { - for _, alias := range def.Aliases { - normAlias := string(NormalizeTypeKey(alias)) - if existing, ok := reg.byAlias[normAlias]; ok { - return nil, fmt.Errorf("duplicate alias %q (already maps to %q)", alias, existing) - } - if _, ok := reg.byKey[TaskType(normAlias)]; ok { - return nil, fmt.Errorf("alias %q collides with primary key", alias) - } - reg.byAlias[normAlias] = def.Key - } - } - return reg, nil } +// typeDisplay computes "Label Emoji" from parts (shared by constructor and method). +func typeDisplay(label, emoji string) string { + if emoji == "" { + return label + } + return label + " " + emoji +} + // Lookup returns the TypeDef for a given key (normalized) and whether it exists. func (r *TypeRegistry) Lookup(key TaskType) (TypeDef, bool) { def, ok := r.byKey[NormalizeTypeKey(string(key))] @@ -109,29 +122,14 @@ func (r *TypeRegistry) Lookup(key TaskType) (TypeDef, bool) { } // ParseType parses a raw string into a TaskType with validation. -// Returns the canonical key and true if recognized (including aliases), -// or (fallback, false) for unknown types. +// Returns the canonical key and true if recognized, +// or ("", false) for unknown types. No fallback, no coercion. func (r *TypeRegistry) ParseType(s string) (TaskType, bool) { normalized := NormalizeTypeKey(s) - - // check primary keys if _, ok := r.byKey[normalized]; ok { return normalized, true } - - // check aliases - if canonical, ok := r.byAlias[string(normalized)]; ok { - return canonical, true - } - - return r.fallback, false -} - -// NormalizeType normalizes a raw string into a TaskType. -// Unknown types default to the fallback (first registered type). -func (r *TypeRegistry) NormalizeType(s string) TaskType { - t, _ := r.ParseType(s) - return t + return "", false } // TypeLabel returns the human-readable label for a task type. @@ -154,21 +152,22 @@ func (r *TypeRegistry) TypeEmoji(t TaskType) string { func (r *TypeRegistry) TypeDisplay(t TaskType) string { label := r.TypeLabel(t) emoji := r.TypeEmoji(t) - if emoji == "" { - return label - } - return label + " " + emoji + return typeDisplay(label, emoji) } // ParseDisplay reverses a TypeDisplay() string (e.g. "Bug 💥") back to -// its canonical key. Returns (key, true) on match, or (fallback, false). +// its canonical key. Returns (key, true) on match, or ("", false). func (r *TypeRegistry) ParseDisplay(display string) (TaskType, bool) { - for _, def := range r.types { - if r.TypeDisplay(def.Key) == display { - return def.Key, true - } + if key, ok := r.byDisplay[display]; ok { + return key, true } - return r.fallback, false + return "", false +} + +// DefaultType returns the first configured type key — used as the creation +// default when no type is specified. Requires at least one registered type. +func (r *TypeRegistry) DefaultType() TaskType { + return r.types[0].Key } // Keys returns all type keys in definition order. @@ -188,7 +187,7 @@ func (r *TypeRegistry) All() []TypeDef { return result } -// IsValid reports whether key is a recognized type (primary key only, not alias). +// IsValid reports whether key is a recognized type. func (r *TypeRegistry) IsValid(key TaskType) bool { _, ok := r.byKey[NormalizeTypeKey(string(key))] return ok diff --git a/workflow/tasktype_test.go b/workflow/tasktype_test.go index bb69fc0..a36e2a5 100644 --- a/workflow/tasktype_test.go +++ b/workflow/tasktype_test.go @@ -48,12 +48,10 @@ func TestTypeRegistry_ParseType(t *testing.T) { {"bug", "bug", TypeBug, true}, {"spike", "spike", TypeSpike, true}, {"epic", "epic", TypeEpic, true}, - {"feature alias", "feature", TypeStory, true}, - {"task alias", "task", TypeStory, true}, {"case insensitive", "Story", TypeStory, true}, {"uppercase", "BUG", TypeBug, true}, - {"unknown", "unknown", TypeStory, false}, - {"empty", "", TypeStory, false}, + {"unknown returns empty", "unknown", "", false}, + {"empty returns empty", "", "", false}, } for _, tt := range tests { @@ -66,29 +64,29 @@ func TestTypeRegistry_ParseType(t *testing.T) { } } -func TestTypeRegistry_NormalizeType(t *testing.T) { +func TestTypeRegistry_ParseDisplay(t *testing.T) { reg := mustBuildTypeRegistry(t, DefaultTypeDefs()) tests := []struct { - name string - input string - want TaskType + name string + input string + want TaskType + wantOK bool }{ - {"story", "story", TypeStory}, - {"bug", "bug", TypeBug}, - {"spike", "spike", TypeSpike}, - {"epic", "epic", TypeEpic}, - {"feature alias", "feature", TypeStory}, - {"task alias", "task", TypeStory}, - {"case insensitive", "EPIC", TypeEpic}, - {"unknown defaults to story", "unknown", TypeStory}, - {"empty defaults to story", "", TypeStory}, + {"story display", "Story 🌀", TypeStory, true}, + {"bug display", "Bug 💥", TypeBug, true}, + {"spike display", "Spike 🔍", TypeSpike, true}, + {"epic display", "Epic 🗂️", TypeEpic, true}, + {"unknown returns empty", "Unknown", "", false}, + {"label only", "Bug", "", false}, + {"empty returns empty", "", "", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := reg.NormalizeType(tt.input); got != tt.want { - t.Errorf("NormalizeType(%q) = %q, want %q", tt.input, got, tt.want) + got, ok := reg.ParseDisplay(tt.input) + if got != tt.want || ok != tt.wantOK { + t.Errorf("ParseDisplay(%q) = (%q, %v), want (%q, %v)", tt.input, got, ok, tt.want, tt.wantOK) } }) } @@ -163,31 +161,19 @@ func TestTypeRegistry_TypeDisplay(t *testing.T) { } } -func TestTypeRegistry_ParseDisplay(t *testing.T) { +func TestTypeRegistry_DefaultType(t *testing.T) { reg := mustBuildTypeRegistry(t, DefaultTypeDefs()) - - tests := []struct { - name string - input string - want TaskType - wantOK bool - }{ - {"story display", "Story 🌀", TypeStory, true}, - {"bug display", "Bug 💥", TypeBug, true}, - {"spike display", "Spike 🔍", TypeSpike, true}, - {"epic display", "Epic 🗂️", TypeEpic, true}, - {"unknown display", "Unknown", TypeStory, false}, - {"label only", "Bug", TypeStory, false}, - {"empty", "", TypeStory, false}, + if got := reg.DefaultType(); got != TypeStory { + t.Errorf("DefaultType() = %q, want %q", got, TypeStory) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, ok := reg.ParseDisplay(tt.input) - if got != tt.want || ok != tt.wantOK { - t.Errorf("ParseDisplay(%q) = (%q, %v), want (%q, %v)", tt.input, got, ok, tt.want, tt.wantOK) - } - }) + // custom registry: first type is the default + custom := mustBuildTypeRegistry(t, []TypeDef{ + {Key: "task", Label: "Task"}, + {Key: "bug", Label: "Bug"}, + }) + if got := custom.DefaultType(); got != "task" { + t.Errorf("DefaultType() = %q, want %q", got, "task") } } @@ -213,9 +199,6 @@ func TestTypeRegistry_IsValid(t *testing.T) { if !reg.IsValid(TypeStory) { t.Error("expected story to be valid") } - if reg.IsValid("feature") { - t.Error("expected alias 'feature' to not be valid as primary key") - } if reg.IsValid("unknown") { t.Error("expected unknown to not be valid") } @@ -224,7 +207,6 @@ func TestTypeRegistry_IsValid(t *testing.T) { func TestTypeRegistry_LookupNormalizesInput(t *testing.T) { reg := mustBuildTypeRegistry(t, DefaultTypeDefs()) - // Lookup should normalize inputs just like StatusRegistry does tests := []struct { name string input TaskType @@ -245,7 +227,6 @@ func TestTypeRegistry_LookupNormalizesInput(t *testing.T) { }) } - // TypeLabel/TypeEmoji/IsValid should also normalize if label := reg.TypeLabel("BUG"); label != "Bug" { t.Errorf("TypeLabel(BUG) = %q, want %q", label, "Bug") } @@ -276,54 +257,64 @@ func TestNewTypeRegistry_DuplicateKey(t *testing.T) { } } -func TestNewTypeRegistry_DuplicateAlias(t *testing.T) { - defs := []TypeDef{ - {Key: "story", Label: "Story", Aliases: []string{"feature"}}, - {Key: "bug", Label: "Bug", Aliases: []string{"feature"}}, - } - _, err := NewTypeRegistry(defs) - if err == nil { - t.Error("expected error for duplicate alias") - } -} - -func TestNewTypeRegistry_AliasCollidesWithKey(t *testing.T) { - defs := []TypeDef{ - {Key: "story", Label: "Story"}, - {Key: "bug", Label: "Bug", Aliases: []string{"story"}}, - } - _, err := NewTypeRegistry(defs) - if err == nil { - t.Error("expected error when alias collides with primary key") - } -} - -func TestNewTypeRegistry_AliasCollidesWithLaterKey(t *testing.T) { - // alias "feature" on story should collide with later primary key "feature" - defs := []TypeDef{ - {Key: "story", Label: "Story", Aliases: []string{"feature"}}, - {Key: "feature", Label: "Feature"}, - } - _, err := NewTypeRegistry(defs) - if err == nil { - t.Error("expected error when alias collides with a later primary key") - } -} - -func TestNewTypeRegistry_FallbackNormalized(t *testing.T) { +func TestNewTypeRegistry_NonCanonicalKey(t *testing.T) { defs := []TypeDef{ {Key: "Story", Label: "Story"}, - {Key: "bug", Label: "Bug"}, } - reg := mustBuildTypeRegistry(t, defs) + _, err := NewTypeRegistry(defs) + if err == nil { + t.Fatal("expected error for non-canonical key") + } + if got := err.Error(); got != `type key "Story" is not canonical; use "story"` { + t.Errorf("got error %q", got) + } +} - // fallback should be normalized even though the input key was "Story" - got, ok := reg.ParseType("unknown-thing") - if ok { - t.Fatal("expected ok=false for unknown type") +func TestNewTypeRegistry_LabelDefaultsToKey(t *testing.T) { + reg := mustBuildTypeRegistry(t, []TypeDef{ + {Key: "task", Emoji: "📋"}, + }) + if got := reg.TypeLabel("task"); got != "task" { + t.Errorf("expected label to default to key, got %q", got) } - if got != "story" { - t.Errorf("ParseType(unknown) = %q, want %q (normalized fallback)", got, "story") +} + +func TestNewTypeRegistry_EmptyWhitespaceLabel(t *testing.T) { + _, err := NewTypeRegistry([]TypeDef{ + {Key: "task", Label: " "}, + }) + if err == nil { + t.Error("expected error for whitespace-only label") + } +} + +func TestNewTypeRegistry_EmojiTrimmed(t *testing.T) { + reg := mustBuildTypeRegistry(t, []TypeDef{ + {Key: "task", Label: "Task", Emoji: " 🔧 "}, + }) + if got := reg.TypeEmoji("task"); got != "🔧" { + t.Errorf("expected trimmed emoji, got %q", got) + } +} + +func TestNewTypeRegistry_DuplicateDisplay(t *testing.T) { + _, err := NewTypeRegistry([]TypeDef{ + {Key: "task", Label: "Item", Emoji: "📋"}, + {Key: "work", Label: "Item", Emoji: "📋"}, + }) + if err == nil { + t.Error("expected error for duplicate display") + } +} + +func TestNewTypeRegistry_DuplicateDisplayLabelOnly(t *testing.T) { + // duplicate label with no emoji — display is just the label + _, err := NewTypeRegistry([]TypeDef{ + {Key: "task", Label: "Item"}, + {Key: "work", Label: "Item"}, + }) + if err == nil { + t.Error("expected error for duplicate label-only display") } }