From 5eac3bf7ef086f5be0916760e8092a7e6855566b Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 3 Jun 2026 13:41:35 +0200 Subject: [PATCH] perf: hoist invariant git worktree-list out of the agents-status loop, lazy-require cockpit (plan phase 4) Phase 4 of the gitguardex improvement plan: perf hot-paths, no observable behavior change. - B3 agents/inspect.js + agents/status.js: buildAgentsStatusPayload called changedFiles() -> inspectAgentBranch() -> worktreePathForBranch() once PER SESSION, each running `git worktree list --porcelain`. The list is invariant within one (synchronous) payload build, so fetch it once via new listWorktrees() and thread it down through optional options.worktrees (~6N -> ~5N+1 git spawns). Backward-compatible: callers that omit worktrees fetch on demand. - B4 cli/main.js + cli/commands/misc.js: lazy-require('../cockpit') inside the two functions that use it instead of eager top-level. Cockpit pulls 32 modules; now loaded only when it renders. Proven: requiring main.js pulls 0 cockpit modules (was 32). Deferred: B5 finish base-branch hoist (collides with unlanded detectDefaultBaseBranch WIP); cockpit/index.js spread narrowing (export cleanup, no perf benefit). Verification: node --test failing set byte-identical to base (zero new failures). Independent review: no blocking findings. --- .../.openspec.yaml | 2 + .../notes.md | 38 +++++++++++++++++++ src/agents/inspect.js | 21 ++++++++-- src/agents/status.js | 15 +++++--- src/cli/commands/misc.js | 3 +- src/cli/main.js | 3 +- 6 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/.openspec.yaml create mode 100644 openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/notes.md diff --git a/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/.openspec.yaml b/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/.openspec.yaml new file mode 100644 index 00000000..0ba725fb --- /dev/null +++ b/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-03 diff --git a/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/notes.md b/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/notes.md new file mode 100644 index 00000000..02b2d172 --- /dev/null +++ b/openspec/changes/agent-claude-phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26/notes.md @@ -0,0 +1,38 @@ +# phase4-perf-hoist-agents-status-git-prob (minimal / T1) + +Branch: `agent/claude/phase4-perf-hoist-agents-status-git-prob-2026-06-03-13-26` + +Phase 4 of the gitguardex improvement plan: perf hot-paths. No observable behavior change. 4 files, +30/-12. + +## Scope + +- **B3** `agents/inspect.js` + `agents/status.js`: hoist the invariant `git worktree list --porcelain` + out of the per-session loop in `buildAgentsStatusPayload`. Was fetched once per session inside + `worktreePathForBranch` (~6N git spawns); now fetched ONCE via new `listWorktrees(repoRoot)` and + threaded down through `changedFiles`/`inspectAgentBranch` via optional `options.worktrees` + (~6N -> ~5N+1). Backward-compatible: callers that omit it (branchDiff/branchLocks/inspect cmd) + fetch on demand, unchanged. Also hot via `cockpit/state.js` (re-renders each refresh). +- **B4** `cli/main.js` + `cli/commands/misc.js`: lazy-`require('../cockpit')` inside the two + functions that use it (default-cockpit dispatch + `gx cockpit`), instead of eager top-level. + Cockpit pulls 32 modules; now loaded ONLY when the cockpit actually renders. + +## Deferred (not here) + +- **B5** finish base-branch hoist (`finish/index.js`) — optimizes `resolveFinishBaseBranch`, the exact + function the user's staged `detectDefaultBaseBranch` WIP rewrites. Pairs with Phase 5 once WIP lands. +- **cockpit/index.js spread narrowing** — export-surface cleanup, re-export risk, NO perf benefit + (cockpit/index.js still requires control/actions when loaded). Defer to a dead-surface pass. + +## Verification + +- **B4 proven**: requiring `main.js` pulls **0** cockpit modules (was 32). +- **B3 structural**: `listWorktrees` fetched once at `status.js:90`, threaded into `worktreePathForBranch`. + (Not benchmarked for N: 0 active sessions locally; e2e test `agents-status.test.js:143` exercises the + hoisted real-array path end-to-end and passes.) +- `node --test`: failing set **byte-identical to base** (detached-snapshot diff, `comm` = 0 new / 0 fixed). 34 = baseline. +- Independent review: NO BLOCKING FINDINGS (null-handling, behavior equivalence, B4 completeness verified). + +## Cleanup + +- [ ] Gate satisfied manually (clean review + no-new-failures) per baseline-red-CI policy → land via `gx branch finish --wait-for-merge --cleanup`. +- [ ] Record PR URL + `MERGED`. Confirm sandbox pruned. diff --git a/src/agents/inspect.js b/src/agents/inspect.js index 0fa8c990..659280b1 100644 --- a/src/agents/inspect.js +++ b/src/agents/inspect.js @@ -45,10 +45,22 @@ function parseWorktreeList(outputText) { return worktrees; } -function worktreePathForBranch(repoRoot, branch) { +function listWorktrees(repoRoot) { const result = git(repoRoot, ['worktree', 'list', '--porcelain'], { allowFailure: true }); - if (result.status !== 0) return { worktreePath: repoRoot, worktreeFound: false }; - const match = parseWorktreeList(result.stdout).find((entry) => entry.branch === branch); + return result.status === 0 ? parseWorktreeList(result.stdout) : null; +} + +// `worktrees` (pre-parsed via listWorktrees) lets callers iterating many branches +// in one pass hoist the invariant `git worktree list` out of their loop. When +// omitted, the list is fetched on demand — unchanged behavior. +function worktreePathForBranch(repoRoot, branch, worktrees = null) { + let entries = worktrees; + if (entries == null) { + const result = git(repoRoot, ['worktree', 'list', '--porcelain'], { allowFailure: true }); + if (result.status !== 0) return { worktreePath: repoRoot, worktreeFound: false }; + entries = parseWorktreeList(result.stdout); + } + const match = entries.find((entry) => entry.branch === branch); return { worktreePath: match?.path || repoRoot, worktreeFound: Boolean(match?.path), @@ -119,7 +131,7 @@ function inspectAgentBranch(options) { const baseBranch = resolveBaseBranch(repoRoot, branch); const compareRef = compareRefForBase(repoRoot, baseBranch); - const { worktreePath, worktreeFound } = worktreePathForBranch(repoRoot, branch); + const { worktreePath, worktreeFound } = worktreePathForBranch(repoRoot, branch, options.worktrees); return { repoRoot, branch, baseBranch, compareRef, worktreePath, worktreeFound }; } @@ -179,6 +191,7 @@ module.exports = { branchLocks, changedFiles, inspectAgentBranch, + listWorktrees, parseWorktreeList, readLockRegistry, renderDiff, diff --git a/src/agents/status.js b/src/agents/status.js index a5a8461c..170df3bf 100644 --- a/src/agents/status.js +++ b/src/agents/status.js @@ -1,5 +1,5 @@ const { fs, path, LOCK_FILE_RELATIVE, TOOL_NAME } = require('../context'); -const { changedFiles } = require('./inspect'); +const { changedFiles, listWorktrees } = require('./inspect'); const { listAgentSessions } = require('./sessions'); function uniqueSorted(values) { @@ -35,10 +35,10 @@ function readLockDetails(repoRoot) { return { counts, files }; } -function readChangedFiles(repoRoot, branch) { +function readChangedFiles(repoRoot, branch, worktrees) { if (!branch) return []; try { - return changedFiles({ target: repoRoot, branch }).files || []; + return changedFiles({ target: repoRoot, branch, worktrees }).files || []; } catch (_error) { return []; } @@ -53,7 +53,7 @@ function normalizePr(session) { }; } -function normalizeSessionForStatus(session, lockDetails, repoRoot) { +function normalizeSessionForStatus(session, lockDetails, repoRoot, worktrees) { const branch = session.branch || ''; const worktreePath = session.worktreePath || ''; const claimedFiles = uniqueSorted([ @@ -73,7 +73,7 @@ function normalizeSessionForStatus(session, lockDetails, repoRoot) { worktreeExists: worktreePath ? fs.existsSync(worktreePath) : false, lockCount: lockDetails.counts.get(branch) || 0, claimedFiles, - changedFiles: readChangedFiles(repoRoot, branch), + changedFiles: readChangedFiles(repoRoot, branch, worktrees), metadata: session.metadata && typeof session.metadata === 'object' ? session.metadata : {}, launchCommand: session.launchCommand || '', tmux: session.tmux && typeof session.tmux === 'object' ? session.tmux : null, @@ -85,10 +85,13 @@ function normalizeSessionForStatus(session, lockDetails, repoRoot) { function buildAgentsStatusPayload(repoRoot) { const lockDetails = readLockDetails(repoRoot); + // Hoist the invariant `git worktree list` out of the per-session loop: fetch it + // once here instead of once inside every session's changedFiles() (~6N -> ~6+N). + const worktrees = listWorktrees(repoRoot); return { schemaVersion: 1, repoRoot, - sessions: listAgentSessions(repoRoot).map((session) => normalizeSessionForStatus(session, lockDetails, repoRoot)), + sessions: listAgentSessions(repoRoot).map((session) => normalizeSessionForStatus(session, lockDetails, repoRoot, worktrees)), }; } diff --git a/src/cli/commands/misc.js b/src/cli/commands/misc.js index eaf72006..c96206db 100644 --- a/src/cli/commands/misc.js +++ b/src/cli/commands/misc.js @@ -28,7 +28,6 @@ const { } = require('../../core/runtime'); const hooksModule = require('../../hooks'); const submoduleModule = require('../../submodule'); -const cockpitModule = require('../../cockpit'); const { removeLegacyPackageScripts, installUserLevelAsset, @@ -200,6 +199,8 @@ function submodule(rawArgs) { } function cockpit(rawArgs) { + // Lazy-require: cockpit pulls ~32 modules; load only for `gx cockpit`. + const cockpitModule = require('../../cockpit'); cockpitModule.openCockpit(rawArgs, { resolveRepoRoot, toolName: TOOL_NAME, diff --git a/src/cli/main.js b/src/cli/main.js index 37cacafe..85d1114b 100755 --- a/src/cli/main.js +++ b/src/cli/main.js @@ -14,7 +14,6 @@ const toolchainModule = require('../toolchain'); const budgetModule = require('../budget'); const ciInitModule = require('../ci-init'); const speckitModule = require('../speckit'); -const cockpitModule = require('../cockpit'); const { usage, startTransientSpinner } = require('../output'); const { maybeSuggestCommand, @@ -151,6 +150,8 @@ async function main() { if (args.length === 0) { if (isInteractiveTerminal() && !legacyDefaultStatusEnabled() && !defaultCockpitDisabled()) { + // Lazy-require: cockpit pulls ~32 modules; load only when actually rendering it. + const cockpitModule = require('../cockpit'); cockpitModule.openDefaultCockpit({ resolveRepoRoot, toolName: TOOL_NAME,