Skip to content

fix(sync): list autosync writes by subcommand #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcus merged 1 commit into from
Oct 17, 2026
+117 −40
Merged

fix(sync): list autosync writes by subcommand #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 52 additions & 36 deletions cmd/autosync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <id>` 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 <id>`).
"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.
Expand Down
69 changes: 65 additions & 4 deletions cmd/autosync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@ 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)
}
}

// 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)
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
Loading