diff --git a/internal/cli/quest_test.go b/internal/cli/quest_test.go index 6192d82..76da31d 100644 --- a/internal/cli/quest_test.go +++ b/internal/cli/quest_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "database/sql" + "encoding/json" "path/filepath" "strings" "testing" @@ -197,6 +198,83 @@ func TestCLI_QuestEpic(t *testing.T) { } } +// TestCLI_QuestClearDeprecationStderr verifies that invoking `quest clear` +// (cobra alias for quest fulfill) writes the deprecation notice to stderr and +// not to stdout, and that --json mode suppresses it entirely. QUEST-138. +func TestCLI_QuestClearDeprecationStderr(t *testing.T) { + setupQuestCLI(t, "guild-cli-depr") + const proj = "guild-cli-depr" + + // Post a quest to fulfill. + if _, _, err := runQuest(t, []string{"quest", "post", "-p", proj, "depr-test"}); err != nil { + t.Fatalf("post: %v", err) + } + + t.Run("human_mode_notice_on_stderr_not_stdout", func(t *testing.T) { + stdout, stderr, err := runQuest(t, []string{"quest", "clear", + "-p", proj, "QUEST-1", "--report", "done"}) + if err != nil { + t.Fatalf("clear: %v", err) + } + // Success line must be on stdout. + if !strings.Contains(stdout, "fulfilled QUEST-1") { + t.Errorf("stdout missing success line; got: %q", stdout) + } + // Deprecation notice must be on stderr. + if !strings.Contains(stderr, "deprecated") { + t.Errorf("stderr missing deprecation notice; got: %q", stderr) + } + if !strings.Contains(stderr, "quest_fulfill") { + t.Errorf("stderr missing 'quest_fulfill' pointer; got: %q", stderr) + } + // Deprecation notice must NOT appear on stdout. + if strings.Contains(stdout, "deprecated") { + t.Errorf("deprecation notice leaked to stdout; got: %q", stdout) + } + }) + + // Post another quest for the json-mode test. + if _, _, err := runQuest(t, []string{"quest", "post", "-p", proj, "depr-json-test"}); err != nil { + t.Fatalf("post 2: %v", err) + } + + t.Run("json_mode_no_deprecation_notice", func(t *testing.T) { + stdout, stderr, err := runQuest(t, []string{"quest", "clear", + "-p", proj, "QUEST-2", "--report", "done", "--json"}) + if err != nil { + t.Fatalf("clear --json: %v", err) + } + // stdout must be valid JSON without any deprecation content. + if strings.Contains(stdout, "deprecated") { + t.Errorf("deprecation notice leaked into JSON stdout; got: %q", stdout) + } + // stderr must be empty in --json mode. + if strings.Contains(stderr, "deprecated") { + t.Errorf("deprecation notice leaked into stderr in --json mode; got: %q", stderr) + } + // Sanity: stdout is parseable JSON. + var out map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(stdout)), &out); err != nil { + t.Errorf("stdout is not valid JSON: %v\nstdout: %q", err, stdout) + } + }) + + // Verify quest fulfill (primary verb) does NOT emit the notice. + if _, _, err := runQuest(t, []string{"quest", "post", "-p", proj, "fulfill-clean"}); err != nil { + t.Fatalf("post 3: %v", err) + } + t.Run("fulfill_no_deprecation_notice", func(t *testing.T) { + _, stderr, err := runQuest(t, []string{"quest", "fulfill", + "-p", proj, "QUEST-3", "--report", "done"}) + if err != nil { + t.Fatalf("fulfill: %v", err) + } + if strings.Contains(stderr, "deprecated") { + t.Errorf("deprecation notice fired on quest fulfill (primary verb); stderr: %q", stderr) + } + }) +} + // readStatusOwner reads the (status, claimed_by) pair for (pid, tid). func readStatusOwner(t *testing.T, db *sql.DB, pid, tid string) (status, owner string) { t.Helper() diff --git a/internal/command/cobra.go b/internal/command/cobra.go index cb2a429..3f3bdba 100644 --- a/internal/command/cobra.go +++ b/internal/command/cobra.go @@ -106,6 +106,9 @@ func (c *Command[I, O]) cobraRunE(d Deps) func(*cobra.Command, []string) error { _, _ = fmt.Fprint(cc.ErrOrStderr(), w) } } + if notice, ok := c.CLIAliasDeprecations[cc.CalledAs()]; ok && notice != "" { + _, _ = fmt.Fprintln(cc.ErrOrStderr(), notice) + } return nil } } diff --git a/internal/command/command.go b/internal/command/command.go index a0970ed..cebf0ce 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -55,6 +55,11 @@ type Command[I, O any] struct { // the command still exits 0. MCP callers never invoke this; they // encode warnings inside their structured MCPFormat output instead. CLIWarnings func(s CLISink, o O) string + // CLIAliasDeprecations maps a cobra alias name to a deprecation notice + // that should be written to stderr when the command is invoked via that + // alias. The notice is emitted after CLIWarnings, only in human-rendering + // mode (not --json), so scripted consumers see no output change. + CLIAliasDeprecations map[string]string // MCPFormat renders the handler's output for the MCP surface. // Receives a concrete MCPSink with compact-output primitives. // Required unless CLIOnly=true. diff --git a/internal/quest/clear_cmd.go b/internal/quest/clear_cmd.go index 0aa7a8c..ba6b556 100644 --- a/internal/quest/clear_cmd.go +++ b/internal/quest/clear_cmd.go @@ -27,6 +27,13 @@ type FulfillOutput struct { type ClearInput = FulfillInput type ClearOutput = FulfillOutput +// clearDeprecationNotice is the one-line migration gradient emitted +// whenever an agent or user invokes the legacy quest_clear / quest clear +// alias. Positioned after the success line so it doesn't crowd the main +// output. JSON mode is never affected (CLI path exits before this renders; +// MCP format appends it outside the JSON schema). QUEST-138 / LORE-122. +const clearDeprecationNotice = "⚠️ [deprecated] quest_clear will be removed — use quest_fulfill" + // FulfillCommand is the primary quest-completion verb. `quest clear` stays // as a cobra alias for muscle-memory; `quest_fulfill` is the canonical MCP // tool name. QUEST-106 / LORE-80. @@ -109,6 +116,12 @@ var FulfillCommand = &command.Command[FulfillInput, FulfillOutput]{ MCPFormat: func(s command.MCPSink, o FulfillOutput) string { return formatFulfilled(s, o) }, CLIErrorFormat: func(s command.CLISink, err error) (string, bool) { return formatFulfillError(s, err) }, MCPErrorFormat: func(s command.MCPSink, err error) (string, bool) { return formatFulfillError(s, err) }, + // Emit the deprecation notice to stderr when invoked as the `clear` + // alias so agents and users get a migration gradient. JSON mode is + // unaffected — the cobra adapter exits before reaching this path. + CLIAliasDeprecations: map[string]string{ + "clear": clearDeprecationNotice, + }, } // ClearCommand is the backward-compat MCP alias for FulfillCommand. It @@ -123,6 +136,12 @@ var ClearCommand = func() *command.Command[FulfillInput, FulfillOutput] { c.MCPOnly = true c.CLIAliases = nil // avoid cobra registering an orphan alias c.Short = "fulfill a quest (alias for quest_fulfill; cascades unblock)" + // Override MCPFormat to append the deprecation notice after the success + // line. This gives agents a clear pointer to the canonical verb without + // polluting the primary quest_fulfill output. QUEST-138 / LORE-122. + c.MCPFormat = func(s command.MCPSink, o FulfillOutput) string { + return formatFulfilled(s, o) + "\n" + clearDeprecationNotice + } return &c }() diff --git a/internal/quest/clear_cmd_test.go b/internal/quest/clear_cmd_test.go index 0288b41..940d391 100644 --- a/internal/quest/clear_cmd_test.go +++ b/internal/quest/clear_cmd_test.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" + "github.com/mathomhaus/guild/internal/command" "github.com/mathomhaus/guild/internal/quest" ) @@ -68,7 +69,8 @@ func TestFulfillCommand_MCPSurface(t *testing.T) { // TestClearCommand_MCPBackwardCompat verifies the MCP-only alias tool is // still advertised under the legacy name `quest_clear` so agents trained -// on the pre-QUEST-106 verb continue to work. +// on the pre-QUEST-106 verb continue to work, and that its rendered output +// includes the deprecation notice. QUEST-138 / LORE-122. func TestClearCommand_MCPBackwardCompat(t *testing.T) { tool := quest.ClearCommand.BuildMCPForTest(fakeDeps(t)) if tool.Name != "quest_clear" { @@ -78,6 +80,34 @@ func TestClearCommand_MCPBackwardCompat(t *testing.T) { if !strings.Contains(tool.Description, "Fulfill") { t.Errorf("alias description should reference fulfill; got %q", tool.Description) } + + // The MCPFormat for the alias must include the deprecation notice so + // agents get a migration gradient. Build a minimal FulfillOutput and + // render it through the alias formatter. + out := quest.FulfillOutput{ + Result: &quest.FulfillResult{ + Cleared: &quest.Quest{ID: "QUEST-99", Subject: "test"}, + }, + } + rendered := quest.ClearCommand.MCPFormat(command.MCPSink{}, out) + if !strings.Contains(rendered, "deprecated") { + t.Errorf("ClearCommand MCP output missing deprecation notice; got:\n%s", rendered) + } + if !strings.Contains(rendered, "quest_fulfill") { + t.Errorf("ClearCommand MCP output missing 'quest_fulfill' pointer; got:\n%s", rendered) + } + // The deprecation notice must appear AFTER the success line. + successIdx := strings.Index(rendered, "QUEST-99") + deprIdx := strings.Index(rendered, "deprecated") + if successIdx < 0 || deprIdx < 0 || deprIdx <= successIdx { + t.Errorf("deprecation notice not positioned after success line; rendered:\n%s", rendered) + } + + // quest_fulfill's MCPFormat must NOT include the deprecation notice. + renderedFulfill := quest.FulfillCommand.MCPFormat(command.MCPSink{}, out) + if strings.Contains(renderedFulfill, "deprecated") { + t.Errorf("FulfillCommand MCP output must not include deprecation notice; got:\n%s", renderedFulfill) + } } // findSubcommand returns the direct child of parent named name, or nil.