diff --git a/bin/cli.js b/bin/cli.js index 21e8d63c..2186092c 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -33,7 +33,7 @@ var { forkDaemon, devMode, setDaemonWatcherOpts } = require("../lib/cli/daemon-l var { setup, promptRestoreProjects, showMainMenu } = require("../lib/cli/menus"); var { getLocalIP } = require("../lib/cli/net-detect"); var { log, a, sym } = require("../lib/cli/tui"); -var { handleShutdown, handleRestart, handleAdd, handleRemove, handleList } = require("../lib/cli/ipc-subcommands"); +var { handleShutdown, handleRestart, handleAdd, handleRemove, handleList, handleActivityDiagnostics } = require("../lib/cli/ipc-subcommands"); var args = process.argv.slice(2); @@ -69,6 +69,7 @@ var noRestart = false; var addPath = null; var removePath = null; var listMode = false; +var activityDiagnosticsMode = false; var dangerouslySkipPermissions = false; var headlessMode = false; var watchMode = false; @@ -118,6 +119,8 @@ for (var i = 0; i < args.length; i++) { i++; } else if (args[i] === "--list") { listMode = true; + } else if (args[i] === "--activity-diagnostics") { + activityDiagnosticsMode = true; } else if (args[i] === "--headless") { headlessMode = true; autoYes = true; @@ -132,6 +135,7 @@ for (var i = 0; i < args.length; i++) { console.log(" clagentic-console --add Add a project to the running daemon"); console.log(" clagentic-console --remove Remove a project from the running daemon"); console.log(" clagentic-console --list List registered projects"); + console.log(" clagentic-console --activity-diagnostics Print activity-divergence probe totals as JSON (agent-readable)"); console.log(" clagentic-console release list-betas List promotable beta versions (maintainer/release-engineering)"); console.log(""); console.log("Options:"); @@ -149,6 +153,7 @@ for (var i = 0; i < args.length; i++) { console.log(" --add Add a project directory (use '.' for current)"); console.log(" --remove Remove a project directory"); console.log(" --list List all registered projects"); + console.log(" --activity-diagnostics Print activity-divergence probe totals as JSON"); console.log(" --headless Start daemon and exit immediately (implies --yes)"); console.log(" --multi-user Start in multi-user mode (use with --yes for headless)"); console.log(" --os-users Enable OS-level user isolation (Linux, requires root + --multi-user)"); @@ -194,6 +199,12 @@ if (listMode) { return; } +// --- Handle --activity-diagnostics before anything else --- +if (activityDiagnosticsMode) { + handleActivityDiagnostics(); + return; +} + // --multi-user / --os-users are now handled in the main entry flow (setup wizard or repeat run) // Flags are parsed above and applied during forkDaemon() diff --git a/docs/guides/architecture.md b/docs/guides/architecture.md index 592dc4be..33ab3809 100644 --- a/docs/guides/architecture.md +++ b/docs/guides/architecture.md @@ -67,7 +67,50 @@ graph TB P2 --- Sessions2 ``` -The daemon is spawned with `detached: true` and survives CLI exit. Multiple CLI instances share one daemon. IPC commands include `add_project`, `remove_project`, `set_pin`, `set_keep_awake`, `shutdown`, `get_status`. +The daemon is spawned with `detached: true` and survives CLI exit. Multiple CLI instances share one daemon. IPC commands include `add_project`, `remove_project`, `set_pin`, `set_keep_awake`, `shutdown`, `get_status`, `get_activity_diagnostics`. + +### Agent-readable diagnostics over IPC + +Some server-side diagnostic probes (e.g. the activity-source divergence +counter that sizes the `session.isProcessing` vs. registry-derived-active +redesign) are recorded server-side but were historically only reachable +through a live WebSocket client (`process_stats`) — unreachable by a +read-only crew agent holding only `Bash`+`Read`, no browser, no WS client, +no devtools. + +The daemon.sock IPC channel above is already reachable by such an agent (it +is a plain Unix socket, no browser or WS client required), and is gated by +filesystem permissions rather than any per-command auth check: `CONFIG_DIR` +(containing `daemon.sock`) is created `chmod 0700` (`ensureConfigDir`, `lib/config.js`), +so only the daemon's own OS user can connect at all. A read-only diagnostics +command added to this channel inherits that gate for free — it does not +copy the auth gap `process_stats`'s WS handler has (`lib/project-sessions.js:668-709`, +tracked separately), since it is a different transport with its own, +stricter gate. + +**The exact command a Bash-only agent runs** to read the activity-divergence +probe from a running daemon: + +``` +clagentic-console --activity-diagnostics +``` + +This prints JSON to stdout: + +```json +{ + "activeLiveCount": 0, + "activityDivergenceCount": 0, + "activityDivergenceRecentSamples": [] +} +``` + +Samples carry no session-identifying field (`ts`, `rawIsProcessing`, +`derivedIsActive`, `hasQueryInstance` only) — the same shape +`process_stats` already carries, unchanged by adding this second retrieval +path. See `lib/sdk-bridge.js`'s `buildActivityDiagnosticsResponse()` (the +shared, unit-tested function both retrieval paths can call) and +`lib/daemon.js`'s `get_activity_diagnostics` IPC case. ## YOKE Adapter Layer diff --git a/lib/cli/ipc-subcommands.js b/lib/cli/ipc-subcommands.js index 5de5357f..076e88c2 100644 --- a/lib/cli/ipc-subcommands.js +++ b/lib/cli/ipc-subcommands.js @@ -1,10 +1,15 @@ // lib/cli/ipc-subcommands.js // // One-shot IPC subcommand handlers for bin/cli.js: --shutdown, --restart, -// --add , --remove , --list. Each of these talks to the running -// daemon over the Unix socket and exits the process directly (they never -// return control to the caller) — extracted verbatim from bin/cli.js -// (lr-4e49 Part 1), no behavior change. +// --add , --remove , --list, --activity-diagnostics. Each of +// these talks to the running daemon over the Unix socket and exits the +// process directly (they never return control to the caller) — +// --shutdown/--restart/--add/--remove/--list extracted verbatim from +// bin/cli.js (lr-4e49 Part 1), no behavior change. +// --activity-diagnostics added by lr-8b476f: the only agent-readable +// (Bash+Read, no browser/WS/devtools) retrieval path for the lr-58c813 +// server-side activity-divergence probe. Prints raw JSON to stdout so a +// read-only crew agent can run this directly. var fs = require("fs"); var path = require("path"); @@ -177,10 +182,41 @@ function handleList() { }); } +// lr-8b476f: prints the activity-divergence probe totals (same data +// process_stats's WS response folds in, see lib/daemon.js's +// "get_activity_diagnostics" IPC case) as raw JSON to stdout. This is the +// retrieval path for a read-only crew agent (Bash+Read only, no browser, +// no WS client, no devtools): `clagentic-console --activity-diagnostics`. +function handleActivityDiagnostics() { + var diagConfig = loadConfig(); + isDaemonAliveAsync(diagConfig).then(function (alive) { + if (!alive) { + console.error("No running daemon. Start with: npx @clagentic/console"); + process.exit(1); + } + sendIPCCommand(socketPath(), { cmd: "get_activity_diagnostics" }).then(function (res) { + if (!res.ok) { + console.error("Failed: " + (res.error || "unknown error")); + process.exit(1); + return; + } + // Raw JSON to stdout — the point is machine readability, not a + // human-formatted summary (contrast handleList's formatted output). + console.log(JSON.stringify({ + activeLiveCount: res.activeLiveCount, + activityDivergenceCount: res.activityDivergenceCount, + activityDivergenceRecentSamples: res.activityDivergenceRecentSamples, + }, null, 2)); + process.exit(0); + }); + }); +} + module.exports = { handleShutdown: handleShutdown, handleRestart: handleRestart, handleAdd: handleAdd, handleRemove: handleRemove, handleList: handleList, + handleActivityDiagnostics: handleActivityDiagnostics, }; diff --git a/lib/daemon.js b/lib/daemon.js index 5683e103..07ef6abd 100644 --- a/lib/daemon.js +++ b/lib/daemon.js @@ -31,7 +31,7 @@ var usersModule = require("./users"); var { createWorktree, removeWorktree, isWorktree } = require("./worktree"); var { isWorktreeSlug, scanAndRegisterWorktrees, rescanWorktrees, cleanupWorktreesForParent, getFilteredRemovedProjects, registerWorktreeSlug, unregisterWorktreeSlug } = require("./daemon-projects"); var { validateCloneUrl, buildCloneArgs } = require("./clone-validate"); -var { DEFAULT_MEM_AVAILABLE_MIN_MB, DEFAULT_TOKENS_PER_MB_HEADROOM, getActiveLiveCount } = require("./sdk-bridge"); +var { DEFAULT_MEM_AVAILABLE_MIN_MB, DEFAULT_TOKENS_PER_MB_HEADROOM, getActiveLiveCount, buildActivityDiagnosticsResponse } = require("./sdk-bridge"); var { startMemoryHighWatcher, checkAppliedMemoryCeiling } = require("./memory-limits"); var { createDrain } = require("./drain"); var { shedMemory } = require("./memory-shed"); @@ -1553,6 +1553,30 @@ var ipc = createIPCServer(socketPath(), function (msg) { uptime: process.uptime(), }; + // lr-8b476f: agent-readable counterpart to the WS-only process_stats + // handler (lib/project-sessions.js:668-709). Reads the same shared + // module-level counter (lib/sdk-bridge.js) that process_stats already + // folds into its response via getMemoryStats() — buildActivityDiagnosticsResponse + // is unit-tested directly (test/activity-diagnostics-retrieval-lr-8b476f.test.js), + // since daemon.js has no module.exports and cannot be required + // in-process without binding real sockets/HTTP servers. + // + // Read-only, mutates nothing. Gated the same way every other IPC + // command on this socket is gated: CONFIG_DIR (containing daemon.sock) + // is chmod 0700 (ensureConfigDir, lib/config.js), so only the daemon's + // own OS user can connect at all — no new auth surface is introduced, + // and none is copied from process_stats's WS gap (lr-2016fe) either, + // since this is a different transport with its own (stricter, OS-level) + // gate rather than an unaudited role check. + // + // Samples carry no session-identifying field, matching process_stats's + // BOBBIE-remediated shape exactly (lib/sdk-bridge.js + // _recordActivityDivergenceIfAny) — this command reads the SAME + // samples, so nothing new is exposed by adding this second retrieval + // path. + case "get_activity_diagnostics": + return buildActivityDiagnosticsResponse(); + case "set_pin": { config.pinHash = msg.pinHash || null; relay.setAuthToken(config.pinHash); diff --git a/lib/sdk-bridge.js b/lib/sdk-bridge.js index e6fef2af..86a0daee 100644 --- a/lib/sdk-bridge.js +++ b/lib/sdk-bridge.js @@ -135,6 +135,24 @@ function getActivityDivergenceStats() { }; } +// lr-8b476f: shared response builder for the agent-readable retrieval path +// (daemon.js's "get_activity_diagnostics" IPC command, over the existing +// daemon.sock Unix socket). Pulled out here — rather than inlined in +// daemon.js, which has no module.exports and cannot be safely required by a +// test without binding real sockets/HTTP servers — so this is unit-testable +// the same way getActivityDivergenceStats()/getActiveLiveCount() already +// are. Read-only: calls only the two existing module-level accessors above, +// writes nothing. +function buildActivityDiagnosticsResponse() { + var divergence = getActivityDivergenceStats(); + return { + ok: true, + activeLiveCount: getActiveLiveCount(), + activityDivergenceCount: divergence.count, + activityDivergenceRecentSamples: divergence.recentSamples, + }; +} + // --- lr-2d91: MemAvailable gate --- // Default minimum available memory threshold in MB. Referenced by sdk-bridge // and daemon.js — changing this constant is the single place to adjust the default. @@ -2490,5 +2508,5 @@ function getActiveLiveCount() { return _activeLiveCount; } -module.exports = { createSDKBridge, createMessageQueue, readMemAvailableMB, readCgroupHeadroomMB, DEFAULT_MEM_AVAILABLE_MIN_MB, DEFAULT_TOKENS_PER_MB_HEADROOM, getActiveLiveCount, getActivityDivergenceStats }; +module.exports = { createSDKBridge, createMessageQueue, readMemAvailableMB, readCgroupHeadroomMB, DEFAULT_MEM_AVAILABLE_MIN_MB, DEFAULT_TOKENS_PER_MB_HEADROOM, getActiveLiveCount, getActivityDivergenceStats, buildActivityDiagnosticsResponse }; diff --git a/test/activity-diagnostics-retrieval-lr-8b476f.test.js b/test/activity-diagnostics-retrieval-lr-8b476f.test.js new file mode 100644 index 00000000..af907dfe --- /dev/null +++ b/test/activity-diagnostics-retrieval-lr-8b476f.test.js @@ -0,0 +1,226 @@ +/** + * Regression/behavioral tests for lr-8b476f: an agent-readable retrieval + * path for the lr-58c813 server-side activity-divergence probe. + * + * THE PROBLEM this closes: the probe (lib/sdk-bridge.js) was reachable ONLY + * through the process_stats WebSocket message (lib/project-sessions.js:668-709), + * which requires a live WS client — unreachable by a read-only crew agent + * holding Bash+Read only. This adds a second retrieval path over the + * EXISTING daemon.sock Unix IPC socket (lib/daemon.js's "get_activity_diagnostics" + * command), which a Bash-only agent can reach via: + * + * clagentic-console --activity-diagnostics + * + * SCOPE: this file proves the RETRIEVAL PATH actually returns the probe + * data — not that the probe itself is correct (that is lr-58c813's own test + * file, test/activity-divergence-probe-lr-58c813.test.js, left untouched). + * + * Per repo convention (docs/guides/TESTING_CONVENTIONS.md): daemon.js has no + * module.exports and cannot be safely required in-process (it binds real + * sockets/HTTP servers as a side effect of being loaded), so the actual + * response-building logic lives in lib/sdk-bridge.js's + * buildActivityDiagnosticsResponse() — a plain, directly testable function — + * and daemon.js's IPC "get_activity_diagnostics" case is a one-line call + * into it. Test 1 below proves that function genuinely surfaces data + * recorded by a live probe tick (not a hardcoded/stubbed shape); test 2 + * source-checks daemon.js's case body is wired to call the SAME function + * (not a fabricated inline duplicate that could drift from it). + */ + +var test = require("node:test"); +var assert = require("node:assert/strict"); +var fs = require("fs"); +var path = require("path"); + +var sessionActivity = require("../lib/session-activity"); + +function makeSessionManager() { + return { + sessions: new Map(), + currentModel: null, + currentPermissionMode: null, + currentEffort: null, + currentBetas: [], + modelsByVendor: {}, + availableVendors: [], + installedVendors: [], + defaultVendor: "claude", + saveSessionFile: function () {}, + broadcastSessionList: function () {}, + getActiveSession: function () { return null; }, + setSlashCommandsForVendor: function () {}, + sendAndRecord: function (session, obj) { + if (!session.history) session.history = []; + session.history.push(obj); + }, + sendToSession: function () {}, + }; +} + +var _localIdSeq = 1; +function makeSession(overrides) { + return Object.assign({ + localId: "diagsess-" + (_localIdSeq++), + queryInstance: null, + messageQueue: null, + abortController: null, + isProcessing: false, + cliSessionId: null, + history: [], + blocks: {}, + sentToolResults: {}, + pendingPermissions: {}, + pendingAskUser: {}, + pendingElicitations: {}, + activeTaskToolIds: {}, + singleTurn: false, + destroying: false, + lastActivityAt: Date.now(), + }, overrides || {}); +} + +function freshSdkBridge() { + var modPath = require.resolve("../lib/sdk-bridge"); + delete require.cache[modPath]; + return require("../lib/sdk-bridge"); +} + +function makeBridge(sdkBridgeMod) { + var sm = makeSessionManager(); + var bridge = sdkBridgeMod.createSDKBridge({ + cwd: "/tmp", + slug: "test", + sessionManager: sm, + send: function () {}, + adapter: { vendor: "claude" }, + adapters: {}, + onProcessingChanged: function () {}, + }); + return { sm: sm, bridge: bridge }; +} + +// --------------------------------------------------------------------------- +// 1. buildActivityDiagnosticsResponse() genuinely surfaces a divergence +// recorded by a live idle-reaper tick — this is the test that would FAIL +// against pre-fix code, since buildActivityDiagnosticsResponse did not +// exist before this change (confirmed by stash-testing: reverting +// lib/sdk-bridge.js to HEAD~ makes this throw TypeError, not merely +// return a wrong value — see PR body). +// --------------------------------------------------------------------------- + +test("lr-8b476f: buildActivityDiagnosticsResponse() surfaces a divergence recorded by a real idle-reaper tick, not a stubbed/hardcoded shape", function (t) { + t.mock.timers.enable({ apis: ["setInterval"] }); + try { + var sdkBridgeMod = freshSdkBridge(); + var setup = makeBridge(sdkBridgeMod); + var sm = setup.sm; + var bridge = setup.bridge; + + assert.equal(typeof sdkBridgeMod.buildActivityDiagnosticsResponse, "function", + "lib/sdk-bridge.js must export buildActivityDiagnosticsResponse"); + + var before = sdkBridgeMod.buildActivityDiagnosticsResponse(); + assert.equal(before.ok, true); + assert.equal(typeof before.activityDivergenceCount, "number"); + assert.ok(Array.isArray(before.activityDivergenceRecentSamples)); + + // Construct exactly one genuine divergence: isProcessing=true, no matching + // activity token — mirrors the concrete raise path lr-5edd64 names. + var session = makeSession({ isProcessing: true, queryInstance: {} }); + sm.sessions.set(session.localId, session); + + bridge.startIdleReaper(); + t.mock.timers.tick(60 * 1000 * 1); // one reaper tick + + var after = sdkBridgeMod.buildActivityDiagnosticsResponse(); + + assert.equal(after.activityDivergenceCount, before.activityDivergenceCount + 1, + "the retrieval path must reflect the exact divergence just recorded by the probe, not a cached/stale/stubbed count"); + assert.equal(after.activityDivergenceRecentSamples[0].rawIsProcessing, true); + assert.equal(after.activityDivergenceRecentSamples[0].derivedIsActive, false); + assert.equal(typeof after.activeLiveCount, "number"); + + bridge.stopIdleReaper(); + } finally { + t.mock.timers.reset(); + } +}); + +// --------------------------------------------------------------------------- +// 2. No sessionId leaks through the new retrieval path either — this is the +// SAME BOBBIE-remediated shape process_stats already carries; a second +// retrieval path must not quietly reintroduce it. +// --------------------------------------------------------------------------- + +test("lr-8b476f: a sample returned by buildActivityDiagnosticsResponse() never includes a sessionId field", function (t) { + t.mock.timers.enable({ apis: ["setInterval"] }); + try { + var sdkBridgeMod = freshSdkBridge(); + var setup = makeBridge(sdkBridgeMod); + var sm = setup.sm; + var bridge = setup.bridge; + + var session = makeSession({ isProcessing: true, queryInstance: {} }); + sm.sessions.set(session.localId, session); + + bridge.startIdleReaper(); + t.mock.timers.tick(60 * 1000 * 1); + + var stats = sdkBridgeMod.buildActivityDiagnosticsResponse(); + assert.ok(stats.activityDivergenceRecentSamples.length >= 1, "expected at least one recorded sample"); + assert.equal(Object.prototype.hasOwnProperty.call(stats.activityDivergenceRecentSamples[0], "sessionId"), false, + "the IPC retrieval path must not carry a session identifier either — same posture as process_stats"); + + bridge.stopIdleReaper(); + } finally { + t.mock.timers.reset(); + } +}); + +// --------------------------------------------------------------------------- +// 3. daemon.js's "get_activity_diagnostics" IPC case is wired to call the +// SAME function tested above, rather than a hand-duplicated inline copy +// that could drift from it or silently reintroduce a field BOBBIE +// required removed. Source-level check only (daemon.js cannot be +// required in-process — see file header comment); paired with the +// behavioral tests above, not a substitute for them (per lr-58c813's own +// "source inspection alone cannot prove correctness" lesson). +// --------------------------------------------------------------------------- + +test("lib/daemon.js: get_activity_diagnostics IPC case calls buildActivityDiagnosticsResponse() from lib/sdk-bridge.js", function () { + var daemonSrc = fs.readFileSync(path.join(__dirname, "..", "lib", "daemon.js"), "utf8"); + assert.match(daemonSrc, /require\(["']\.\/sdk-bridge["']\)/, "daemon.js must require lib/sdk-bridge.js"); + assert.match(daemonSrc, /buildActivityDiagnosticsResponse/, "daemon.js must reference buildActivityDiagnosticsResponse"); + + var caseStart = daemonSrc.indexOf('case "get_activity_diagnostics"'); + assert.ok(caseStart !== -1, 'expected a "get_activity_diagnostics" IPC case in lib/daemon.js'); + var caseEnd = daemonSrc.indexOf("case ", caseStart + 1); + var caseBody = daemonSrc.slice(caseStart, caseEnd === -1 ? caseStart + 400 : caseEnd); + + assert.match(caseBody, /buildActivityDiagnosticsResponse\(\)/, + "the get_activity_diagnostics case must call the shared, unit-tested buildActivityDiagnosticsResponse() rather than duplicating its logic inline"); +}); + +// --------------------------------------------------------------------------- +// 4. The CLI subcommand (the actual command a Bash-only agent runs) sends +// the right IPC cmd and exits non-interactively. +// --------------------------------------------------------------------------- + +test("lib/cli/ipc-subcommands.js: handleActivityDiagnostics sends {cmd: \"get_activity_diagnostics\"} over the daemon socket", function () { + var src = fs.readFileSync(path.join(__dirname, "..", "lib", "cli", "ipc-subcommands.js"), "utf8"); + assert.match(src, /function handleActivityDiagnostics\(/, "expected handleActivityDiagnostics to be defined"); + var start = src.indexOf("function handleActivityDiagnostics"); + var end = src.indexOf("\nfunction ", start + 1); + var body = src.slice(start, end === -1 ? src.length : end); + assert.match(body, /cmd:\s*["']get_activity_diagnostics["']/, + "handleActivityDiagnostics must send the get_activity_diagnostics IPC command"); + assert.match(body, /console\.log\(JSON\.stringify/, + "handleActivityDiagnostics must print JSON to stdout (the point is machine readability for a Bash-only agent)"); + assert.match(src, /handleActivityDiagnostics:\s*handleActivityDiagnostics/, "handleActivityDiagnostics must be exported"); +}); + +test("bin/cli.js: --activity-diagnostics flag is wired to handleActivityDiagnostics", function () { + var src = fs.readFileSync(path.join(__dirname, "..", "bin", "cli.js"), "utf8"); + assert.match(src, /--activity-diagnostics/, "expected a --activity-diagnostics flag in bin/cli.js"); + assert.match(src, /handleActivityDiagnostics\(\)/, "the flag must call handleActivityDiagnostics()"); +});