fix(hints): fire no-brief-24h on quest_fulfill via canonical mapping (QUEST-137)

internal/hints/engine.go

@@ -8,6 +8,8 @@ import (
 	"strconv"
 	"sync"
 	"time"
+
+	"github.com/mathomhaus/guild/internal/quest"
 )
 
 // FollowThroughWindow is how many subsequent events the engine tracks
@@ -206,7 +208,11 @@ func (e *Engine) Evaluate(ctx context.Context, ev CallEvent) Fire {
 		if !cr.row.Enabled {
 			continue
 		}
-		if cr.row.TriggerTool != "*" && cr.row.TriggerTool != ev.Tool {
+		// Normalize both sides to canonical tool names before comparing so
+		// backward-compat aliases (e.g. quest_clear → quest_fulfill) trigger
+		// rules authored against the canonical name. See quest.CanonicalToolName.
+		if cr.row.TriggerTool != "*" &&
+			quest.CanonicalToolName(cr.row.TriggerTool) != quest.CanonicalToolName(ev.Tool) {
 			continue
 		}
 		if cr.rule.Trigger == nil || !cr.rule.Trigger(e.context, ev) {

internal/hints/rules.go

@@ -79,10 +79,10 @@ func Definitions() []Rule {
 		},
 		{
 			ID:            "no-brief-24h",
-			TriggerTool:   "quest_clear",
+			TriggerTool:   "quest_fulfill",
 			Trigger:       triggerNoBrief24h,
 			FollowThrough: followQuestBrief,
-			Caveat:        "Era-aware severity: 💡 hint on MCP, ℹ️ fyi on Bash CLI (18.7pp gap in ENTRY-29 calibration).",
+			Caveat:        "Era-aware severity: 💡 hint on MCP, ℹ️ fyi on Bash CLI (18.7pp gap in ENTRY-29 calibration). TriggerTool is canonical quest_fulfill; quest_clear alias is handled by quest.CanonicalToolName in the engine.",
 		},
 
 		// ℹ️ fyi (demote) — 3 rules.
@@ -95,10 +95,10 @@ func Definitions() []Rule {
 		},
 		{
 			ID:            "clear-without-report-detail",
-			TriggerTool:   "quest_clear",
+			TriggerTool:   "quest_fulfill",
 			Trigger:       triggerClearWithoutReportDetail,
 			FollowThrough: followQuestUpdateOrJournal,
-			Caveat:        "Demoted to fyi; short reports are legitimate for trivial clears.",
+			Caveat:        "Demoted to fyi; short reports are legitimate for trivial clears. TriggerTool is canonical quest_fulfill; quest_clear alias is handled by quest.CanonicalToolName in the engine.",
 		},
 		{
 			ID:            "principle-too-long",

internal/mcp/hints_e2e_test.go

@@ -176,6 +176,72 @@ func TestHints_E2E_SlugQuery_OnlyOnZeroResult(t *testing.T) {
 	}
 }
 
+// TestHints_E2E_NoBrief24h_BothVerbsFire is the QUEST-137 regression test:
+// the no-brief-24h rule must fire on BOTH quest_fulfill (canonical) AND
+// quest_clear (backward-compat alias). Each sub-test spins up an isolated
+// server so the singleton engine's session state is fresh per run.
+func TestHints_E2E_NoBrief24h_BothVerbsFire(t *testing.T) {
+	longReport := "commit abc123, files: x.go y.go; follow-ups: run integration tests and verify deploy pipeline stays green"
+
+	for _, toolName := range []string{"quest_fulfill", "quest_clear"} {
+		toolName := toolName // capture range variable for sub-test closure
+		t.Run(toolName, func(t *testing.T) {
+			pid := isolateProject(t)
+			ctx := context.Background()
+
+			db, err := openQuestDB(ctx)
+			if err != nil {
+				t.Fatalf("open quest db: %v", err)
+			}
+			defer func() { _ = db.Close() }()
+
+			q, err := quest.Post(ctx, db, pid, quest.PostParams{Subject: "brief-regression-" + toolName})
+			if err != nil {
+				t.Fatalf("post: %v", err)
+			}
+
+			s, err := build()
+			if err != nil {
+				t.Fatalf("build: %v", err)
+			}
+			_, client, cleanup := connectInMemory(t, s)
+			defer cleanup()
+
+			if _, err := client.CallTool(ctx, &sdkmcp.CallToolParams{
+				Name:      "guild_session_start",
+				Arguments: map[string]any{"project": pid},
+			}); err != nil {
+				t.Fatalf("bootstrap: %v", err)
+			}
+
+			// Use the tool under test — either quest_fulfill or quest_clear.
+			// A long report keeps clear-without-report-detail from winning the
+			// budget slot over no-brief-24h.
+			res, err := client.CallTool(ctx, &sdkmcp.CallToolParams{
+				Name: toolName,
+				Arguments: map[string]any{
+					"quest_id": q.ID,
+					"report":   longReport,
+					"project":  pid,
+				},
+			})
+			if err != nil {
+				t.Fatalf("%s: %v", toolName, err)
+			}
+			if res.IsError {
+				t.Fatalf("%s IsError=true: %s", toolName, textOf(res.Content))
+			}
+			body := textOf(res.Content)
+			if !strings.Contains(body, "no quest_brief yet") {
+				t.Errorf("%s: expected no-brief-24h hint in body; got:\n%s", toolName, body)
+			}
+			if !strings.Contains(body, "💡") {
+				t.Errorf("%s: expected 💡 emoji in body; got:\n%s", toolName, body)
+			}
+		})
+	}
+}
+
 // TestHints_E2E_ClearWithoutBrief fires no-brief-24h through a full MCP
 // round-trip — the canonical migration target of BriefHint.
 func TestHints_E2E_ClearWithoutBrief(t *testing.T) {

internal/quest/aliases.go

@@ -0,0 +1,26 @@
+package quest
+
+// toolAliases maps backward-compat MCP tool names to their canonical
+// successors. Used by the hint engine to normalize trigger_tool matches
+// so rules authored against the canonical name fire on alias calls too.
+//
+// Add entries here whenever a tool is renamed and the old name is kept
+// as a backward-compat alias. The map is intentionally small — only
+// renames that affect live hint rules need an entry.
+var toolAliases = map[string]string{
+	// QUEST-106 / LORE-80: quest_clear renamed to quest_fulfill.
+	// quest_clear stays as a backward-compat MCP alias; hint rules use
+	// quest_fulfill as the canonical trigger_tool.
+	"quest_clear": "quest_fulfill",
+}
+
+// CanonicalToolName returns the canonical tool name for name. If name is
+// a known backward-compat alias, the canonical successor is returned.
+// Otherwise name is returned unchanged. Safe for concurrent use (reads a
+// package-level map that is never mutated after init).
+func CanonicalToolName(name string) string {
+	if canonical, ok := toolAliases[name]; ok {
+		return canonical
+	}
+	return name
+}

internal/storage/migrations/001_init.up.sql

@@ -179,7 +179,7 @@ INSERT OR IGNORE INTO hints (rule_id, trigger_tool, severity, template, cooldown
   ('session-end-without-brief',   '*',             'hint', 'session is long (30+ guild calls) with no quest_brief — consider quest_brief("what was done, what''s next") before compact',             10, NULL),
   ('slug-query',                  'lore_appraise', 'hint', 'query looks slug-like — did you mean quest_list or quest_scroll?',                                                                        5, NULL),
   ('journal-outside-accepted',    'quest_journal', 'hint', 'journaling on a quest not accepted this session — accept it first (quest_accept) or file a fresh quest_post',                             5, NULL),
-  ('no-brief-24h',                'quest_clear',   'hint', 'no quest_brief yet this session — consider quest_brief("what was done, what''s next") before compact',                                    5, '{"mcp":"hint","bash":"fyi"}'),
+  ('no-brief-24h',                'quest_fulfill', 'hint', 'no quest_brief yet this session — consider quest_brief("what was done, what''s next") before compact',                                    5, '{"mcp":"hint","bash":"fyi"}'),
   ('inscribe-without-appraise',   'lore_inscribe', 'fyi',  'no lore_appraise in the last 5 calls — consider appraise(query="…", all_projects=true) before inscribing to avoid duplicates',            5, NULL),
-  ('clear-without-report-detail', 'quest_clear',   'fyi',  'report is short (<20 words) — a richer report (commit hash, files, follow-ups) makes cold-start handoff cleaner',                         5, NULL),
+  ('clear-without-report-detail', 'quest_fulfill', 'fyi',  'report is short (<20 words) — a richer report (commit hash, files, follow-ups) makes cold-start handoff cleaner',                         5, NULL),
   ('principle-too-long',          'lore_inscribe', 'fyi',  'principle exceeds the ≤60-word oath-hygiene target — consider kind=decision for long rationale and re-inscribe a shorter principle',      5, NULL);