Repository navigation
perf: 27% faster CLI, 21-29% faster hooks, 14% faster pull - #1226
Conversation
simple-git settles a git command on close, or 50 ms after exit when close never comes. The 50 ms timer stayed armed after close won, so every command that ends on its own waited up to 50 ms after its last git call. The build now unrefs that timer, and fails if simple-git changes the code it patches.
Importing ora up front ran string-width's setup (an Intl.Segmenter and a Unicode-property regex) in every command, hooks included. spinner() now imports ora on first use, so its callers await it.
Every command, each hook included, started git twice (rev-parse and worktree list) to resolve project anchors that only change with the repository stamp. Keep those answers under the same stamps in teamai-<uid>/git-answers.json in the temporary directory, beside Node's compile cache, so a dry run still leaves the TeamAI home untouched. The directory is trusted only when it is this user's alone. The repository stamp now also covers git's config files, the commondir link and the search-limit variables, so a core.worktree change is seen within a run and across runs.
The store moves to src/utils/git-answers.ts, which owns both layers (this run's memory, then the file) so each call site asks once. Writes from one run go one at a time, each taking every answer not yet written: answers asked for at once were lost before. A worktree list is kept per repository (its common directory), not per directory. A directory that no longer exists gets no stamp, so it is never answered from an earlier run, and a worktrees/ directory that cannot be read for a reason other than being absent keeps nothing. Windows, where the directory owner cannot be checked this way, keeps nothing, and config.worktree is part of the repository stamp. Tests now get their own temporary directory, so the suite neither shares answers nor fills the file the real CLI reads. The dry-run contract records that temporary-directory caches are not something a dry run changes.
The patch threw only when a matched file lacked the patched text. A simple-git release that moves its entry, or a require() reaching the CJS build, matched nothing and shipped the 50 ms wait silently. Both builds now match, and the build fails when nothing was patched.
V8 keeps a source holding any non-ASCII character as two bytes per character, so a few em dashes made every run decode the 5.5 MB bundle into 11 MB of UTF-16, then hash, compile and collect it. esbuild's ASCII charset escapes strings, templates and identifiers, but not regular expressions or the comments it keeps. A tsup plugin escapes those with acorn's tokenizer, and fails the build on any non-ASCII character left elsewhere or following a backslash.
Skill, rule and agent sync asked each tool whether it is installed for every item, and wrote "Skipping ... sync for <tool>: tool not installed" each time: one synchronous append per skill for every tool not installed, 481 lines in a pull of 36 skills and 1,599 at four times the content. debugOnce, beside warnOnce, writes such a line once per run; a pull now writes 123 lines whatever the number of skills.
…eds it Workflow sync ran rev-parse HEAD, for-each-ref --contains and rev-parse --show-toplevel on every pull before listing workflows/. The revision is now resolved on the first valid workflow file, so a team without workflows starts no git there.
…at origin Each refresh ran status, rev-list --count and merge --ff-only one after another. The status and one rev-list --left-right --count (behind and ahead) now run together, and a worktree already at origin's commit stops there.
The PR description documents sufficient testing: a built real-CLI run, output/exit-code comparisons, and unit/e2e results at the head commit. No testing-record finding. Nothing was run, built, or installed. |
Git takes core.worktree from the repository's own config files and never from a file they include (include.path, includeIf), so the repository stamp leaves included files out. An agreement case edits only the included file after the first answer and expects git's answer, so it fails first should git ever read core.worktree there.
… per version - linkedWorktreesStamp also stats config.worktree, where a main checkout can be made bare; a later run then lists no main checkout, as git does. - The kept-answers file name carries the CLI version. - syncWorktree finishes restoring conflicted files before a failed count throws. - The skip debug lines name the scope, so debugOnce keeps one per scope.
72e837c to
7e35228
Compare
|
Re the [P1 blocking] on included config and cached checkout locations: not reproducible. Git takes Probe on git 2.56.0: Same result with The real gap nearby is |
Earlier finding: Resolved. The current diff adds an agreement test demonstrating that Git ignores included Testing record: Sufficient. The description documents built real-CLI runs and output/exit-code comparisons. Older verification commits are a note, not a blocker; no additional provider × agent matrix is required. Review was read-only. Nothing from the PR was run, built, or installed. |
Git follows symlinks in core.worktree, so retargeting one moved the work tree while every stamped file stayed the same, and a later run kept the old root. The stamp now carries the real path of each worktree value in the stamped config files; a value it cannot resolve leaves no stamp. Submodules, whose config always names core.worktree, stay kept between runs.
|
Re the [P3 nit] on retargeting a Tests compare teamai with |
Earlier findings: Both resolved: the included- Testing record: Sufficient. The description documents built real-CLI runs and output/exit-code comparisons. Older verification commits and coverage deferred to CI are not blockers. Review was read-only. Nothing from the PR was run, built, or installed. |
…orktree list git worktree list reads the main entry's core.bare from the common config, each checkout's own config.worktree and the GIT_CONFIG* variables. The stamp now covers all three, and gives no stamp where that config includes another file, which it does not follow.
|
Re the [P3 nit] on included Tests compare teamai's later-run list with |
|
No new findings in the specified PR diff.
Review was read-only. Nothing from the PR was run, built, or installed. |
Summary
Production code only: what the CLI does on every start, and on
pull. Dashboard and test speed are out of scope.The kept git answers live in
src/utils/git-answers.ts, one module that owns both layers (this run's memory, then the file). The file is trusted only when its directory is this user's alone (uid, mode 0700, not a symlink). Where that can't be checked (Windows), nothing is kept. The repository stamp now also coversconfig,config.worktree, the common config,commondir, the real path eachcore.worktreethere resolves to (git follows its symlinks) and the variables that stop git's search. The worktree-list stamp covers the commonconfig, each checkout's ownconfig.worktreeand theGIT_CONFIG*variables, everything git reads the main entry'score.barefrom. The file name carries the CLI version, so another release never reads these answers.Evidence
Before:
origin/main8dc60b0 · After: 72e837c (the timed code; later commits change review-found edge cases, not the hot path). Interleaved hyperfine A/B, 3 rounds × 10 runs per side per command (order alternated per round, 3 warmups each). Node 24.21, Apple silicon. Fixture: agit-provider team with 36 skills, 6 rules, 3 agents, 20 learnings and Claude hooks injected. Medians:pullmost). The means give the same -27.1%. Each command's output and exit code are identical between before and after.debug.loglines perpullwent from 1,599 to 123. Git processes perpullstay the same at 1x and 4x content (16).dist/index.jsagainst the fixture.hook post-tool-usetook ~80 ms on head against ~97 ms on base, so a first run after install or temp cleanup is not slower.npx tsc --noEmitandnpm run lintpass.npx vitest run: 471 files, 11,223 passed, 21 skipped.npm run test:e2efull on 72e837c (test isolation changed): 157 files, 1,312 passed, 27 skipped; CI runs it on the head.pull,status,list,hook post-tool-use,recall,push --dry-runexit 0 with output equal to base apart from timings and random ids;git-answers-<version>.jsonis written in a 0700 directory; a linked worktree's kept list agrees withgit worktree list --porcelainafter the main checkout is made bare in itsconfig.worktree. On 1f81044 (the last production change), the built CLI'shook post-tool-usespawns git 6 times cold and 4 warm on the fixture, as before the change. ThesyncWorktreereorder (restore before a failed count can throw) is verified by reading only.Measured and dropped (within noise or slower):
splitting(pull+17%)--no-lazycompiledebug.logopenNot kept, asked of git each run instead: a repository whose own config has an
[include]or[includeIf](git may readcore.barefrom the included file; it ignores an includedcore.worktree). Aworktreeconfig line the stamp cannot read (quoted, escaped, commented) leaves no stamp, so that repository asks git every run.Not covered: gitlab/github providers and Codex/CodeBuddy/OpenCode agents beyond what the e2e suite runs. Left to CI.
Merge Danger
Door: two-way
Nothing persisted moves. The only new file is a disposable cache in the temporary directory. Deleting it or reverting this PR costs one extra
git rev-parseandgit worktree listper command.Blast Radius: startup
Every command and hook goes through the kept git answers. A stale answer would point teamai at the wrong checkout root or exclude file. The stamps cover what git reads to find the repository, and the agreement tests compare teamai with git for each change case. The two build plugins fail closed: a simple-git release that changes the patched line or moves its entry, or a non-ASCII character outside a comment or regex, fails
npm run buildrather than shipping slower.