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 0000000..0ba725f --- /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 0000000..02b2d17 --- /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 0fa8c99..659280b 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 a5a8461..170df3b 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 eaf7200..c96206d 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 37cacaf..85d1114 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,