From 091b7e2c4ba5c33a3e3706df4b76a2efd48d7506 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 20:21:15 -0400 Subject: [PATCH] perf(#305): single-pass max-by-mtime for statusline todo lookup (#385) Replace the per-render readdirSync().filter().map(statSync).sort() chain with a single-pass max-by-mtime loop. Drops the O(n log n) sort and the throwaway intermediate array; I/O and resolved-file behavior are identical. Adds the first behavior-lock test for the todo-resolution path. The larger disk-backed cache win from the issue is deferred: statusline is a fresh child process per render, so any cache must be disk-backed with invalidation/atomic-write design that needs maintainer input. Co-authored-by: Claude Opus 4.7 (1M context) --- .changeset/swift-otter-hum.md | 5 +++ hooks/gsd-statusline.js | 18 +++++--- tests/gsd-statusline.test.cjs | 80 +++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 .changeset/swift-otter-hum.md diff --git a/.changeset/swift-otter-hum.md b/.changeset/swift-otter-hum.md new file mode 100644 index 000000000..0aee0e631 --- /dev/null +++ b/.changeset/swift-otter-hum.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 305 +--- +Use a single-pass max-by-mtime scan for statusline todo lookup, dropping the per-render O(n log n) sort (#305). diff --git a/hooks/gsd-statusline.js b/hooks/gsd-statusline.js index dde1b3b6c..32057a9d1 100755 --- a/hooks/gsd-statusline.js +++ b/hooks/gsd-statusline.js @@ -368,14 +368,20 @@ function runStatusline() { const todosDir = path.join(claudeDir, 'todos'); if (session && fs.existsSync(todosDir)) { try { - const files = fs.readdirSync(todosDir) - .filter(f => f.startsWith(session) && f.includes('-agent-') && f.endsWith('.json')) - .map(f => ({ name: f, mtime: fs.statSync(path.join(todosDir, f)).mtime })) - .sort((a, b) => b.mtime - a.mtime); + // Single-pass max-by-mtime scan: only the newest matching todos file + // is needed, so the O(n log n) sort and the intermediate array from the + // prior `.filter().map(statSync).sort()` chain are unnecessary. Identical + // I/O (one statSync per match) and identical result. (#305) + let latest = null; + for (const entry of fs.readdirSync(todosDir)) { + if (!entry.startsWith(session) || !entry.includes('-agent-') || !entry.endsWith('.json')) continue; + const mtime = fs.statSync(path.join(todosDir, entry)).mtime; + if (!latest || mtime > latest.mtime) latest = { name: entry, mtime }; + } - if (files.length > 0) { + if (latest) { try { - const todos = JSON.parse(fs.readFileSync(path.join(todosDir, files[0].name), 'utf8')); + const todos = JSON.parse(fs.readFileSync(path.join(todosDir, latest.name), 'utf8')); const inProgress = todos.find(t => t.status === 'in_progress'); if (inProgress) task = inProgress.activeForm || ''; } catch (e) {} diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index d996855ea..dae5a2505 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -386,3 +386,83 @@ describe('context meter respects CLAUDE_CODE_AUTO_COMPACT_WINDOW (#2219)', () => 'bridge used_pct must be raw (100-50=50) regardless of CLAUDE_CODE_AUTO_COMPACT_WINDOW'); }); }); + +// ─── todo-resolution path (#305) ──────────────────────────────────────────── + +describe('todo-resolution: resolves in_progress task from the newest matching todos file (#305)', () => { + const { execFileSync } = require('node:child_process'); + const hookPath = path.join(__dirname, '..', 'hooks', 'gsd-statusline.js'); + + test('resolves in_progress task from the newest matching todos file (#305)', (t) => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-305-')); + t.after(() => { + try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch {} + }); + + const todosDir = path.join(tempDir, 'todos'); + fs.mkdirSync(todosDir, { recursive: true }); + + const session = `sess-305-${Math.random().toString(36).slice(2)}`; + const now = Date.now() / 1000; // seconds for utimesSync + + // Older matching file — should NOT be selected + const olderPath = path.join(todosDir, `${session}-agent-A.json`); + fs.writeFileSync(olderPath, JSON.stringify([ + { content: 'old task', status: 'in_progress', activeForm: 'OLDER TASK 305' }, + ])); + const olderTime = now - 10000; + fs.utimesSync(olderPath, olderTime, olderTime); + + // Newer matching file — should be selected + const newerPath = path.join(todosDir, `${session}-agent-B.json`); + fs.writeFileSync(newerPath, JSON.stringify([ + { content: 'new task', status: 'in_progress', activeForm: 'NEWER TASK 305' }, + ])); + const newerTime = now - 1000; + fs.utimesSync(newerPath, newerTime, newerTime); + + // Distractor: different session prefix — must be ignored even with very-new mtime + const wrongSessPath = path.join(todosDir, 'other-sess-agent-Z.json'); + fs.writeFileSync(wrongSessPath, JSON.stringify([ + { content: 'wrong session', status: 'in_progress', activeForm: 'WRONG SESSION 305' }, + ])); + fs.utimesSync(wrongSessPath, now, now); + + // Distractor: matches session + .json but lacks -agent- — must be ignored + const notAgentPath = path.join(todosDir, `${session}-notagent.json`); + fs.writeFileSync(notAgentPath, JSON.stringify([ + { content: 'not agent', status: 'in_progress', activeForm: 'NOT AGENT 305' }, + ])); + fs.utimesSync(notAgentPath, now, now); + + const payload = JSON.stringify({ + model: { display_name: 'Claude' }, + workspace: { current_dir: os.tmpdir() }, + session_id: session, + context_window: { remaining_percentage: 80, total_tokens: 1_000_000 }, + }); + + const env = { ...process.env, CLAUDE_CONFIG_DIR: tempDir }; + + let stdout = ''; + try { + stdout = execFileSync(process.execPath, [hookPath], { + input: payload, + env, + encoding: 'utf8', + timeout: 4000, + }); + } catch (e) { + stdout = e.stdout || ''; + } + + assert.ok(stdout.includes('NEWER TASK 305'), + `expected stdout to contain "NEWER TASK 305", got: ${stdout}`); + assert.ok(!stdout.includes('OLDER TASK 305'), + `stdout must NOT contain "OLDER TASK 305", got: ${stdout}`); + assert.ok(!stdout.includes('WRONG SESSION 305'), + `stdout must NOT contain "WRONG SESSION 305", got: ${stdout}`); + assert.ok(!stdout.includes('NOT AGENT 305'), + `stdout must NOT contain "NOT AGENT 305", got: ${stdout}`); + }); +});