fix(commands): route nested sub_repos by longest prefix, not array order (#1130)

groupFilesBySubrepo selected the first sub_repos entry in array order
whose prefix matched a file, so a file under a more-specific nested
sub-repo (e.g. packages/core/widget.js with sub_repos
["packages", "packages/core"]) was mis-routed to the less-specific
parent ("packages").

Select the longest (most-specific) matching prefix within each
first-segment bucket instead, making routing independent of sub_repos
array order. String()-guard the length comparison so non-string entries
still never throw (preserves the #311 tolerance). Update the stale doc
comment that claimed first-match semantics.

Closes #391.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

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 {

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, []);
+  });
 });
 
 // ─────────────────────────────────────────────────────────────────────────────