fix: IMP-4/5/6 — parser allowlist, delete ID resolution, consistent errors

IMP-5: Replace strings.Contains(arg, ":") heuristic with an allowlist
of recognized attribute keys (ValidAttributeKeys). Colons in task
descriptions (URLs, "Meeting: topic") are no longer misinterpreted as
modifiers. Canonical key sets live in engine/keys.go and are shared
across parseAddArgs, ParseFilter, and ParseModifier. ParseModifier now
errors on unknown keys.

IMP-4: delete command now loads the working set and resolves display IDs
via GetTaskByDisplayID, matching the pattern used by done/modify.

IMP-6: All action commands (done, delete, modify, start, stop) now
return an error on no-match (stderr, exit 1). Previously done/delete
printed to stdout and exited 0; start/stop had no check at all.

Also adds requirements and design docs for the CLI UX improvements.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-19 13:37:33 +01:00
parent a551f50cef
commit f57baee6bc
11 changed files with 1573 additions and 32 deletions
+16 -10
View File
@@ -71,23 +71,29 @@ func addTask(args []string) error {
return nil
}
// parseAddArgs extracts description and modifiers from args
// Description = all non-filter/modifier words joined with spaces
// Filters/Modifiers = args with +, -, or containing :
// parseAddArgs extracts description and modifiers from args.
// Tags (+tag, -tag) are always modifiers. For key:value tokens, only
// recognized attribute keys (engine.ValidAttributeKeys) are treated as
// modifiers — everything else becomes part of the description.
func parseAddArgs(args []string) (string, []string, error) {
var descParts []string
var modifiers []string
for _, arg := range args {
isFilterOrModifier := strings.HasPrefix(arg, "+") ||
strings.HasPrefix(arg, "-") ||
strings.Contains(arg, ":")
if isFilterOrModifier {
if strings.HasPrefix(arg, "+") || strings.HasPrefix(arg, "-") {
modifiers = append(modifiers, arg)
} else {
descParts = append(descParts, arg)
continue
}
if idx := strings.Index(arg, ":"); idx > 0 {
key := arg[:idx]
if engine.ValidAttributeKeys[key] {
modifiers = append(modifiers, arg)
continue
}
}
descParts = append(descParts, arg)
}
if len(descParts) == 0 {
+21 -4
View File
@@ -26,14 +26,31 @@ func deleteTasks(args []string) error {
return err
}
tasks, err := engine.GetTasks(filter)
// Load working set to resolve display IDs (matches done/modify pattern)
ws, err := engine.LoadWorkingSet()
if err != nil {
return err
return fmt.Errorf("failed to load working set: %w", err)
}
var tasks []*engine.Task
if len(filter.IDs) > 0 {
for _, id := range filter.IDs {
task, err := ws.GetTaskByDisplayID(id)
if err != nil {
return err
}
tasks = append(tasks, task)
}
} else {
tasks, err = engine.GetTasks(filter)
if err != nil {
return err
}
}
if len(tasks) == 0 {
fmt.Println("No tasks matched.")
return nil
return fmt.Errorf("no tasks matched filter")
}
fmt.Printf("Delete %d task(s)? (y/N): ", len(tasks))
+1 -2
View File
@@ -63,8 +63,7 @@ func completeTasks(args []string) error {
}
if len(tasks) == 0 {
fmt.Println("No tasks matched.")
return nil
return fmt.Errorf("no tasks matched filter")
}
// Confirm if multiple tasks
+1 -1
View File
@@ -82,7 +82,7 @@ func modifyTasks(filterArgs, modifierArgs []string) error {
}
if len(tasks) == 0 {
return fmt.Errorf("no tasks matched")
return fmt.Errorf("no tasks matched filter")
}
// Confirm if multiple tasks or no filters specified
+4
View File
@@ -43,6 +43,10 @@ func startTasks(args []string) error {
}
}
if len(tasks) == 0 {
return fmt.Errorf("no tasks matched filter")
}
for _, task := range tasks {
task.StartTask()
fmt.Printf("Started task: %s\n", task.Description)
+4
View File
@@ -43,6 +43,10 @@ func stopTasks(args []string) error {
}
}
if len(tasks) == 0 {
return fmt.Errorf("no tasks matched filter")
}
for _, task := range tasks {
task.StopTask()
fmt.Printf("Stopped task: %s\n", task.Description)
+8 -7
View File
@@ -42,17 +42,18 @@ func ParseFilter(args []string) (*Filter, error) {
} else if strings.HasPrefix(arg, "-") && !strings.Contains(arg, ":") {
// Exclude tag (but not negative modifiers like priority:-)
f.ExcludeTags = append(f.ExcludeTags, strings.TrimPrefix(arg, "-"))
} else if strings.Contains(arg, ":") {
// Attribute filter
parts := strings.SplitN(arg, ":", 2)
key := parts[0]
value := parts[1]
} else if idx := strings.Index(arg, ":"); idx > 0 {
key := arg[:idx]
value := arg[idx+1:]
if key == "uuid" {
if FilterOnlyKeys[key] {
// Filter-only keys (e.g., uuid)
f.UUIDs = append(f.UUIDs, value)
} else {
} else if ValidAttributeKeys[key] {
// Known attribute filter
f.Attributes[key] = value
}
// Unrecognized key:value tokens are silently ignored
} else {
// Try parsing as numeric ID
id, err := strconv.Atoi(arg)
+30
View File
@@ -0,0 +1,30 @@
package engine
// ValidAttributeKeys are the recognized key:value attribute names.
// Used by parseAddArgs (cmd/add.go), ParseFilter, and ParseModifier
// to distinguish modifiers from description text.
var ValidAttributeKeys = map[string]bool{
"due": true,
"priority": true,
"project": true,
"recur": true,
"status": true,
"wait": true,
"scheduled": true,
"until": true,
}
// DateKeys is the subset of ValidAttributeKeys that hold date values.
// Used by Modifier.Apply and Modifier.ApplyToNew for date parsing.
var DateKeys = map[string]bool{
"due": true,
"scheduled": true,
"wait": true,
"until": true,
}
// FilterOnlyKeys are additional keys valid in filter context but not as
// modifiers (e.g., uuid is a filter target, not something you set via modify).
var FilterOnlyKeys = map[string]bool{
"uuid": true,
}
+12 -8
View File
@@ -23,7 +23,9 @@ func NewModifier() *Modifier {
}
}
// ParseModifier parses command-line args into Modifier
// ParseModifier parses command-line args into Modifier.
// Only recognized attribute keys (ValidAttributeKeys) are accepted;
// unrecognized key:value tokens produce an error.
func ParseModifier(args []string) (*Modifier, error) {
m := NewModifier()
@@ -34,11 +36,13 @@ func ParseModifier(args []string) (*Modifier, error) {
} else if strings.HasPrefix(arg, "-") && !strings.Contains(arg, ":") {
// Remove tag
m.RemoveTags = append(m.RemoveTags, strings.TrimPrefix(arg, "-"))
} else if strings.Contains(arg, ":") {
// Attribute modification
parts := strings.SplitN(arg, ":", 2)
key := parts[0]
value := parts[1]
} else if idx := strings.Index(arg, ":"); idx > 0 {
key := arg[:idx]
value := arg[idx+1:]
if !ValidAttributeKeys[key] {
return nil, fmt.Errorf("unknown modifier: %q (known: due, priority, project, recur, status, wait, scheduled, until)", key)
}
if value == "" {
// Clear attribute (priority: with no value)
@@ -83,7 +87,7 @@ func (m *Modifier) Apply(task *Task) error {
resolvedDates["modified"] = task.Modified
// Apply attributes in the order they were specified (important for relative references)
dateKeys := map[string]bool{"due": true, "scheduled": true, "wait": true, "until": true}
dateKeys := DateKeys
for _, key := range m.AttributeOrder {
valuePtr := m.SetAttributes[key]
@@ -151,7 +155,7 @@ func (m *Modifier) ApplyToNew(task *Task) error {
}
// Apply attributes in the order they were specified (important for relative references)
dateKeys := map[string]bool{"due": true, "scheduled": true, "wait": true, "until": true}
dateKeys := DateKeys
for _, key := range m.AttributeOrder {
valuePtr := m.SetAttributes[key]