Repository navigation
[feat] Skip the per-load members.yaml git reads for teams that do not use it #1393
Description
Problem
Follow-up to #1191. Every config load now probes the team repo for manifest/members.yaml from origin/<default>, including for teams that never add the file. applyRoleAssignment runs on each loadLocalConfig, loadLocalConfigForScope and readConfigFrom, and pull/init call refreshRoleAssignment on top. Each probe costs two git subprocesses (rev-parse --verify origin/<default>^{commit} and git show origin/<default>:./manifest/members.yaml). getDefaultBranch and parseManifestOnce already cache their part; the git reads are not cached.
The feature is opt-in, so a team that does not use it should not pay for it. The cost has not been measured yet: it is two short-lived processes per load, multiplied by however many loads a command does (workspace children, multiple scopes).
Proposed Solution
- Measure first: count loads and the added wall time for
teamai pullandteamai statusin a workspace with children, with and withoutmembers.yaml. If the cost is negligible, close this. - If it is not, memoize the result of
loadRoleAssignmentsIfPresentper process, keyed by repo path and source kind, following the existingdefaultBranchCacheinsrc/utils/git.ts. Invalidate it where the team repo moves: after the fetch inpull(beforerefreshRoleAssignment) and after the clone/update ininit.
The fail-closed contract must hold: absent file is the only result that hands role selection back. A cached "absent" must not outlive a fetch that brings the file in, and a cached read error must not become "absent".
Alternatives Considered
- Keying the cache on the resolved
origin/<default>sha: no explicit invalidation, but still onerev-parseper load.
Activity
SaulMoro commentedon Oct 25, 2026
Measured on current
mainwith a workspace whose team repo has nomembers.yaml: the same build, with and withoutloadRoleAssignmentsIfPresentstubbed tonull, 15 interleaved runs per side.pull(fast path, "Already synced")statusMedians. Every probe is a pair,
rev-parse --verify origin/<default>^{commit}plusshow origin/<default>:./manifest/members.yaml, about 4.5 ms per call here.pullprobes three times: at load, inrefreshRoleAssignmentafter the fetch, and at a later load. Add onerev-parse --abbrev-ref origin/HEADthe first timegetDefaultBranchruns.What the remedies would save:
refreshRoleAssignmentremoves only the third pair onpull(about 9 ms) and nothing onstatusorroles list, which probe once. It also adds the invalidation risk described above.ls-tree origin/<default> -- ./manifest/members.yamlper probe (an error means no fetched branch, empty output means absent, thenshowonly when present) removes therev-parse: about 13 ms onpulland 4.5 ms onstatus, with no state.So the absent-file cost is about 30 ms per pull, and the cheapest safe fix recovers less than half of it. I'm not opening a PR for that now. A real saving would mean resolving the assignment once per pull instead of once per config load, which is a larger change. Leaving this open in case that becomes worth it.
SaulMoro commentedon Oct 25, 2026
Closing per the measurement above: the safe fix recovers about 13 ms per pull, which does not justify the change. Reopen if config loads multiply (for example many workspace children) and the probe cost becomes visible.