diff --git a/src/commands.cts b/src/commands.cts index 78e37f5f4..4a891d14b 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -621,13 +621,12 @@ function cmdCommit(cwd: string, message: string | undefined, files: string[] | u /** * Route a list of changed files to their sub-repo prefixes. * - * Bucket sub-repos by their first path segment. Any file that matches a + * Bucket sub-repos by their first path segment (#311). Any file that matches a * sub-repo prefix must share that sub-repo's first segment, so we only scan - * the (small) bucket for the file's first segment instead of all sub-repos - * — O(F + R) expected vs the prior O(F*R) find-in-loop. Candidates stay in - * sub-repo array order, preserving the original first-match semantics - * (incl. multi-segment sub-repos like "vendor/pkg", which resolve via the - * inner startsWith). (#311) + * the (small) same-first-segment bucket instead of all sub-repos. Within that + * bucket all candidates are scanned to find the longest (most-specific) + * matching prefix, so nested sub_repos (e.g. ['packages', 'packages/core']) + * route to the deepest match regardless of sub_repos array order (#391). * * @param files - changed file paths (relative to project root) * @param subRepos - sub-repo path prefixes from config.sub_repos @@ -644,7 +643,23 @@ function groupFilesBySubrepo(files: string[], subRepos: string[]): GroupFilesByS const unmatched: string[] = []; for (const file of files) { const candidates = reposByFirstSeg.get(file.split('/')[0]); - const match = candidates ? candidates.find(repo => file.startsWith(repo + '/')) : undefined; + // Select the longest (most-specific) matching sub-repo prefix so nested + // sub_repos (e.g. ['packages', 'packages/core']) route correctly regardless + // of array order. (#391) String() guards the length read so non-string + // entries never throw, matching the tolerance of the prior `.find` path. + let match: string | undefined; + let matchLen = -1; + if (candidates) { + for (const repo of candidates) { + if (file.startsWith(repo + '/')) { + const repoLen = String(repo).length; + if (repoLen > matchLen) { + match = repo; + matchLen = repoLen; + } + } + } + } if (match) { (grouped[match] ||= []).push(file); } else { diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 96052db29..abfea1b9e 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -1481,13 +1481,50 @@ describe('groupFilesBySubrepo (#311)', () => { assert.deepStrictEqual(result.unmatched, ['vendor/other.js']); }); - test('first-match-in-array-order wins (not longest-prefix)', () => { - // 'app' comes before 'app/sub' in subRepos array; 'app/sub/f.js' matches 'app' first + test('longest-prefix wins, not first-match-in-array-order (#391)', () => { + // 'app' precedes 'app/sub' in array order, but 'app/sub' is the more specific + // configured sub-repo, so 'app/sub/f.js' must route to 'app/sub'. const result = groupFilesBySubrepo( ['app/sub/f.js'], ['app', 'app/sub'] ); - assert.deepStrictEqual(result.grouped, { app: ['app/sub/f.js'] }); + assert.deepStrictEqual(result.grouped, { 'app/sub': ['app/sub/f.js'] }); + assert.deepStrictEqual(result.unmatched, []); + }); + + test('longest-prefix selection is independent of sub_repos array order (#391)', () => { + // Reverse array order: longest-prefix must still win (no array-order workaround). + const result = groupFilesBySubrepo( + ['app/sub/f.js'], + ['app/sub', 'app'] + ); + assert.deepStrictEqual(result.grouped, { 'app/sub': ['app/sub/f.js'] }); + assert.deepStrictEqual(result.unmatched, []); + }); + + test('nested sub-repos route by specificity; shallow files stay shallow (#391)', () => { + // Exact repro from #391 plus a shallow sibling file under the parent sub-repo. + const result = groupFilesBySubrepo( + ['packages/core/widget.js', 'packages/util.js'], + ['packages', 'packages/core'] + ); + assert.deepStrictEqual(result.grouped, { + 'packages/core': ['packages/core/widget.js'], + packages: ['packages/util.js'], + }); + assert.deepStrictEqual(result.unmatched, []); + }); + + test('three-level nesting routes to the deepest matching prefix (#391)', () => { + const result = groupFilesBySubrepo( + ['a/b/c/f.js', 'a/b/g.js', 'a/h.js'], + ['a', 'a/b', 'a/b/c'] + ); + assert.deepStrictEqual(result.grouped, { + 'a/b/c': ['a/b/c/f.js'], + 'a/b': ['a/b/g.js'], + a: ['a/h.js'], + }); assert.deepStrictEqual(result.unmatched, []); }); @@ -1524,6 +1561,17 @@ describe('groupFilesBySubrepo (#311)', () => { assert.deepStrictEqual(result.grouped, { a: ['a/b'] }); assert.deepStrictEqual(result.unmatched, ['README.md']); }); + + test('non-string entry in a matched bucket does not throw (#391)', () => { + // A null sub_repos entry shares the 'null' first-segment bucket with a real + // multi-segment entry; longest-prefix selection must not throw reading length. + let result; + assert.doesNotThrow(() => { + result = groupFilesBySubrepo(['null/a/f.js'], [null, 'null/a']); + }); + assert.deepStrictEqual(result.grouped, { 'null/a': ['null/a/f.js'] }); + assert.deepStrictEqual(result.unmatched, []); + }); }); // ─────────────────────────────────────────────────────────────────────────────