From efb4b9a1fead0d930ebee60f66caab3401ea68db Mon Sep 17 00:00:00 2001 From: Justin Bishop Date: Sat, 17 Oct 2026 11:14:17 -0700 Subject: [PATCH] fix(sync): list autosync writes by subcommand blocked-by, depends-on, focus, unfocus, and the list, show, and current subcommands of board, epic, task, and ws write nothing that syncs, so they no longer push and pull after they run. defer, due, and note's subcommands that write now sync after they run. A test fails when one of these parents gains a subcommand that is not classified. Co-Authored-By: Claude Opus 5.5 --- cmd/autosync.go | 88 ++++++++++++++++++++++++++------------------ cmd/autosync_test.go | 69 ++++++++++++++++++++++++++++++++-- 2 files changed, 117 insertions(+), 40 deletions(-) diff --git a/cmd/autosync.go b/cmd/autosync.go index 49087561..fd521096 100644 --- a/cmd/autosync.go +++ b/cmd/autosync.go @@ -34,44 +34,60 @@ var ( strandedWarnMu sync.Mutex ) -// mutatingCommands lists commands that modify local data and should trigger auto-sync. -// A top-level name also covers its subcommands. A full path such as "comments add" -// covers just that subcommand, for a parent whose own run only reads. +// mutatingCommands lists commands that write data that syncs, so they should trigger auto-sync. +// A top-level name also covers its subcommands. A full path such as "board create" +// covers just that subcommand. var mutatingCommands = map[string]bool{ - "create": true, - "update": true, - "delete": true, - "restore": true, - "start": true, - "unstart": true, - "close": true, - "block": true, - "unblock": true, - "reopen": true, - "review": true, - "approve": true, - "reject": true, - "log": true, - "handoff": true, - "focus": true, - "unfocus": true, - "link": true, - "unlink": true, - "comment": true, - "undo": true, - "import": true, - "init": true, - "board": true, - "dep": true, - "blocked-by": true, - "depends-on": true, - "epic": true, - "task": true, - "ws": true, - "monitor": true, + "create": true, + "update": true, + "delete": true, + "restore": true, + "start": true, + "unstart": true, + "close": true, + "block": true, + "unblock": true, + "reopen": true, + "review": true, + "approve": true, + "reject": true, + "log": true, + "handoff": true, + "link": true, + "unlink": true, + "comment": true, + "undo": true, + "import": true, + "init": true, + "dep": true, + "defer": true, + "due": true, + "monitor": true, - // Not "comments": `td comments ` only lists. - "comments add": true, + // These parents also have subcommands that only read, so only the ones that + // write are listed. `comments` itself only lists (`td comments `). + "board create": true, + "board delete": true, + "board edit": true, + "board move": true, + "board unposition": true, + "comments add": true, + "epic create": true, + "note add": true, + "note archive": true, + "note delete": true, + "note edit": true, + "note pin": true, + "note restore": true, + "note unarchive": true, + "note unpin": true, + "task create": true, + "ws end": true, + "ws handoff": true, + "ws log": true, + "ws start": true, + "ws tag": true, + "ws untag": true, } // isMutatingCommand checks if the given command name triggers auto-sync. diff --git a/cmd/autosync_test.go b/cmd/autosync_test.go index c236d7cd..0b816b61 100644 --- a/cmd/autosync_test.go +++ b/cmd/autosync_test.go @@ -11,7 +11,8 @@ import ( func TestIsMutatingCommand(t *testing.T) { // Commands that should trigger auto-sync - mutating := []string{"create", "update", "delete", "start", "close", "log", "handoff", "board", "dep", "ws", "comments add", "monitor"} + mutating := []string{"create", "update", "delete", "start", "close", "log", "handoff", "dep", "defer", "due", "monitor", + "board create", "comments add", "epic create", "note add", "task create", "ws start"} for _, name := range mutating { if !isMutatingCommand(name) { t.Errorf("expected %q to be mutating", name) @@ -19,7 +20,8 @@ func TestIsMutatingCommand(t *testing.T) { } // Commands that should NOT trigger auto-sync - readOnly := []string{"list", "show", "search", "query", "sync", "auth", "status", "info", "version", "help", "doctor", "comments"} + readOnly := []string{"list", "show", "search", "query", "sync", "auth", "status", "info", "version", "help", "doctor", "comments", + "blocked-by", "depends-on", "focus", "unfocus", "board", "epic", "note", "task", "ws"} for _, name := range readOnly { if isMutatingCommand(name) { t.Errorf("expected %q to NOT be mutating", name) @@ -35,7 +37,25 @@ func TestIsMutatingInvocation(t *testing.T) { {[]string{"comments", "td-1"}, false}, {[]string{"comments", "add", "td-1", "text"}, true}, {[]string{"comment", "td-1", "text"}, true}, - {[]string{"board", "list"}, true}, + {[]string{"board", "list"}, false}, + {[]string{"board", "show", "b"}, false}, + {[]string{"board", "create", "b"}, true}, + {[]string{"epic", "list"}, false}, + {[]string{"epic", "create", "e"}, true}, + {[]string{"task", "list"}, false}, + {[]string{"task", "create", "t"}, true}, + {[]string{"ws", "current"}, false}, + {[]string{"ws", "show", "w"}, false}, + {[]string{"ws", "start", "w"}, true}, + {[]string{"note", "list"}, false}, + {[]string{"note", "show", "n"}, false}, + {[]string{"note", "add", "n"}, true}, + {[]string{"blocked-by", "td-1"}, false}, + {[]string{"depends-on", "td-1"}, false}, + {[]string{"focus", "td-1"}, false}, + {[]string{"unfocus"}, false}, + {[]string{"defer", "td-1", "+1w"}, true}, + {[]string{"due", "td-1", "friday"}, true}, {[]string{"list"}, false}, } for _, tt := range tests { @@ -51,7 +71,43 @@ func TestIsMutatingInvocation(t *testing.T) { } } -func TestMutationHookSkipsCommentsListing(t *testing.T) { +// TestSubcommandsAreClassified fails when a parent whose writes are listed one +// subcommand at a time gains a subcommand that is neither listed as mutating +// nor known to be read-only, so a new write cannot silently skip autosync. +func TestSubcommandsAreClassified(t *testing.T) { + readOnly := map[string]bool{ + "board list": true, + "board show": true, + "epic list": true, + "note list": true, + "note show": true, + "task list": true, + "ws current": true, + "ws list": true, + "ws show": true, + } + for _, parent := range []string{"board", "comments", "epic", "note", "task", "ws"} { + cmd, _, err := rootCmd.Find([]string{parent}) + if err != nil { + t.Fatalf("find %q: %v", parent, err) + } + if !cmd.HasSubCommands() { + t.Fatalf("%q has no subcommands", parent) + } + for _, sub := range cmd.Commands() { + path := strings.TrimPrefix(sub.CommandPath(), rootCmd.CommandPath()+" ") + if mutating := isMutatingInvocation(sub); mutating == readOnly[path] { + t.Errorf("%q: isMutatingInvocation = %v; list it in mutatingCommands if it writes, or only in this test's read-only list if it only reads", path, mutating) + } + } + } +} + +func TestMutationHookRunsOnlyForMutatingCommands(t *testing.T) { + // The hook runs only while the autosync gate is open, so ignore any kill-switch + // in the developer's environment. + t.Setenv("TD_FEATURE_SYNC_AUTOSYNC", "") + t.Setenv("TD_SYNC_AUTO", "") t.Setenv("TD_AUTH_KEY", "test-key") dir := setupSyncStateDir(t, "proj-test", false) saveAndRestoreGlobals(t) @@ -70,6 +126,11 @@ func TestMutationHookSkipsCommentsListing(t *testing.T) { }{ {[]string{"comments", "td-1"}, 0}, {[]string{"comments", "add", "td-1", "text"}, 1}, + {[]string{"board", "list"}, 0}, + {[]string{"board", "create", "b"}, 1}, + {[]string{"blocked-by", "td-1"}, 0}, + {[]string{"due", "td-1", "friday"}, 1}, + {[]string{"note", "add", "n"}, 1}, } for _, tt := range tests { t.Run(strings.Join(tt.args, " "), func(t *testing.T) {