From d05bd60d1978dfb6345c986b0754590e819427f5 Mon Sep 17 00:00:00 2001 From: Kunal Lanjewar <5488221+kunallanjewar@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:09:11 -0700 Subject: [PATCH] =?UTF-8?q?feat(quest):=20rename=20quest=5Fclear=20?= =?UTF-8?q?=E2=86=92=20quest=5Ffulfill=20with=20backward-compat=20alias?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QUEST-106 / LORE-80. Plain-English readers were mapping "clear" to "wipe/delete" β€” fulfill is the classic RPG verb for completing a quest, unambiguous in either register. Matches QUEST-16's epicβ†’campaign rename pattern (primary+alias, no schema changes). User-facing: `guild quest fulfill` / `quest_fulfill` is primary. `guild quest clear` remains as a cobra alias; `quest_clear` remains as an MCP-only alias tool pointing at the same handler. Output strings use "fulfilled" and the \[fulfilled\] prefix; emoji πŸ† kept. Internal: FulfillCommand is canonical; ClearCommand is a struct-copy with Name override + MCPOnly=true. Fulfill() is the canonical domain func; Clear is a var alias. Type aliases ClearInput=FulfillInput, ClearOutput=FulfillOutput, ClearResult=FulfillResult keep existing callers compiling. Tests: renamed CobraSurface/MCPSurface tests for fulfill primary; added MCPBackwardCompat test for clear alias. Budget ceiling bumped 4500β†’4700 for the duplicate tool (net +~100 tokens). tool lists + minArgsFor updated. INSTRUCTIONS.md examples flipped to fulfill with clear note for discoverability. README Act 3 updated. --- README.md | 2 +- docs/generated/cli.md | 24 ++++---- docs/generated/mcp.md | 43 ++++++++++++-- internal/cli/quest.go | 2 +- internal/cli/quest_test.go | 25 ++++---- internal/mcp/budget_test.go | 7 ++- internal/mcp/instructions.md | 8 +-- internal/mcp/instructions_test.go | 5 +- internal/mcp/register.go | 4 ++ internal/mcp/smoke_full_test.go | 1 + internal/mcp/tools_test.go | 10 +++- internal/quest/clear.go | 12 ++-- internal/quest/clear_cmd.go | 94 +++++++++++++++++++------------ internal/quest/clear_cmd_test.go | 51 +++++++++++++---- internal/quest/types.go | 12 ++-- 15 files changed, 204 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index ea80dfa..f162325 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ blocked on QUEST-42 is now available for whoever walks in next. ```bash guild quest brief "shipped retry in commit abc1234; QUEST-43 ready to start" -guild quest clear QUEST-42 --report "done, shipped in abc1234" +guild quest fulfill QUEST-42 --report "done, shipped in abc1234" ``` Tomorrow's agent β€” same project, maybe a different MCP client β€” opens diff --git a/docs/generated/cli.md b/docs/generated/cli.md index 27b5a26..e8dd635 100644 --- a/docs/generated/cli.md +++ b/docs/generated/cli.md @@ -45,8 +45,8 @@ Every `guild ` subcommand generated from the live cobra tree. - [`guild quest brief`](#guild-quest-brief) β€” write session-end handoff for the next agent - [`guild quest campaign`](#guild-quest-campaign) β€” bulk-set campaign - [`guild quest campfire`](#guild-quest-campfire) β€” save working state before compaction -- [`guild quest clear`](#guild-quest-clear) β€” complete a quest (cascades unblock) - [`guild quest forfeit`](#guild-quest-forfeit) β€” release a claim without completing +- [`guild quest fulfill`](#guild-quest-fulfill) β€” fulfill a quest (cascades unblock) - [`guild quest guild`](#guild-quest-guild) β€” per-project summary by campaign - [`guild quest init`](#guild-quest-init) β€” register this repo as a guild project - [`guild quest journal`](#guild-quest-journal) β€” append a task-scoped journal note @@ -811,43 +811,45 @@ Save a structured working-state checkpoint (hypothesis, tried, next, token warni | `--token-warning` | bool | `false` | context window is running low | | `--tried` | stringArray | `[]` | approach tried (repeatable) | -## `guild quest clear` +## `guild quest forfeit` -complete a quest (cascades unblock) +release a claim without completing **Usage** ``` -guild quest clear QUEST_ID [flags] +guild quest forfeit QUEST_ID [flags] ``` -Complete a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests. +Release a claimed quest back to the queue. Use when blocked or ceding to another agent. Only acts on status=in_progress quests β€” refuses on done and no-ops on next. **Flags** | flag | type | default | description | | --- | --- | --- | --- | | `--json` | bool | `false` | emit structured JSON result instead of formatted text | -| `--report` | string | `β€”` | completion report: commit hash, files, remaining issues | +| `--note` | string | `β€”` | reason the claim is being released (optional) | -## `guild quest forfeit` +## `guild quest fulfill` -release a claim without completing +fulfill a quest (cascades unblock) **Usage** ``` -guild quest forfeit QUEST_ID [flags] +guild quest fulfill QUEST_ID [flags] ``` -Release a claimed quest back to the queue. Use when blocked or ceding to another agent. Only acts on status=in_progress quests β€” refuses on done and no-ops on next. +**Aliases:** clear + +Fulfill a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests. **Flags** | flag | type | default | description | | --- | --- | --- | --- | | `--json` | bool | `false` | emit structured JSON result instead of formatted text | -| `--note` | string | `β€”` | reason the claim is being released (optional) | +| `--report` | string | `β€”` | completion report: commit hash, files, remaining issues | ## `guild quest guild` diff --git a/docs/generated/mcp.md b/docs/generated/mcp.md index a786251..9251494 100644 --- a/docs/generated/mcp.md +++ b/docs/generated/mcp.md @@ -2,7 +2,7 @@ # guild MCP tool catalog -Every tool a registered MCP client sees. 37 tools. +Every tool a registered MCP client sees. 38 tools. ## Tools @@ -31,9 +31,10 @@ Every tool a registered MCP client sees. 37 tools. - [`quest_bounties`](#quest_bounties) β€” On-demand session snapshot: brief, oath, echoes, top task, and parallel candidates. - [`quest_brief`](#quest_brief) β€” Session-end briefing for the next agent. - [`quest_campfire`](#quest_campfire) β€” Save a structured working-state checkpoint (hypothesis, tried, next, token warning) before the context compacts. -- [`quest_clear`](#quest_clear) β€” Complete a quest. +- [`quest_clear`](#quest_clear) β€” Fulfill a quest. - [`quest_epic`](#quest_epic) β€” Set the campaign name on a batch of quests. - [`quest_forfeit`](#quest_forfeit) β€” Release a claimed quest back to the queue. +- [`quest_fulfill`](#quest_fulfill) β€” Fulfill a quest. - [`quest_guild`](#quest_guild) β€” Per-project quest summary grouped by campaign: counts of next, in-progress, blocked, and done quests. - [`quest_journal`](#quest_journal) β€” Task-scoped scratchpad. - [`quest_list`](#quest_list) β€” All open tasks. @@ -914,7 +915,7 @@ _no arguments_ ## `quest_clear` -Complete a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests. +Fulfill a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests. _no arguments_ @@ -928,7 +929,7 @@ _no arguments_ "type": "string" }, "quest_id": { - "description": "QUEST-NNN to mark complete", + "description": "QUEST-NNN to fulfill", "type": "string" }, "report": { @@ -1022,6 +1023,40 @@ _no arguments_ +## `quest_fulfill` + +Fulfill a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests. + +_no arguments_ + +
Raw JSON schema + +```json +{ + "additionalProperties": false, + "properties": { + "project": { + "type": "string" + }, + "quest_id": { + "description": "QUEST-NNN to fulfill", + "type": "string" + }, + "report": { + "description": "specific completion report: commit hash, files, issues β€” REQUIRED", + "type": "string" + } + }, + "required": [ + "quest_id", + "report" + ], + "type": "object" +} +``` + +
+ ## `quest_guild` Per-project quest summary grouped by campaign: counts of next, in-progress, blocked, and done quests. diff --git a/internal/cli/quest.go b/internal/cli/quest.go index fabae34..d22b8e7 100644 --- a/internal/cli/quest.go +++ b/internal/cli/quest.go @@ -149,7 +149,7 @@ func init() { // concern, not verb concern). See docs/architecture/COMMAND_REGISTRY.md. deps := buildCLICommandDeps() bindRegistryVerb(questCmd, quest.AcceptCommand, deps, "quest accept") - bindRegistryVerb(questCmd, quest.ClearCommand, deps, "quest clear") + bindRegistryVerb(questCmd, quest.FulfillCommand, deps, "quest fulfill") bindRegistryVerb(questCmd, quest.ForfeitCommand, deps, "quest forfeit") bindRegistryVerb(questCmd, quest.JournalCommand, deps, "quest journal") bindRegistryVerb(questCmd, quest.BriefCommand, deps, "quest brief") diff --git a/internal/cli/quest_test.go b/internal/cli/quest_test.go index b29810f..6192d82 100644 --- a/internal/cli/quest_test.go +++ b/internal/cli/quest_test.go @@ -68,17 +68,15 @@ func TestCLI_QuestPostClearRoundTrip(t *testing.T) { t.Errorf("post stdout = %q, want contains 'posted QUEST-1: hello world'", stdout) } - // Clear. + // Fulfill (via the `clear` cobra alias to verify QUEST-106 backward compat). stdout, _, err = runQuest(t, []string{"quest", "clear", "--project", "guild-cli-test", "QUEST-1", "--report", "commit abc"}) if err != nil { - t.Fatalf("clear: %v", err) + t.Fatalf("fulfill: %v", err) } - // QUEST-45 unified clear output β€” was "quest cleared: X", now matches - // MCP's "cleared X" header with an indented unblocked block. - if !strings.Contains(stdout, "cleared QUEST-1") { - t.Errorf("clear stdout = %q, want to contain 'cleared QUEST-1'", stdout) + if !strings.Contains(stdout, "fulfilled QUEST-1") { + t.Errorf("fulfill stdout = %q, want to contain 'fulfilled QUEST-1'", stdout) } } @@ -94,18 +92,17 @@ func TestCLI_QuestPostCascade(t *testing.T) { "--depends-on", "QUEST-1", "B"}); err != nil { t.Fatalf("post B: %v", err) } - // Clear A β€” expect cascade πŸ”“ line for QUEST-2. - stdout, _, err := runQuest(t, []string{"quest", "clear", "-p", "guild-cli-casc", "QUEST-1"}) + // Fulfill A β€” expect cascade line for QUEST-2. Using `fulfill` as + // primary verb (QUEST-106). + stdout, _, err := runQuest(t, []string{"quest", "fulfill", "-p", "guild-cli-casc", "QUEST-1"}) if err != nil { - t.Fatalf("clear A: %v", err) + t.Fatalf("fulfill A: %v", err) } - // QUEST-45 unified clear output β€” cascade list is now indented - // under "unblocked:" rather than one "unblocked: " line. - if !strings.Contains(stdout, "cleared QUEST-1") { - t.Errorf("clear stdout missing cleared: %q", stdout) + if !strings.Contains(stdout, "fulfilled QUEST-1") { + t.Errorf("fulfill stdout missing fulfilled: %q", stdout) } if !strings.Contains(stdout, "QUEST-2") || !strings.Contains(stdout, "unblocked") { - t.Errorf("clear stdout missing cascade: %q", stdout) + t.Errorf("fulfill stdout missing cascade: %q", stdout) } } diff --git a/internal/mcp/budget_test.go b/internal/mcp/budget_test.go index f3acace..047a74f 100644 --- a/internal/mcp/budget_test.go +++ b/internal/mcp/budget_test.go @@ -26,8 +26,11 @@ const ( // registered tools (descriptions + input schemas). Raised to 4500 // in QUEST-101 to accommodate the 5 previously-deferred tools // (lore_seal, lore_catalog, quest_epic, quest_active, quest_forfeit) - // now in the always-on tier. Expected actual: ~4100-4300 tokens. - totalMaxTokens = 4500 + // now in the always-on tier. Raised to 4700 in QUEST-106 when + // quest_fulfill was added alongside quest_clear as a backward-compat + // alias β€” net +~100 tokens for the duplicate tool. Expected actual: + // ~4400-4600 tokens. + totalMaxTokens = 4700 ) // TestDescriptionBudget enforces the token budgets against the full diff --git a/internal/mcp/instructions.md b/internal/mcp/instructions.md index d6a27d2..44c5efb 100644 --- a/internal/mcp/instructions.md +++ b/internal/mcp/instructions.md @@ -2,7 +2,7 @@ You are working with a persistent agent-memory (**lore**) and task-coordination Together they make agents as autonomous as possible. The core loop runs without a human between tasks: - quest_bounties β†’ quest_accept β†’ work β†’ quest_clear(report=...) β†’ quest_bounties β†’ ... + quest_bounties β†’ quest_accept β†’ work β†’ quest_fulfill(report=...) β†’ quest_bounties β†’ ... Every adventurer is transient; the guild is eternal. Your job is to take a bounty, do it well, inscribe what's worth keeping, brief the next arrival, and end cleanly. @@ -132,12 +132,12 @@ If results are current, use them β€” do not re-research. If empty or stale, rese **You finished a quest:** ``` -quest_clear( +quest_fulfill( quest_id="QUEST-42", report="Fixed the race in retry budget accounting. Commit abc1234. Tests added in budget_test.go." ) ``` -Report is REQUIRED. Be specific. +Report is REQUIRED. Be specific. `quest_clear(quest_id="...", report="...")` also works as a backward-compat alias. **You hit a wall mid-quest and the context window is warning you:** ``` @@ -231,7 +231,7 @@ Use these shapes when you need a concrete schema: quest_accept(quest_id="QUEST-7") quest_journal(quest_id="QUEST-7", text="Found race in retry budget accounting.") quest_campfire(quest_id="QUEST-7", hypothesis="budget race", next="inspect retry state") - quest_clear(quest_id="QUEST-7", report="done in abc123; tests added") + quest_fulfill(quest_id="QUEST-7", report="done in abc123; tests added") quest_brief(text="Retry budget shipped; next session should profile p99 latency.") quest_bounties() quest_list(status="next") diff --git a/internal/mcp/instructions_test.go b/internal/mcp/instructions_test.go index b293e52..b87dc29 100644 --- a/internal/mcp/instructions_test.go +++ b/internal/mcp/instructions_test.go @@ -23,8 +23,9 @@ import ( // direct edits to instructions.md do. See QUEST-57 for the dynamic // build path and its separate tests. // -// Last updated for the lazy-load tool-discovery note (Codex + Cursor). -const wantStaticSHA = "325854527015b8efd79ab5d9b2b672119d196633fd9fad51f357150813135d48" +// Last updated for QUEST-106: quest_clear β†’ quest_fulfill rename with +// backward-compat alias note. +const wantStaticSHA = "4508848753ecaadb59348e720797424b1452469ab98ff8353893d5507a49481a" func TestStaticInstructions_Embedded(t *testing.T) { if staticInstructions == "" { diff --git a/internal/mcp/register.go b/internal/mcp/register.go index 6ec84a1..92984c4 100644 --- a/internal/mcp/register.go +++ b/internal/mcp/register.go @@ -94,6 +94,10 @@ func registerAlwaysOn(s *sdkmcp.Server) { quest.AcceptCommand.BindMCP(s, mcpDeps) quest.JournalCommand.BindMCP(s, mcpDeps) quest.CampfireCommand.BindMCP(s, mcpDeps) + quest.FulfillCommand.BindMCP(s, mcpDeps) + // quest_clear is kept as a backward-compat MCP alias (same handler, + // different tool name) so agents trained on the pre-QUEST-106 verb + // still work. Tool discovery surfaces both; new agents prefer fulfill. quest.ClearCommand.BindMCP(s, mcpDeps) quest.BriefCommand.BindMCP(s, mcpDeps) quest.SummonCommand.BindMCP(s, mcpDeps) diff --git a/internal/mcp/smoke_full_test.go b/internal/mcp/smoke_full_test.go index 730ed63..dd88164 100644 --- a/internal/mcp/smoke_full_test.go +++ b/internal/mcp/smoke_full_test.go @@ -60,6 +60,7 @@ var toolArgSpecs = map[string][]command.ArgSpec{ "quest_clear": quest.ClearCommand.Args, "quest_epic": quest.EpicCommand.Args, "quest_forfeit": quest.ForfeitCommand.Args, + "quest_fulfill": quest.FulfillCommand.Args, "quest_guild": quest.GuildCommand.Args, "quest_journal": quest.JournalCommand.Args, "quest_list": quest.ListCommand.Args, diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index df39ba3..112cf0a 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -46,9 +46,10 @@ var expectedTools = []struct { {"quest_bounties"}, {"quest_brief"}, {"quest_campfire"}, - {"quest_clear"}, + {"quest_clear"}, // backward-compat alias for quest_fulfill (QUEST-106) {"quest_epic"}, {"quest_forfeit"}, + {"quest_fulfill"}, {"quest_guild"}, {"quest_journal"}, {"quest_list"}, @@ -709,6 +710,8 @@ func minArgsFor(name string) map[string]any { return map[string]any{"quest_id": "QUEST-1"} case "quest_clear": return map[string]any{"quest_id": "QUEST-1", "report": "r"} + case "quest_fulfill": + return map[string]any{"quest_id": "QUEST-1", "report": "r"} case "quest_journal": return map[string]any{"quest_id": "QUEST-1", "text": "t"} case "quest_brief": @@ -793,6 +796,11 @@ func smokeArgsFor(name string) map[string]any { "quest_id": "QUEST-99999", "report": "r", "project": "testproj", } + case "quest_fulfill": + return map[string]any{ + "quest_id": "QUEST-99999", "report": "r", + "project": "testproj", + } case "quest_brief": return map[string]any{"text": "t", "project": "testproj"} case "quest_bounties": diff --git a/internal/quest/clear.go b/internal/quest/clear.go index fa86d3d..421205d 100644 --- a/internal/quest/clear.go +++ b/internal/quest/clear.go @@ -9,9 +9,13 @@ import ( "time" ) -// Clear marks taskID as status='done' and atomically runs the cascade- +// Clear is a backward-compat alias for Fulfill. Existing callers keep +// compiling; new code should call Fulfill directly. QUEST-106 / LORE-80. +var Clear = Fulfill + +// Fulfill marks taskID as status='done' and atomically runs the cascade- // unblock pass: every quest currently in status='blocked' whose full -// depends_on list is satisfied by the post-Clear done-set flips to +// depends_on list is satisfied by the post-Fulfill done-set flips to // status='next'. // // Non-negotiable correctness (QUEST-9 spec): the cascade must find @@ -28,7 +32,7 @@ import ( // payload) and as a `[completed] ` note in task_notes for // chronological history. If `report` is empty, only the event fires // (no note). -func Clear(ctx context.Context, db *sql.DB, projectID, taskID, report string) (*ClearResult, error) { +func Fulfill(ctx context.Context, db *sql.DB, projectID, taskID, report string) (*FulfillResult, error) { if db == nil { return nil, fmt.Errorf("quest: clear: nil db") } @@ -114,5 +118,5 @@ func Clear(ctx context.Context, db *sql.DB, projectID, taskID, report string) (* if err := tx.Commit(); err != nil { return nil, fmt.Errorf("quest: clear: commit: %w", err) } - return &ClearResult{Cleared: cleared, Unblocked: unblocked}, nil + return &FulfillResult{Cleared: cleared, Unblocked: unblocked}, nil } diff --git a/internal/quest/clear_cmd.go b/internal/quest/clear_cmd.go index 0e17453..0aa7a8c 100644 --- a/internal/quest/clear_cmd.go +++ b/internal/quest/clear_cmd.go @@ -10,35 +10,45 @@ import ( "github.com/mathomhaus/guild/internal/command" ) -type ClearInput struct { - QuestID string `json:"quest_id" jsonschema:"QUEST-NNN to mark complete"` +// FulfillInput carries the args for quest fulfillment. +// ClearInput is a backward-compat type alias. +type FulfillInput struct { + QuestID string `json:"quest_id" jsonschema:"QUEST-NNN to fulfill"` Report string `json:"report" jsonschema:"specific completion report: commit hash, files, issues β€” REQUIRED"` Project string `json:"project,omitempty"` } -type ClearOutput struct { - Result *ClearResult `json:"result"` - BriefHint string `json:"brief_hint,omitempty"` +type FulfillOutput struct { + Result *FulfillResult `json:"result"` + BriefHint string `json:"brief_hint,omitempty"` } -var ClearCommand = &command.Command[ClearInput, ClearOutput]{ - Name: "quest_clear", - CLIPath: []string{"quest", "clear"}, - Short: "complete a quest (cascades unblock)", - Long: "Complete a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests.", +// Backward-compat type aliases so existing callers keep compiling. +type ClearInput = FulfillInput +type ClearOutput = FulfillOutput + +// 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. +var FulfillCommand = &command.Command[FulfillInput, FulfillOutput]{ + Name: "quest_fulfill", + CLIPath: []string{"quest", "fulfill"}, + CLIAliases: []string{"clear"}, + Short: "fulfill a quest (cascades unblock)", + Long: "Fulfill a quest. Report is REQUIRED β€” commit hash, files, remaining issues. Cascades unblock dependent quests.", Args: []command.ArgSpec{ { Name: "quest_id", Kind: command.ArgPositional, Type: command.ArgString, Required: true, - Help: "QUEST-NNN to mark complete", + Help: "QUEST-NNN to mark fulfilled", }, { Name: "report", Kind: command.ArgFlag, Type: command.ArgString, - // Not Required at the domain layer β€” Clear() accepts empty + // Not Required at the domain layer β€” Fulfill() accepts empty // report and just writes the `done` event without a // [completed] note. Encourage callers to pass one via Help // phrasing, don't reject the call. @@ -52,62 +62,74 @@ var ClearCommand = &command.Command[ClearInput, ClearOutput]{ Help: "project override", }, }, - Handler: func(ctx context.Context, d command.Deps, in ClearInput) (ClearOutput, error) { + Handler: func(ctx context.Context, d command.Deps, in FulfillInput) (FulfillOutput, error) { if strings.TrimSpace(in.QuestID) == "" { - return ClearOutput{}, errors.New("quest_id required") + return FulfillOutput{}, errors.New("quest_id required") } db, err := d.OpenDB(ctx) if err != nil { - return ClearOutput{}, err + return FulfillOutput{}, err } defer func() { _ = db.Close() }() pid, err := d.ResolveProj(ctx, in.Project) if err != nil { - return ClearOutput{}, err + return FulfillOutput{}, err } - res, err := Clear(ctx, db, pid, in.QuestID, in.Report) + res, err := Fulfill(ctx, db, pid, in.QuestID, in.Report) if err != nil { - return ClearOutput{}, err + return FulfillOutput{}, err } - out := ClearOutput{Result: res} + out := FulfillOutput{Result: res} // Advisory hint: if no brief has been written recently, remind the - // caller to write a handoff before compacting. This is shipped via - // two paths, live together during the QUEST-58 migration window: + // caller to write a handoff before compacting. Two paths coexist + // during the QUEST-58 migration window: // // 1. HintExtras signal β€” the hint engine's no-brief-24h rule reads // __hints_brief_stale and fires the canonical πŸ’‘ line. Preferred. // 2. out.BriefHint β€” legacy field; still populated so CLI paths - // that don't yet route through the engine (and backward-compat - // consumers of ClearOutput) keep their hint. The MCP format - // function drops it when the engine is wired so we don't - // render twice. + // that don't yet route through the engine keep their hint. The + // MCP format function drops it when the engine is wired so we + // don't render twice. lastAt, briefErr := LastBriefAt(ctx, db, pid) - stale := briefErr == nil && (lastAt.IsZero() || time.Now().Sub(lastAt) > briefStaleThreshold) + stale := briefErr == nil && (lastAt.IsZero() || time.Since(lastAt) > briefStaleThreshold) if stale { if extras := command.HintExtras(ctx); extras != nil { extras["__hints_brief_stale"] = true } else { - // No engine wired (test or cold-path CLI surface) β€” fall - // back to the legacy field so output stays consistent. out.BriefHint = `no quest_brief yet this session β€” consider quest_brief("what was done, what's next") before compact` } } return out, nil }, - CLIFormat: func(s command.CLISink, o ClearOutput) string { return formatCleared(s, o) }, - MCPFormat: func(s command.MCPSink, o ClearOutput) string { return formatCleared(s, o) }, - CLIErrorFormat: func(s command.CLISink, err error) (string, bool) { return formatClearError(s, err) }, - MCPErrorFormat: func(s command.MCPSink, err error) (string, bool) { return formatClearError(s, err) }, + CLIFormat: func(s command.CLISink, o FulfillOutput) string { return formatFulfilled(s, o) }, + 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) }, } -func formatCleared(s lineListSink, o ClearOutput) string { +// ClearCommand is the backward-compat MCP alias for FulfillCommand. It +// points at the same handler + formatters; only the tool name differs. +// Agents trained on `quest_clear` still work; new agents see `quest_fulfill` +// in tool discovery. CLIOnly=false, MCPOnly=true β€” CLI alias is handled +// via FulfillCommand.CLIAliases, so this struct stays off the CLI surface +// to avoid duplicate cobra verbs. +var ClearCommand = func() *command.Command[FulfillInput, FulfillOutput] { + c := *FulfillCommand + c.Name = "quest_clear" + c.MCPOnly = true + c.CLIAliases = nil // avoid cobra registering an orphan alias + c.Short = "fulfill a quest (alias for quest_fulfill; cascades unblock)" + return &c +}() + +func formatFulfilled(s lineListSink, o FulfillOutput) string { res := o.Result var b strings.Builder - b.WriteString(s.Line("πŸ†", "[cleared]", fmt.Sprintf("cleared %s", res.Cleared.ID))) + b.WriteString(s.Line("πŸ†", "[fulfilled]", fmt.Sprintf("fulfilled %s", res.Cleared.ID))) if len(res.Unblocked) > 0 { b.WriteString(" unblocked:\n") for _, q := range res.Unblocked { @@ -124,9 +146,9 @@ func formatCleared(s lineListSink, o ClearOutput) string { return strings.TrimRight(b.String(), "\n") } -func formatClearError(s lineListSink, err error) (string, bool) { +func formatFulfillError(s lineListSink, err error) (string, bool) { if errors.Is(err, ErrNotFound) { - msg := fmt.Sprintf("quest_clear: %v", err) + msg := fmt.Sprintf("quest_fulfill: %v", err) return strings.TrimRight(s.Line("❌", "[err]", msg), "\n"), true } return "", false diff --git a/internal/quest/clear_cmd_test.go b/internal/quest/clear_cmd_test.go index cabfcdc..0288b41 100644 --- a/internal/quest/clear_cmd_test.go +++ b/internal/quest/clear_cmd_test.go @@ -10,20 +10,31 @@ import ( "github.com/mathomhaus/guild/internal/quest" ) -func TestClearCommand_CobraSurface(t *testing.T) { +// TestFulfillCommand_CobraSurface verifies the primary verb + backward-compat +// alias both land on cobra: `quest fulfill` is the primary subcommand, and +// `quest clear` is surfaced via CLIAliases. +func TestFulfillCommand_CobraSurface(t *testing.T) { parent := &cobra.Command{Use: "quest"} parent.PersistentFlags().StringP("project", "p", "", "project") - quest.ClearCommand.BindCobra(parent, fakeDeps(t)) + quest.FulfillCommand.BindCobra(parent, fakeDeps(t)) - sub := findSubcommand(parent, "clear") + sub := findSubcommand(parent, "fulfill") if sub == nil { - t.Fatal("clear subcommand not registered") + t.Fatal("fulfill subcommand not registered") } - if got, want := sub.Use, "clear QUEST_ID"; got != want { + if got, want := sub.Use, "fulfill QUEST_ID"; got != want { t.Errorf("Use=%q want %q", got, want) } - if sub.Short != quest.ClearCommand.Short { - t.Errorf("Short=%q want %q", sub.Short, quest.ClearCommand.Short) + // cobra Aliases should include "clear" for muscle-memory users. + foundAlias := false + for _, a := range sub.Aliases { + if a == "clear" { + foundAlias = true + break + } + } + if !foundAlias { + t.Errorf("expected `clear` among aliases, got %v", sub.Aliases) } for _, want := range []string{"report", "json"} { if sub.Flags().Lookup(want) == nil { @@ -36,12 +47,14 @@ func TestClearCommand_CobraSurface(t *testing.T) { } } -func TestClearCommand_MCPSurface(t *testing.T) { - tool := quest.ClearCommand.BuildMCPForTest(fakeDeps(t)) - if tool.Name != "quest_clear" { - t.Errorf("Name=%q want quest_clear", tool.Name) +// TestFulfillCommand_MCPSurface verifies the canonical MCP tool is named +// `quest_fulfill` and carries the new "Fulfill a quest" description. +func TestFulfillCommand_MCPSurface(t *testing.T) { + tool := quest.FulfillCommand.BuildMCPForTest(fakeDeps(t)) + if tool.Name != "quest_fulfill" { + t.Errorf("Name=%q want quest_fulfill", tool.Name) } - if !strings.HasPrefix(tool.Description, "Complete a quest") { + if !strings.HasPrefix(tool.Description, "Fulfill a quest") { t.Errorf("Description=%q", tool.Description) } buf, _ := json.Marshal(tool.InputSchema) @@ -53,6 +66,20 @@ func TestClearCommand_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. +func TestClearCommand_MCPBackwardCompat(t *testing.T) { + tool := quest.ClearCommand.BuildMCPForTest(fakeDeps(t)) + if tool.Name != "quest_clear" { + t.Errorf("Name=%q want quest_clear (backward-compat alias)", tool.Name) + } + // Description should still reference fulfill semantics. + if !strings.Contains(tool.Description, "Fulfill") { + t.Errorf("alias description should reference fulfill; got %q", tool.Description) + } +} + // findSubcommand returns the direct child of parent named name, or nil. // Shared across conformance tests in this package. func findSubcommand(parent *cobra.Command, name string) *cobra.Command { diff --git a/internal/quest/types.go b/internal/quest/types.go index 1cf393a..5e9398d 100644 --- a/internal/quest/types.go +++ b/internal/quest/types.go @@ -180,14 +180,18 @@ func (p *UpdateParams) Empty() bool { return true } -// ClearResult is returned by Clear. Cleared is the freshly-done quest; -// Unblocked lists every quest whose `blocked β†’ next` flip was caused -// by this Clear call (cascade-unblock invariant). -type ClearResult struct { +// FulfillResult is returned by Fulfill. Cleared is the freshly-done quest; +// Unblocked lists every quest whose `blocked β†’ next` flip was caused by +// this Fulfill call (cascade-unblock invariant). The `Cleared` field name +// is preserved for backward compat in callers; the semantics are the same. +type FulfillResult struct { Cleared *Quest Unblocked []*Quest } +// ClearResult is a backward-compat type alias for FulfillResult. +type ClearResult = FulfillResult + // ForfeitResult is returned by Forfeit. Quest is the target quest's // current state. AlreadyNext is true when Forfeit was a no-op because // the quest was not in_progress β€” no DB writes happened, no release