diff --git a/lib/public/modules/diagnostic-format.js b/lib/public/modules/diagnostic-format.js index 3bcea767..89687494 100644 --- a/lib/public/modules/diagnostic-format.js +++ b/lib/public/modules/diagnostic-format.js @@ -13,14 +13,30 @@ var SOURCE_LABELS = { preflight: "Preflight", }; +// Terse scope suffixes shown alongside the source label (lr-7e22). Settings +// preflight runs against both ~/.claude/settings.json and the project's +// .claude/settings.json independently, emitting one diagnostic per unknown +// key per file — without a scope suffix, two distinct warnings render an +// identical "Preflight" header and visually stack as apparent duplicates. +var SCOPE_LABELS = { + user: "user", + project: "project", +}; + /** * Format a raw diagnostic source identifier for display. * @param {string} source - The source field from the diagnostic message. + * @param {string} [scope] - Optional file/config scope ('user' | 'project'). + * When present and recognized, appended to the + * source label so diagnostics from different + * scopes are distinguishable at a glance. * @returns {string} Human-readable source label. */ -export function formatDiagnosticSource(source) { +export function formatDiagnosticSource(source, scope) { if (!source) return "Diagnostic"; - return SOURCE_LABELS[source] || source; + var label = SOURCE_LABELS[source] || source; + var scopeLabel = scope && SCOPE_LABELS[scope]; + return scopeLabel ? label + " · " + scopeLabel : label; } /** diff --git a/lib/public/modules/diagnostics.js b/lib/public/modules/diagnostics.js index 93ad6aeb..58d74618 100644 --- a/lib/public/modules/diagnostics.js +++ b/lib/public/modules/diagnostics.js @@ -89,6 +89,7 @@ export function addDiagnostic(msg) { var entry = { severity: msg.severity || "info", source: msg.source || "", + scope: msg.scope || "", message: msg.message || "", actionable: msg.actionable || null, ts: Date.now(), @@ -132,7 +133,7 @@ function _showDiagnosticToast(entry) { var sourceEl = document.createElement("span"); sourceEl.className = "toast-diagnostic-source"; - sourceEl.textContent = _formatSource(entry.source); + sourceEl.textContent = _formatSource(entry.source, entry.scope); var dismissBtn = document.createElement("button"); dismissBtn.className = "toast-diagnostic-dismiss"; @@ -233,7 +234,7 @@ function _appendEntry(entry) { var srcEl = document.createElement("span"); srcEl.className = "diagnostics-source"; - srcEl.textContent = _formatSource(entry.source); + srcEl.textContent = _formatSource(entry.source, entry.scope); meta.appendChild(sevEl); meta.appendChild(srcEl); diff --git a/lib/settings-preflight.js b/lib/settings-preflight.js index fb67136b..fee540bd 100644 --- a/lib/settings-preflight.js +++ b/lib/settings-preflight.js @@ -5,12 +5,19 @@ // without relying on CLI stderr pattern matching. // // Public surface: -// validateSettingsObject(obj, filePath) — pure: settings obj → diagnostic[] -// runPreflight(opts) — IO: reads files, validates, returns diagnostic[] -// VALID_HOOK_EVENTS — exported for tests +// validateSettingsObject(obj, filePath, scope) — pure: settings obj → diagnostic[] +// runPreflight(opts) — IO: reads files, validates, returns diagnostic[] +// VALID_HOOK_EVENTS — exported for tests // // Diagnostic shape (conforms to validateDiagnosticEvent from YOKE interface): -// { type: 'diagnostic', severity, source: 'preflight', message } +// { type: 'diagnostic', severity, source: 'preflight', scope?: 'user'|'project', message } +// +// scope (lr-7e22) distinguishes which settings file a diagnostic came from — +// runPreflight checks both ~/.claude/settings.json (scope:'user') and +// /.claude/settings.json (scope:'project') independently, so a +// single unknown hook key can legitimately produce one diagnostic per file. +// The render layer (diagnostic-format.js) uses scope to keep those visually +// distinct instead of both showing an identical "Preflight" source label. "use strict"; @@ -21,20 +28,27 @@ var os = require("os"); // --------------------------------------------------------------------------- // Hook event allowlist // -// SOURCE: This list was derived from the Claude Code CLI documentation and SDK -// source as of 2026-06-30 (claude@1.x). The hooks.AgentTeams case (the +// SOURCE: Verified live against the Claude Code hooks documentation +// (https://code.claude.com/docs/en/hooks, hook lifecycle table) as of +// 2026-07-01, cross-checked via web search. The hooks.AgentTeams case (the // motivating screenshot for this stage) is a real example of an unknown key // that surfaces as a silent no-op in the CLI but clutters the warnings stream. +// The 8 events added under lr-7e22 (PreCompact, PostCompact, SessionStart, +// UserPromptSubmit, PermissionRequest, TaskCreated, TeammateIdle, +// TaskCompleted) were confirmed present in that same table — they are not an +// exhaustive list of all documented events (the docs list ~30 total as of +// this sync), only the ones observed in the field that needed verification. // // IMPORTANT: This list is the ONLY place the valid hook events are defined in // the Console codebase. Do not duplicate it elsewhere — drift between copies // is exactly the parity bug we are fixing. // // TODO(lr-1a26): Sync this list whenever the Claude Code CLI releases new hook -// event types. The authoritative upstream is the CLI's hook documentation or -// source (search for "hook" + "event" in the @anthropic-ai/claude-code package). -// Consider scripting a CI check against the installed CLI version's known events -// if the CLI exposes a machine-readable list in a future release. +// event types. The authoritative upstream is the CLI's hook documentation +// (https://code.claude.com/docs/en/hooks) or source (search for "hook" + +// "event" in the @anthropic-ai/claude-code package). Consider scripting a CI +// check against the installed CLI version's known events if the CLI exposes +// a machine-readable list in a future release. // --------------------------------------------------------------------------- var VALID_HOOK_EVENTS = [ "PreToolUse", @@ -42,6 +56,15 @@ var VALID_HOOK_EVENTS = [ "Notification", "Stop", "SubagentStop", + // lr-7e22: verified 2026-07-01 against code.claude.com/docs/en/hooks + "PreCompact", + "PostCompact", + "SessionStart", + "UserPromptSubmit", + "PermissionRequest", + "TaskCreated", + "TeammateIdle", + "TaskCompleted", ]; // --------------------------------------------------------------------------- @@ -55,9 +78,13 @@ var VALID_HOOK_EVENTS = [ * * @param {object} obj - Parsed settings JSON (any shape; we check what we know) * @param {string} filePath - Path string used only in diagnostic messages - * @returns {Array<{type,severity,source,message}>} Diagnostics (empty = no issues) + * @param {string} [scope] - 'user' | 'project', attached to each diagnostic so the + * render layer can distinguish which settings file a + * warning came from (lr-7e22). Omitted when the caller + * has no file-scope context (e.g. direct unit tests). + * @returns {Array<{type,severity,source,scope,message}>} Diagnostics (empty = no issues) */ -function validateSettingsObject(obj, filePath) { +function validateSettingsObject(obj, filePath, scope) { var diagnostics = []; var label = filePath || "settings.json"; @@ -75,6 +102,7 @@ function validateSettingsObject(obj, filePath) { type: "diagnostic", severity: "warning", source: "preflight", + scope: scope || undefined, message: label + ": 'hooks' field is not an object — expected { EventName: [...] }.", }); } else { @@ -86,6 +114,7 @@ function validateSettingsObject(obj, filePath) { type: "diagnostic", severity: "warning", source: "preflight", + scope: scope || undefined, message: label + ": unknown hook event '" + key + "'. Valid events: " + VALID_HOOK_EVENTS.join(", ") + ". This hook will be silently ignored by the CLI.", }); } @@ -112,9 +141,14 @@ function validateSettingsObject(obj, filePath) { * per the non-blocking preflight contract. * * @param {string} filePath - Absolute path to settings.json + * @param {string} [scope] - 'user' | 'project', attached to each diagnostic (lr-7e22) + * so multiple legitimate warnings across files render as + * visibly distinct entries instead of stacking as apparent + * duplicates. Optional — omitted when the caller has no + * file-scope context. * @returns {Array} diagnostics */ -function validateSettingsFile(filePath) { +function validateSettingsFile(filePath, scope) { var diagnostics = []; // Missing file → nothing to validate. Not an error. @@ -128,6 +162,7 @@ function validateSettingsFile(filePath) { type: "diagnostic", severity: "info", source: "preflight", + scope: scope || undefined, message: filePath + ": could not read settings file (" + e.code + " — " + (e.message || String(e)) + ").", }); return diagnostics; @@ -142,13 +177,14 @@ function validateSettingsFile(filePath) { type: "diagnostic", severity: "error", source: "preflight", + scope: scope || undefined, message: filePath + ": settings.json is not valid JSON — " + (e.message || String(e)) + ".", }); return diagnostics; } // (b) Structural validation - var structural = validateSettingsObject(obj, filePath); + var structural = validateSettingsObject(obj, filePath, scope); for (var i = 0; i < structural.length; i++) { diagnostics.push(structural[i]); } @@ -186,7 +222,7 @@ function runPreflight(opts) { // User global: ~/.claude/settings.json var userSettingsPath = path.join(userHome, ".claude", "settings.json"); - var userDiags = validateSettingsFile(userSettingsPath); + var userDiags = validateSettingsFile(userSettingsPath, "user"); for (var i = 0; i < userDiags.length; i++) { diagnostics.push(userDiags[i]); } @@ -195,7 +231,7 @@ function runPreflight(opts) { var projectDir = opts.projectDir; if (projectDir && typeof projectDir === "string" && projectDir.length > 0) { var projectSettingsPath = path.join(projectDir, ".claude", "settings.json"); - var projectDiags = validateSettingsFile(projectSettingsPath); + var projectDiags = validateSettingsFile(projectSettingsPath, "project"); for (var j = 0; j < projectDiags.length; j++) { diagnostics.push(projectDiags[j]); } diff --git a/test/diagnostic-format-lr-8294.test.js b/test/diagnostic-format-lr-8294.test.js index 6f349242..a408bda0 100644 --- a/test/diagnostic-format-lr-8294.test.js +++ b/test/diagnostic-format-lr-8294.test.js @@ -45,6 +45,42 @@ test("formatDiagnosticSource: 'preflight' maps to 'Preflight' (stage 5/5 lr-1a26 assert.strictEqual(formatDiagnosticSource("preflight"), "Preflight"); }); +// ============================================================ +// formatDiagnosticSource + scope (lr-7e22) +// ============================================================ +// +// runPreflight validates BOTH ~/.claude/settings.json and the project's +// .claude/settings.json independently, emitting one diagnostic per unknown +// hook key per file. Without a scope suffix, two legitimate, distinct +// warnings render an identical "Preflight" header and visually stack as +// apparent duplicates (lr-5db5 investigation). scope distinguishes them. + +test("formatDiagnosticSource: 'preflight' + scope:'user' appends a terse user suffix", () => { + assert.strictEqual(formatDiagnosticSource("preflight", "user"), "Preflight · user"); +}); + +test("formatDiagnosticSource: 'preflight' + scope:'project' appends a terse project suffix", () => { + assert.strictEqual(formatDiagnosticSource("preflight", "project"), "Preflight · project"); +}); + +test("formatDiagnosticSource: 'preflight' + scope:'user' and scope:'project' render different labels", () => { + var userLabel = formatDiagnosticSource("preflight", "user"); + var projectLabel = formatDiagnosticSource("preflight", "project"); + assert.notStrictEqual(userLabel, projectLabel, "labels must be visually distinguishable"); +}); + +test("formatDiagnosticSource: scope omitted falls back to the bare source label (no regression)", () => { + assert.strictEqual(formatDiagnosticSource("preflight"), "Preflight"); +}); + +test("formatDiagnosticSource: unrecognized scope value is ignored (no suffix appended)", () => { + assert.strictEqual(formatDiagnosticSource("preflight", "bogus-scope"), "Preflight"); +}); + +test("formatDiagnosticSource: scope suffix also applies to non-preflight sources", () => { + assert.strictEqual(formatDiagnosticSource("hook", "project"), "Hook · project"); +}); + test("formatDiagnosticSource: unknown source passes through unchanged", () => { assert.strictEqual(formatDiagnosticSource("mcp-server"), "mcp-server"); }); diff --git a/test/diagnostics-preflight-scope-label-lr-7e22.test.js b/test/diagnostics-preflight-scope-label-lr-7e22.test.js new file mode 100644 index 00000000..0c5acf3a --- /dev/null +++ b/test/diagnostics-preflight-scope-label-lr-7e22.test.js @@ -0,0 +1,67 @@ +// diagnostics-preflight-scope-label-lr-7e22.test.js — lr-7e22 regression coverage. +// +// Root cause (lr-5db5 investigation, split into lr-7e22): runPreflight() +// (lib/settings-preflight.js) validates BOTH ~/.claude/settings.json and +// /.claude/settings.json independently, emitting one warning +// diagnostic per unknown hook key per file. These are correct, distinct +// diagnostics, but formatDiagnosticSource('preflight') rendered the identical +// label "Preflight" for every one, so two-or-more legitimate warnings visually +// stacked as apparent duplicates in the fixed-position toast slot. +// +// Fix: settings-preflight.js now attaches scope:'user'|'project' to each +// diagnostic (based on which settings file produced it). diagnostics.js reads +// entry.scope and passes it through to formatDiagnosticSource(source, scope) +// at both render sites (toast header + panel row), producing distinguishable +// labels like "Preflight · user" vs "Preflight · project". +// +// These are source-text regression checks (no DOM harness required), matching +// the project's existing diagnostics-panel-pointer-events-lr-b580.test.js +// convention for modules that only do DOM work inside functions. + +"use strict"; + +var test = require("node:test"); +var assert = require("node:assert/strict"); +var fs = require("fs"); +var path = require("path"); + +var DIAGNOSTICS_JS = fs.readFileSync( + path.join(__dirname, "../lib/public/modules/diagnostics.js"), + "utf8" +); + +test("diagnostics.js: addDiagnostic reads msg.scope onto the entry", () => { + var idx = DIAGNOSTICS_JS.indexOf("export function addDiagnostic"); + assert.ok(idx !== -1, "expected addDiagnostic to be exported"); + var block = DIAGNOSTICS_JS.slice(idx, idx + 400); + + assert.match( + block, + /scope:\s*msg\.scope\s*\|\|\s*["']["']/, + "addDiagnostic must carry msg.scope onto the internal entry so the render sites can distinguish user vs project preflight diagnostics" + ); +}); + +test("diagnostics.js: toast header passes entry.scope into _formatSource", () => { + var idx = DIAGNOSTICS_JS.indexOf("function _showDiagnosticToast"); + assert.ok(idx !== -1, "expected _showDiagnosticToast to exist"); + var block = DIAGNOSTICS_JS.slice(idx, idx + 1200); + + assert.match( + block, + /_formatSource\(entry\.source,\s*entry\.scope\)/, + "toast header must pass entry.scope to _formatSource so multiple legitimate preflight warnings across settings files render distinguishable source labels (lr-7e22)" + ); +}); + +test("diagnostics.js: panel row passes entry.scope into _formatSource", () => { + var idx = DIAGNOSTICS_JS.indexOf("function _appendEntry"); + assert.ok(idx !== -1, "expected _appendEntry to exist"); + var block = DIAGNOSTICS_JS.slice(idx, idx + 1200); + + assert.match( + block, + /_formatSource\(entry\.source,\s*entry\.scope\)/, + "panel row must pass entry.scope to _formatSource, matching the toast header so both surfaces agree on the distinguishable label (lr-7e22)" + ); +}); diff --git a/test/settings-preflight-lr-1a26.test.js b/test/settings-preflight-lr-1a26.test.js index 4d5954e0..c698a2e8 100644 --- a/test/settings-preflight-lr-1a26.test.js +++ b/test/settings-preflight-lr-1a26.test.js @@ -31,6 +31,7 @@ var path = require("path"); var preflight = require("../lib/settings-preflight"); var validateSettingsObject = preflight.validateSettingsObject; var validateSettingsFile = preflight.validateSettingsFile; +var runPreflight = preflight.runPreflight; var VALID_HOOK_EVENTS = preflight.VALID_HOOK_EVENTS; // ============================================================ @@ -54,6 +55,37 @@ test("VALID_HOOK_EVENTS contains known hook names", function() { assert.ok(VALID_HOOK_EVENTS.indexOf("Stop") !== -1, "Stop must be valid"); }); +test("VALID_HOOK_EVENTS: lr-7e22 sync adds events verified against code.claude.com/docs/en/hooks", function() { + // Fixes false-positive "unknown hook event" warnings for operators using + // these real, current Claude Code hook events that the older allowlist + // predates. See settings-preflight.js header comment for verification source. + var addedEvents = [ + "PreCompact", + "PostCompact", + "SessionStart", + "UserPromptSubmit", + "PermissionRequest", + "TaskCreated", + "TeammateIdle", + "TaskCompleted", + ]; + for (var i = 0; i < addedEvents.length; i++) { + assert.ok(VALID_HOOK_EVENTS.indexOf(addedEvents[i]) !== -1, addedEvents[i] + " must be valid (lr-7e22)"); + } +}); + +test("validateSettingsObject: PreCompact hook key produces no diagnostic (lr-7e22 regression)", function() { + // Regression test for the operator's false-positive: PreCompact is a real + // Claude Code hook event and must not be flagged as unknown. + var settings = { + hooks: { + PreCompact: [{ hooks: [{ type: "command", command: "lore hook precompact-handler" }] }], + }, + }; + var diags = validateSettingsObject(settings, "/home/user/.claude/settings.json"); + assert.strictEqual(diags.length, 0, "PreCompact must not produce a diagnostic"); +}); + // ============================================================ // validateSettingsObject — pure function tests // ============================================================ @@ -110,8 +142,28 @@ test("validateSettingsObject: multiple unknown hook keys produce one warning eac var messages = diags.map(function(d) { return d.message; }); assert.ok(messages.some(function(m) { return m.indexOf("AgentTeams") !== -1; }), "AgentTeams mentioned"); assert.ok(messages.some(function(m) { return m.indexOf("CustomHookXyz") !== -1; }), "CustomHookXyz mentioned"); - // No diagnostic for PreToolUse - assert.ok(!messages.some(function(m) { return m.indexOf("PreToolUse") !== -1; }), "PreToolUse must not appear"); + // No diagnostic entry names PreToolUse as the offending unknown key. Note: + // each diagnostic message legitimately echoes the full "Valid events: ..." + // list (by design, for operator guidance), so PreToolUse — a valid event — + // DOES appear as a substring of both messages. The correct assertion is + // that neither message's "unknown hook event ''" quoted key is + // PreToolUse, not that the substring never appears anywhere (lr-7e22 fix — + // this assertion was checking the wrong thing prior to this change). + assert.ok(!messages.some(function(m) { return m.indexOf("unknown hook event 'PreToolUse'") !== -1; }), "PreToolUse must not be flagged as an unknown key"); +}); + +test("validateSettingsObject: scope param is attached to each diagnostic (lr-7e22)", function() { + var settings = { hooks: { AgentTeams: [] } }; + var diagsUser = validateSettingsObject(settings, "/home/user/.claude/settings.json", "user"); + var diagsProject = validateSettingsObject(settings, "/repo/.claude/settings.json", "project"); + assert.strictEqual(diagsUser[0].scope, "user", "scope must be 'user' when passed"); + assert.strictEqual(diagsProject[0].scope, "project", "scope must be 'project' when passed"); +}); + +test("validateSettingsObject: scope is omitted (undefined) when caller does not pass it", function() { + var settings = { hooks: { AgentTeams: [] } }; + var diags = validateSettingsObject(settings, "settings.json"); + assert.strictEqual(diags[0].scope, undefined, "scope must be undefined when not provided"); }); test("validateSettingsObject: hooks field is wrong type → warning diagnostic", function() { @@ -211,3 +263,43 @@ test("validateSettingsFile: valid JSON with unknown hook key → warning naming assert.strictEqual(diags[0].severity, "warning", "must be warning"); assert.ok(diags[0].message.indexOf("AgentTeams") !== -1, "message must name AgentTeams"); }); + +test("validateSettingsFile: scope param is attached to the resulting diagnostics (lr-7e22)", function() { + var settings = { hooks: { AgentTeams: [] } }; + var filePath = writeTmp("settings.json", JSON.stringify(settings)); + var diags = validateSettingsFile(filePath, "project"); + assert.strictEqual(diags.length, 1); + assert.strictEqual(diags[0].scope, "project", "scope must propagate from validateSettingsFile to the diagnostic"); +}); + +// ============================================================ +// runPreflight — scope tagging across user vs project files (lr-7e22) +// ============================================================ + +test("runPreflight: user settings diagnostics carry scope:'user', project diagnostics carry scope:'project'", function() { + var userDir = fs.mkdtempSync(path.join(os.tmpdir(), "clagentic-preflight-user-")); + fs.mkdirSync(path.join(userDir, ".claude"), { recursive: true }); + fs.writeFileSync( + path.join(userDir, ".claude", "settings.json"), + JSON.stringify({ hooks: { AgentTeams: [] } }), + "utf8" + ); + + var projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "clagentic-preflight-project-")); + fs.mkdirSync(path.join(projectDir, ".claude"), { recursive: true }); + fs.writeFileSync( + path.join(projectDir, ".claude", "settings.json"), + JSON.stringify({ hooks: { CustomHookXyz: [] } }), + "utf8" + ); + + var diags = runPreflight({ userHome: userDir, projectDir: projectDir }); + assert.strictEqual(diags.length, 2, "one diagnostic per file"); + + var userDiag = diags.filter(function(d) { return d.message.indexOf("AgentTeams") !== -1; })[0]; + var projectDiag = diags.filter(function(d) { return d.message.indexOf("CustomHookXyz") !== -1; })[0]; + assert.ok(userDiag, "AgentTeams diagnostic must be present"); + assert.ok(projectDiag, "CustomHookXyz diagnostic must be present"); + assert.strictEqual(userDiag.scope, "user", "diagnostic from ~/.claude/settings.json must carry scope:'user'"); + assert.strictEqual(projectDiag.scope, "project", "diagnostic from /.claude/settings.json must carry scope:'project'"); +});