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
20 changes: 18 additions & 2 deletions lib/public/modules/diagnostic-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
5 changes: 3 additions & 2 deletions lib/public/modules/diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
68 changes: 52 additions & 16 deletions lib/settings-preflight.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
// <projectDir>/.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";

Expand All @@ -21,27 +28,43 @@ 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",
"PostToolUse",
"Notification",
"Stop",
"SubagentStop",
// lr-7e22: verified 2026-07-01 against code.claude.com/docs/en/hooks
"PreCompact",
"PostCompact",
"SessionStart",
"UserPromptSubmit",
"PermissionRequest",
"TaskCreated",
"TeammateIdle",
"TaskCompleted",
];

// ---------------------------------------------------------------------------
Expand All @@ -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";

Expand All @@ -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 {
Expand All @@ -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.",
});
}
Expand All @@ -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<object>} diagnostics
*/
function validateSettingsFile(filePath) {
function validateSettingsFile(filePath, scope) {
var diagnostics = [];

// Missing file → nothing to validate. Not an error.
Expand All @@ -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;
Expand All @@ -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]);
}
Expand Down Expand Up @@ -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]);
}
Expand All @@ -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]);
}
Expand Down
36 changes: 36 additions & 0 deletions test/diagnostic-format-lr-8294.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
Expand Down
67 changes: 67 additions & 0 deletions test/diagnostics-preflight-scope-label-lr-7e22.test.js
Original file line number Diff line number Diff line change
@@ -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
// <projectDir>/.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)"
);
});
Loading
Loading