Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
78 changes: 78 additions & 0 deletions internal/cli/quest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"database/sql"
"encoding/json"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions internal/command/cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
5 changes: 5 additions & 0 deletions internal/command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 19 additions & 0 deletions internal/quest/clear_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
}()

Expand Down
32 changes: 31 additions & 1 deletion internal/quest/clear_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/spf13/cobra"

"github.com/mathomhaus/guild/internal/command"
"github.com/mathomhaus/guild/internal/quest"
)

Expand Down Expand Up @@ -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" {
Expand All @@ -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.
Expand Down
Loading