perf(hooks): cut SessionStart blocking git spawns + duplicate briefing reads (#194)#215
Merged
Merged
Conversation
…g reads (#194) Latency-only change: no injected startup state changed. See test additions for eliminated-spawn/read proof.
…nwired (CRITICAL) Security review found the performance-002 hooks-install re-probe skip (203d78d) trusted a cached hooks_dir without re-confirming git would still read hooks from that location. A LOCAL core.hooksPath change after the cache was written (e.g. adopting husky / pre-commit-framework) left install() returning [] while the NEW hooks dir got no codeArbiter shim at all -- silently voiding the H-01/H-02/H-09b/H-10b/H-14 git-level backstop. The fast path (zero git spawns) now additionally requires: the cached dir be exactly the default <root>/.git/hooks (never a cached custom hooksPath), and a direct read of .git/config (+ config.worktree, + an [include] safety net) positively confirms no local core.hooksPath override is set right now. Any miss/ambiguity falls through to the full git-based probe -- fail direction is "install when unsure," never "skip when unsure". Also keys the cache on ~/.gitconfig's mtime so a later global hooksPath edit invalidates a previously-fast-pathable cache; a system-wide (/etc/gitconfig) change remains a documented, accepted residual (cold/first install always resolves it correctly via the full probe regardless). Regression tests added: local hooksPath added/removed after install must never be skipped as a no-op; a cached CUSTOM hooksPath is never fast-pathed; _confirmed_no_local_hooks_path unit coverage (silent/set/include-directive).
…sidual doc (HIGH+MEDIUM) HIGH (second spelling of the same skip -> backstop-unwire class): the section/key line parser in _confirmed_no_local_hooks_path missed git's SAME-LINE config form -- `[core] hooksPath = x`, `[core]hooksPath=x`, and `[CORE]HooksPath=x` are all valid, real-git-honored spellings that matched _SECTION_RE and `continue`d, discarding the rest of the line, so the key was never seen. Replaced the hand-rolled parser with a grammar-free, case-insensitive substring scan for "hookspath" (and "[include") anywhere in the config text: any occurrence in any spelling/placement/casing, or any read failure, returns "not confirmed" and falls through to the real probe. This can only ever over-trigger a harmless extra git spawn on a false positive (e.g. a stray comment) -- never falsely confirm an override is absent when one is present. Strictly "install when unsure." MEDIUM: the ~/.gitconfig-only mtime invalidation token under-scoped the residual -- git also reads the XDG `~/.config/git/config` (or $XDG_CONFIG_HOME/git/config). The cache-validity token now covers both paths; the module-header residual note now correctly names GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM env overrides and /etc/gitconfig as the remaining (documented, accepted) uncovered cases. Regression tests added: same-line hooksPath (spaced, no-space, upper section+key) each proven NOT confirmed via the unit-level helper, plus an end-to-end test (with a real-git sanity check that the same-line spelling is actually honored) proving install() lands in the custom dir rather than being skipped as a no-op. All prior regression tests (git-config-driven add/remove, cached-custom-never-fast-pathed, include-directive) remain green.
Version bump + README badge sync for #194; documents the fail-safe hooks-install re-probe fast-path in security-controls.md §Hook security. Claude-Session: https://claude.ai/code/session_01PVTCDji6bEuf9SifQ8tfsa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lane E3 — #194 (performance — sessionstart-latency: performance-002/003). The linchpin SessionStart hook blocked on redundant work before the persona reached the model.
What
_githooks.install) now skips its git spawns when a cheap on-disk cache proves the shims are current — fail-safe (see below); (b) the 5 independentassemble_summarygit reads (status/rev-list/branch/worktree/stash) now run concurrently via aThreadPoolExecutorinstead of strictly sequentially — parsing is unchanged, output byte-identical.CONTEXT.md/open-tasks.md/open-questions.mdviaarbiter_stateaftermain()already read them — the already-read text is now threaded through (new optionalctx_text/ot_text/oq_textkwargs,None= original disk-read behavior for all other callers).Fail-safe hooks-install skip (reviewed hard — this touches the #161 backstop)
The skip fires ONLY when it can positively, spawn-free confirm no hooks redirect: cached dir is exactly
<root>/.git/hooksAND_confirmed_no_local_hooks_path(a grammar-free case-insensitivehookspathsubstring scan of.git/config/config.worktree— cannot under-detect any git-config spelling) finds none AND no[include]AND the shims still match the current enforcer path. Any read failure /hookspath/[include]/ cached custom hooksPath / global-config change (~/.gitconfig+XDG mtime token) → fall through to the full install. Fail direction is install-when-unsure, never skip-when-unsure — the fast path can never leave the git-enforce backstop unwired.Verification
core.hooksPathregressions);test_hooks_cold_install.pyPASS (a genuine cold install still wires the hooks); existingsession-start/wire-statusline/git-enforceassertions UNMODIFIED and green;py_compileclean; LF.core.hooksPathchange (CRITICAL) — fixed with the spawn-free.git/configconfirmation; a follow-up caught git's same-line config spelling (HIGH) — fixed with the grammar-free substring scan. Both regressions verified to fail against the pre-fix commits. Fan-out (no shared state, best-effort) and thread-through (identical parsing) cleared first pass.Bumps ca 2.8.9 → 2.8.10. Documents the fast-path posture in
security-controls.md§Hook security.Closes #194
https://claude.ai/code/session_01PVTCDji6bEuf9SifQ8tfsa