Fix memory-core QMD slugified path mapping for legacy collection names

[rrpsantos]

Fixes a deterministic extensions failure seen in CI (checks-fast-extensions):

• extensions/memory-core/src/memory/qmd-manager.slugified-paths.test.ts was expecting slugified QMD URIs like qmd://vault-main/... to map back to indexed paths, but results were [].

Root cause:

• Collection root lookup was too strict: it only looked up an exact collection key in collectionRoots. In some scenarios, the manager’s configured collections use an agent suffix (e.g. workspace-main), while search results / indexed paths can reference the legacy/alternate name.

Change:

• Add a collection lookup helper that tries both the provided collection name and an agent-suffixed/unsuffixed variant, then uses that for path resolution.

Local QA:

• corepack pnpm check
• corepack pnpm vitest run --config vitest.extensions.config.ts extensions/memory-core/src/memory/qmd-manager.slugified-paths.test.ts

[greptile-apps]

Greptile Summary

This PR fixes a path resolution failure in QmdMemoryManager where slugified QMD URIs (e.g., qmd://workspace-main/...) returned by the search backend could not be mapped back to indexed filesystem paths because the collection name in the search result (workspace-main) differed from the configured collection name (workspace) by the agent suffix.

Changes:

• Adds listCollectionLookupNames(collection): generates the exact name plus one alternate (strips or appends the agent suffix) to try when the exact key is absent from collectionRoots.
• Adds resolveCollectionRoot(collection): iterates the candidate names and returns the first matching CollectionRoot, replacing the three direct collectionRoots.get(collection) calls in toCollectionRelativePath, toDocLocation, and resolveReadPath.
• Three new integration-style tests cover the workspace collection path, an extra-vault collection path, and slug-collision disambiguation.

Minor note: readCounts() at line 2162 still uses a raw collectionRoots.get(row.collection) call; in the same mismatch scenario this PR addresses, it would silently bucket documents under the wrong source kind in the status() output.

Confidence Score: 5/5

Safe to merge; the path-resolution logic is sound, and the one remaining finding is a stats-only inconsistency in readCounts().

All functional path-resolution call sites have been updated to use the new resolveCollectionRoot helper, the fix is well-covered by three new tests, and the sole outstanding finding (direct collectionRoots.get in readCounts()) only affects display metrics — not data integrity or correctness.

extensions/memory-core/src/memory/qmd-manager.ts — specifically the readCounts() method at line 2162, which still uses a direct map lookup.

Important Files Changed

Filename	Overview
extensions/memory-core/src/memory/qmd-manager.ts	Adds listCollectionLookupNames and resolveCollectionRoot helpers to handle agent-suffixed vs. base collection name mismatches, and applies them to the three path-resolution call sites; one remaining direct collectionRoots.get in readCounts() is a minor stats-only inconsistency.

Comments Outside Diff (1)

1. extensions/memory-core/src/memory/qmd-manager.ts, line 2162-2163 (link)

   readCounts() still uses a direct map lookup

   readCounts() still uses this.collectionRoots.get(row.collection) directly instead of the new resolveCollectionRoot helper. In the exact scenario this PR fixes — where the DB has documents indexed under an agent-suffixed name (e.g., workspace-main) but collectionRoots only holds the base name (workspace) — this lookup will return undefined, causing every document in that collection to be bucketed under the fallback "memory" source kind rather than its real kind. The impact is limited to the status() reporting surface (sourceCounts), but it's the same class of mismatch the rest of the PR corrects.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/memory-core/src/memory/qmd-manager.ts
   Line: 2162-2163
   
   Comment:
   **`readCounts()` still uses a direct map lookup**
   
   `readCounts()` still uses `this.collectionRoots.get(row.collection)` directly instead of the new `resolveCollectionRoot` helper. In the exact scenario this PR fixes — where the DB has documents indexed under an agent-suffixed name (e.g., `workspace-main`) but `collectionRoots` only holds the base name (`workspace`) — this lookup will return `undefined`, causing every document in that collection to be bucketed under the fallback `"memory"` source kind rather than its real kind. The impact is limited to the `status()` reporting surface (`sourceCounts`), but it's the same class of mismatch the rest of the PR corrects.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/memory-core/src/memory/qmd-manager.ts
Line: 2162-2163

Comment:
**`readCounts()` still uses a direct map lookup**

`readCounts()` still uses `this.collectionRoots.get(row.collection)` directly instead of the new `resolveCollectionRoot` helper. In the exact scenario this PR fixes — where the DB has documents indexed under an agent-suffixed name (e.g., `workspace-main`) but `collectionRoots` only holds the base name (`workspace`) — this lookup will return `undefined`, causing every document in that collection to be bucketed under the fallback `"memory"` source kind rather than its real kind. The impact is limited to the `status()` reporting surface (`sourceCounts`), but it's the same class of mismatch the rest of the PR corrects.

```suggestion
        const root = this.resolveCollectionRoot(row.collection);
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(memory-core): resolve qmd collection..." | Re-trigger Greptile}

[byungsker]

Good approach for bridging the legacy vs. slugified collection naming gap. A couple of things to consider:

1. listCollectionLookupNames early-exit optimization: The first thing the method does is check this.collectionRoots.has(trimmed) and returns early if found, avoiding the suffix logic. That's a nice short-circuit, but the comment-less code makes it easy to miss. A brief inline comment (e.g. // already registered verbatim) would help the next reader.

2. Bidirectional ambiguity: If a collection is named foo-<agentId> and another is also registered as foo, listCollectionLookupNames('foo') will return ['foo', 'foo-<agentId>'] and resolveCollectionRoot will match foo first (correct). But listCollectionLookupNames('foo-<agentId>') will also try the legacy name foo as a secondary candidate — is that desired, or could it produce cross-collection path confusion?

3. Missing test for the ambiguous case: A unit test that registers both foo and foo-<agentId> and verifies which root wins in each lookup direction would lock in the intended behavior.

4. resolveDocLocation / other callers: Are there other call sites that still reference this.collectionRoots.get(collection) directly? It might be worth a quick grep to make sure none were missed.

[byungsker]

Nice approach using listCollectionLookupNames + resolveCollectionRoot to transparently handle both new slugified names and legacy names. A few thoughts:

Edge case — mutual collision: If a collection is named "foo-agentid" literally (not as a generated slug), listCollectionLookupNames would strip the suffix and also try "foo", potentially resolving to the wrong root. Is there a guard against this, or is it acceptable given how collection names are generated?

Array.from(new Set(names)): This deduplication is only necessary when trimmed already ends with agentSuffix AND the stripped prefix equals trimmed itself (i.e. empty prefix). That seems impossible given the if (legacyName) guard, so the Set wrap is a no-op in practice. Can simplify to just returning names.

resolveDocLocation / other callers: The three call sites updated (toCollectionRelativePath, the path resolver in the unnamed method, and listCollectionLookupNames itself) look complete. Are there any other places that call collectionRoots.get(collection) directly? A codebase search would confirm there are no remaining raw .get() calls that bypass the new fallback.

No test update: The PR description mentions this fixes a legacy name mapping issue — adding a test with a slugified name that maps to a legacy root would lock in the behavior and document the intended semantics.

Overall the abstraction is clean and the fix is targeted. The edge case around name collisions is the main thing worth verifying.

[rrpsantos]

Closing this PR. It was opened from an autonomous agent workflow without the explicit approval we now require for external OSS contributions. Preserving local evidence, but withdrawing the upstream contribution. Apologies for the noise.