feat(quest): error on --depends-on "" instead of silent no-op

docs/CAMPAIGNS.md

@@ -57,21 +57,23 @@ Create a new campaign when:
   single new campaign would meaningfully clarify reporting.
 
 If you would have to explain the campaign in two sentences, it's probably
-two campaigns. If you're not sure, reuse — `quest epic` (the rename verb)
-exists and is cheap to apply later.
+two campaigns. If you're not sure, reuse — `quest campaign` (the rename
+verb) exists and is cheap to apply later.
 
 ## Setting and changing the campaign on a quest
 
 Set the campaign at quest creation time, or apply it to a batch of existing
 quests with the rename verb:
 
 ```bash
-guild quest epic <campaign-name> QUEST-1 QUEST-2 ...
+guild quest campaign <campaign-name> QUEST-1 QUEST-2 ...
+# `quest epic` works as an alias for the same operation.
 ```
 
-`quest epic` is an alias for the same operation; the underlying field is
-`campaign`. Use whichever spelling reads more naturally to you. The MCP
-tool is `quest_epic` with `--campaign` and `--epic` both accepted as input.
+`quest campaign` is the canonical CLI verb; `quest epic` is registered as
+an alias. The underlying field is `campaign` and the MCP tool name stays
+`quest_epic` for backward compatibility, with `--campaign` and `--epic`
+both accepted as input.
 
 ## Out of scope
 
@@ -81,4 +83,5 @@ file a quest for the merge work itself rather than treating it as a docs
 update.
 
 There is also no `guild campaign` verb to manage the campaign list
-programmatically. Filings happen via `quest epic` on the affected quests.
+programmatically. Filings happen via `quest campaign` on the affected
+quests.

internal/quest/types.go

@@ -80,6 +80,13 @@ var (
 	// ReplaceFiles).
 	ErrConflictingUpdate = errors.New("conflicting update: cannot set both append and replace for the same field")
 
+	// ErrEmptyDependsOn is returned when an Update call passes only
+	// empty/whitespace strings as DependsOn entries without also setting
+	// ClearDependsOn or ReplaceDependsOn. The historical behavior was a
+	// silent no-op, which made `--depends-on ""` indistinguishable from
+	// "no change" for callers (especially agents). Issue #47.
+	ErrEmptyDependsOn = errors.New("depends_on contains only empty values: use --clear-depends-on to remove all dependencies")
+
 	// ErrAlreadyDone is returned by Forfeit when the target quest is
 	// status='done'. Forfeit refuses to silently reopen a completed
 	// quest — the caller should explicitly rework or reopen.

internal/quest/update.go

@@ -70,6 +70,27 @@ func Update(ctx context.Context, db *sql.DB, projectID, taskID string, params Up
 		return nil, fmt.Errorf("%w: %s", ErrConflictingUpdate, conflicts[0])
 	}
 
+	// Issue #47: callers (especially agents) sometimes pass `--depends-on ""`
+	// or `depends_on=[""]` expecting it to mean "drop all dependencies". The
+	// downstream loop trims/skips empty strings and the call silently no-ops,
+	// which is indistinguishable from "no change". Detect that shape and
+	// redirect to --clear-depends-on rather than failing silently. The check
+	// only fires when ClearDependsOn / ReplaceDependsOn are NOT also set, so
+	// a normal `--depends-on QUEST-1 --depends-on ""` call (an empty entry
+	// mixed with real ones) keeps the trim-and-skip behavior.
+	if len(params.DependsOn) > 0 && len(params.ReplaceDependsOn) == 0 && !params.ClearDependsOn {
+		allEmpty := true
+		for _, d := range params.DependsOn {
+			if strings.TrimSpace(d) != "" {
+				allEmpty = false
+				break
+			}
+		}
+		if allEmpty {
+			return nil, ErrEmptyDependsOn
+		}
+	}
+
 	agent := agentOrDefault(params.Agent)
 	now := time.Now().UTC().Format(time.RFC3339Nano)
 

internal/quest/update_test.go

@@ -279,3 +279,71 @@ func TestUpdate_ClearAcceptance(t *testing.T) {
 		t.Errorf("acceptance = %v, want empty", got.Acceptance)
 	}
 }
+
+// TestUpdate_ClearDependsOn_BeforeTwoAfterZero is the explicit acceptance-criterion
+// shape for issue #47: a quest with two deps, --clear-depends-on drops both.
+func TestUpdate_ClearDependsOn_BeforeTwoAfterZero(t *testing.T) {
+	db, pid := newTestDB(t)
+	ctx := context.Background()
+	a := mustPost(t, db, pid, PostParams{Subject: "A"})
+	c := mustPost(t, db, pid, PostParams{Subject: "C"})
+	b := mustPost(t, db, pid, PostParams{Subject: "B", DependsOn: []string{a.ID, c.ID}})
+
+	before := mustLoad(t, db, pid, b.ID)
+	if len(before.DependsOn) != 2 {
+		t.Fatalf("B before clear: deps = %v, want 2", before.DependsOn)
+	}
+
+	if _, err := Update(ctx, db, pid, b.ID, UpdateParams{ClearDependsOn: true}); err != nil {
+		t.Fatalf("Update ClearDependsOn: %v", err)
+	}
+
+	after := mustLoad(t, db, pid, b.ID)
+	if len(after.DependsOn) != 0 {
+		t.Errorf("B after clear: deps = %v, want empty", after.DependsOn)
+	}
+}
+
+// TestUpdate_EmptyDependsOn_RedirectsToClear covers issue #47 acceptance criterion #3:
+// `--depends-on ""` (empty/whitespace-only entries) without --clear-depends-on or
+// --replace-depends-on must error pointing at --clear-depends-on rather than
+// silently no-op'ing.
+func TestUpdate_EmptyDependsOn_RedirectsToClear(t *testing.T) {
+	db, pid := newTestDB(t)
+	ctx := context.Background()
+	a := mustPost(t, db, pid, PostParams{Subject: "A"})
+	b := mustPost(t, db, pid, PostParams{Subject: "B", DependsOn: []string{a.ID}})
+
+	cases := []struct {
+		name string
+		deps []string
+	}{
+		{"single empty", []string{""}},
+		{"single whitespace", []string{"   "}},
+		{"multiple empty", []string{"", "  ", "\t"}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := Update(ctx, db, pid, b.ID, UpdateParams{DependsOn: tc.deps})
+			if !errors.Is(err, ErrEmptyDependsOn) {
+				t.Fatalf("Update with deps=%v: err = %v, want ErrEmptyDependsOn", tc.deps, err)
+			}
+			// State must be untouched on error. Use Load(ctx, ...) directly so
+			// contextcheck sees the parent ctx flow into the subtest closure
+			// (the mustLoad helper synthesizes a fresh context.Background()).
+			reloaded, err := Load(ctx, db, pid, b.ID)
+			if err != nil {
+				t.Fatalf("Load %s: %v", b.ID, err)
+			}
+			if len(reloaded.DependsOn) != 1 || reloaded.DependsOn[0] != a.ID {
+				t.Errorf("B deps after refused empty update = %v, want [%s]", reloaded.DependsOn, a.ID)
+			}
+		})
+	}
+
+	// Sanity: a mixed slice (real ID + empty) is NOT redirected — the trim/skip
+	// path still applies for that case so existing callers aren't broken.
+	if _, err := Update(ctx, db, pid, b.ID, UpdateParams{DependsOn: []string{a.ID, ""}}); err != nil {
+		t.Fatalf("Update with mixed deps: unexpected error %v", err)
+	}
+}