fix(public): resolve duplicate guide slugs by precedence, not scan order

modules/public/helpers/public.docs.tree.js

@@ -19,11 +19,18 @@
  * every audience ({@link DEFAULT_PERSONA}). A guide whose prefix falls outside
  * every configured range (or when no sections are configured) is grouped under
  * its capitalised module name so it is never silently dropped.
+ *
+ * A slug collision between two guide files is resolved by {@link resolveGuideEntries}
+ * BEFORE the tree is built, using a precedence policy (application-level guide
+ * overrides a framework-provided one) — see that function for details. The
+ * caller (the public docs service) runs the same deduped list through both
+ * `buildDocsTree` and its slug index, so the two endpoints can never disagree.
  */
 import fs from 'fs';
 import path from 'path';
 
 import logger from '../../../lib/services/logger.js';
+import configHelper from '../../../lib/helpers/config.js';
 
 /**
  * Default persona audience applied when a section declares none.
@@ -182,6 +189,77 @@ const loadGuideEntries = (filePaths) => {
     .sort((a, b) => a.order - b.order || a.slug.localeCompare(b.slug));
 };
 
+/**
+ * Precedence tier for a guide entry on a slug collision. A module in
+ * `configHelper.CORE_MODULES` (`core`/`auth`/`users`/`home` — the stack's
+ * existing "ships with the framework, never deactivated" classification,
+ * see `lib/helpers/config.js`) is framework-provided; every other module is
+ * application-level — added by the downstream consumer project (either a
+ * wholly new module, or extra guides dropped into an existing module's
+ * `doc/guides/`). Guides ship exclusively under `modules/home/doc/guides`
+ * today (see the two shipped samples), so in practice this currently
+ * resolves to "home vs. everything else" — but it reuses the stack's single
+ * existing framework-module set rather than declaring a second, narrower one.
+ * @param {{ path: string }} entry - Guide entry (from {@link loadGuideEntries}).
+ * @returns {'framework'|'application'} Precedence tier.
+ */
+const precedenceTier = (entry) => (configHelper.CORE_MODULES.has(moduleFromPath(entry.path)) ? 'framework' : 'application');
+
+/**
+ * Resolve slug collisions across guide entries by precedence, producing the
+ * single deduped list that feeds BOTH `buildDocsTree` and the service's slug
+ * index — so the listing and the fetch endpoint can never disagree, and the
+ * outcome no longer depends on file-scan order.
+ *
+ * Policy:
+ *   1. **Cross-tier collision** — an application-level guide (per
+ *      {@link precedenceTier}) always overrides a framework-provided one,
+ *      regardless of scan order. This is the supported override mechanism
+ *      for a consumer that ships its own `00-welcome`/`01-quickstart`; it is
+ *      silent (debug-logged only), not an error.
+ *   2. **Same-tier collision** (two framework guides, or two application
+ *      guides) — precedence cannot disambiguate, so the kept guide is decided
+ *      by {@link loadGuideEntries}'s existing deterministic sort (numeric
+ *      prefix, then slug): the later entry in that order wins, same as before
+ *      this fix. A warning is logged naming the actual policy, since neither
+ *      guide "wins" by design.
+ *
+ * @param {ReturnType<typeof loadGuideEntries>} entries - Structured guides,
+ *   already sorted by {@link loadGuideEntries}.
+ * @returns {ReturnType<typeof loadGuideEntries>} Entries with slug collisions
+ *   resolved — at most one entry per slug, original relative order preserved.
+ */
+const resolveGuideEntries = (entries) => {
+  const list = Array.isArray(entries) ? entries : [];
+  const winners = new Map(); // slug -> winning entry
+  for (const entry of list) {
+    const incumbent = winners.get(entry.slug);
+    if (!incumbent) {
+      winners.set(entry.slug, entry);
+      continue;
+    }
+    const incumbentTier = precedenceTier(incumbent);
+    const challengerTier = precedenceTier(entry);
+    if (incumbentTier === challengerTier) {
+      logger.warn(
+        `[public/docs] duplicate guide slug "${entry.slug}" between two ${challengerTier} guides `
+        + `("${incumbent.path}" vs "${entry.path}") — same precedence tier, so the kept guide is decided by file order, `
+        + 'not policy; rename one guide, or move it to a module with different precedence (application overrides framework), to resolve deliberately.',
+      );
+      winners.set(entry.slug, entry); // later entry wins — existing deterministic tiebreak
+      continue;
+    }
+    if (challengerTier === 'application') {
+      // incumbentTier must be 'framework' here — the equal-tier case already
+      // returned above, and 'application'/'framework' are the only two tiers.
+      logger.debug(`[public/docs] guide slug "${entry.slug}" — application guide (${entry.path}) overrides framework guide (${incumbent.path})`);
+      winners.set(entry.slug, entry);
+    }
+    // else: challenger is framework, incumbent is application — incumbent already wins.
+  }
+  return list.filter((entry) => winners.get(entry.slug) === entry);
+};
+
 /**
  * Slugify a label into a URL-safe category id.
  * @param {string} label - Human category label.
@@ -282,5 +360,7 @@ export default {
   stripLeadingH1,
   firstParagraph,
   loadGuideEntries,
+  precedenceTier,
+  resolveGuideEntries,
   buildDocsTree,
 };

modules/public/services/public.docs.service.js

@@ -38,19 +38,17 @@ const guideFiles = () => (Array.isArray(config.files?.guides) ? [...config.files
 const guideSections = () => (Array.isArray(config.docs?.guideSections) ? config.docs.guideSections.map((s) => ({ ...s })) : []);
 
 /**
- * @desc Build the docs tree + slug index from disk.
+ * @desc Build the docs tree + slug index from disk. Slug collisions are
+ * resolved once, up front, by {@link docsTree.resolveGuideEntries} (precedence
+ * policy: application-level guide overrides a framework-provided one). Both the
+ * tree and the slug index are built from that SAME deduped entry list, so the
+ * listing (`getTree`) and the fetch endpoint (`getMarkdown`) can never disagree.
  * @returns {{ tree: { categories: Object[] }, bySlug: Map<string, Object> }}
  */
 const compute = () => {
-  const entries = docsTree.loadGuideEntries(guideFiles());
+  const entries = docsTree.resolveGuideEntries(docsTree.loadGuideEntries(guideFiles()));
   const tree = docsTree.buildDocsTree(entries, guideSections());
-  const bySlug = new Map();
-  for (const entry of entries) {
-    if (bySlug.has(entry.slug)) {
-      logger.warn(`[public/docs] duplicate guide slug "${entry.slug}" — later guide wins; rename one to avoid the collision`);
-    }
-    bySlug.set(entry.slug, entry);
-  }
+  const bySlug = new Map(entries.map((entry) => [entry.slug, entry]));
   return { tree, bySlug };
 };
 

modules/public/tests/public.docs.integration.tests.js

@@ -8,13 +8,16 @@
  */
 import request from 'supertest';
 import path from 'path';
+import fs from 'fs';
+import os from 'os';
 
 import {
-  afterAll, beforeAll, beforeEach, describe, test, expect,
+  jest, afterAll, beforeAll, beforeEach, describe, test, expect,
 } from '@jest/globals';
 import { bootstrap } from '../../../lib/app.js';
 import mongooseService from '../../../lib/services/mongoose.js';
 import config from '../../../config/index.js';
+import logger from '../../../lib/services/logger.js';
 
 describe('Public docs integration tests:', () => {
   let app;
@@ -112,3 +115,128 @@ describe('Public docs integration tests:', () => {
     expect(result.text.trimStart().startsWith('---')).toBe(false);
   });
 });
+
+describe('Public docs integration tests — slug-collision precedence:', () => {
+  let app;
+  let PublicDocsService;
+  let debugSpy;
+  let tmpDir;
+  let appGuidePath;
+  let reverseFixtureAbsDir;
+  let reverseFixtureAbsPath;
+  let reverseGuideRelPath;
+  const originalOrgEnabled = config.organizations?.enabled;
+  const originalGuides = Array.isArray(config.files?.guides) ? [...config.files.guides] : [];
+
+  beforeAll(async () => {
+    if (config.organizations) config.organizations.enabled = false;
+    const init = await bootstrap();
+    app = init.app;
+    PublicDocsService = (await import(path.resolve('./modules/public/services/public.docs.service.js'))).default;
+    debugSpy = jest.spyOn(logger, 'debug');
+
+    // A real on-disk fixture guide, under a module path other than "home" —
+    // simulates a consumer/application module shipping its own "welcome"
+    // guide, colliding with the framework-shipped modules/home/00-welcome.md.
+    // Path substring only needs to contain "modules/<name>/", it does not
+    // need to live under the app's real modules/ directory. loadGuideEntries
+    // sorts ALL guide paths alphabetically before assigning scan order, and an
+    // absolute OS-tmpdir path always sorts before the relative "modules/..."
+    // path config.files.guides uses for real guides — so this fixture is
+    // scanned BEFORE modules/home/00-welcome.md (app is the earlier entry,
+    // already-incumbent when the framework guide arrives as challenger — the
+    // SILENT "incumbent already wins" branch in resolveGuideEntries, no
+    // logger.debug call).
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'public-docs-precedence-'));
+    const appModuleDir = path.join(tmpDir, 'modules', 'app-fixture', 'doc', 'guides');
+    fs.mkdirSync(appModuleDir, { recursive: true });
+    appGuidePath = path.join(appModuleDir, '00-welcome.md');
+    fs.writeFileSync(appGuidePath, '# App Welcome\n\nApplication-level welcome guide.\n');
+
+    // Second fixture, deliberately scanned in the OPPOSITE direction. The
+    // file is still written at an absolute path (fs calls need one), but the
+    // string pushed into config.files.guides below is made GENUINELY
+    // relative via path.relative() — path.join(process.cwd(), ...) is
+    // ALWAYS absolute (an earlier version of this test wrongly assumed
+    // otherwise, so both fixtures ended up absolute and this direction was
+    // never actually exercised — caught by review). A relative string
+    // starting with "zzz-" sorts AFTER "modules/home/...", so THIS fixture
+    // is scanned AFTER modules/home/01-quickstart.md: the framework guide is
+    // incumbent, the app guide is the LATER challenger that must explicitly
+    // override it — the logger.debug branch in resolveGuideEntries, asserted
+    // below.
+    reverseFixtureAbsDir = path.join(process.cwd(), 'zzz-tmp-pr3979-fixture', 'doc', 'guides');
+    fs.mkdirSync(reverseFixtureAbsDir, { recursive: true });
+    reverseFixtureAbsPath = path.join(reverseFixtureAbsDir, '01-quickstart.md');
+    fs.writeFileSync(reverseFixtureAbsPath, '# App Quickstart\n\nApplication-level quickstart guide.\n');
+    reverseGuideRelPath = path.relative(process.cwd(), reverseFixtureAbsPath);
+
+    config.files.guides = [...originalGuides, appGuidePath, reverseGuideRelPath];
+  });
+
+  afterAll(async () => {
+    if (config.organizations) config.organizations.enabled = originalOrgEnabled;
+    config.files.guides = originalGuides;
+    if (debugSpy) debugSpy.mockRestore();
+    if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
+    if (reverseFixtureAbsDir) fs.rmSync(path.join(process.cwd(), 'zzz-tmp-pr3979-fixture'), { recursive: true, force: true });
+    await mongooseService.disconnect();
+  });
+
+  beforeEach(() => {
+    if (PublicDocsService) PublicDocsService.clearCache();
+    if (debugSpy) debugSpy.mockClear();
+  });
+
+  test('the application guide wins the "welcome" slug in the listing (no duplicate) — app scanned BEFORE the framework guide (silent "incumbent already wins" branch)', async () => {
+    const result = await request(app).get('/api/public/docs').expect(200);
+    const { categories } = result.body.data;
+    const guides = categories.flatMap((c) => c.guides);
+    const welcomeGuides = guides.filter((g) => g.slug === 'welcome');
+
+    // Exactly one "welcome" entry — the listing never shows a duplicate.
+    expect(welcomeGuides).toHaveLength(1);
+    expect(welcomeGuides[0].title).toBe('App Welcome');
+
+    // This direction wins because the app guide is already incumbent — it
+    // does NOT go through the explicit override branch (that's proven
+    // separately by the "quickstart" case below). Scope the assertion to
+    // "welcome" specifically: the same compute() pass also resolves the
+    // quickstart collision (both fixtures share one config.files.guides),
+    // which DOES log — asserting a blanket "not called" here would be a
+    // false negative.
+    expect(debugSpy).not.toHaveBeenCalledWith(expect.stringContaining('guide slug "welcome"'));
+  });
+
+  test('the application guide wins the "welcome" slug on fetch — listing and fetch agree', async () => {
+    const result = await request(app).get('/api/public/docs/welcome.md').expect(200);
+    expect(result.text).toContain('Application-level welcome guide.');
+    expect(result.text).not.toContain('Welcome to');
+  });
+
+  test('the application guide overrides the framework guide for "quickstart" — app scanned AFTER it (opposite scan-order direction), proven via the explicit override branch firing', async () => {
+    const result = await request(app).get('/api/public/docs').expect(200);
+    const { categories } = result.body.data;
+    const guides = categories.flatMap((c) => c.guides);
+    const quickstartGuides = guides.filter((g) => g.slug === 'quickstart');
+
+    expect(quickstartGuides).toHaveLength(1);
+    expect(quickstartGuides[0].title).toBe('App Quickstart');
+
+    const fetchResult = await request(app).get('/api/public/docs/quickstart.md').expect(200);
+    expect(fetchResult.text).toContain('Application-level quickstart guide.');
+    expect(fetchResult.text).not.toContain('<YOUR_API_KEY>');
+
+    // Proof this exercised the explicit "challenger overrides incumbent"
+    // branch in resolveGuideEntries (public.docs.tree.js) — that branch is
+    // the ONLY place this debug line is emitted, so its presence is direct
+    // evidence the framework guide was incumbent and the app guide, scanned
+    // later, actively overrode it (not merely "already winning").
+    expect(debugSpy).toHaveBeenCalledWith(
+      expect.stringContaining('guide slug "quickstart"'),
+    );
+    expect(debugSpy).toHaveBeenCalledWith(
+      expect.stringContaining('overrides framework guide'),
+    );
+  });
+});

modules/public/tests/public.docs.service.fallback.unit.tests.js

@@ -21,6 +21,7 @@ jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
 jest.unstable_mockModule('../helpers/public.docs.tree.js', () => ({
   default: {
     loadGuideEntries: jest.fn().mockReturnValue([]),
+    resolveGuideEntries: jest.fn((entries) => entries),
     buildDocsTree: jest.fn().mockReturnValue({ categories: [] }),
   },
 }));

modules/public/tests/public.docs.service.unit.tests.js

@@ -2,8 +2,11 @@
  * Unit tests for the public docs service.
  * Verifies tree assembly (driven by config.docs.guideSections), slug lookup,
  * the 404 (null) path, and TTL caching — with config + the docs-tree helper
- * mocked so the test never touches disk. (The tree parsing itself is exercised
- * in public.docs.tree.unit.tests.js.)
+ * mocked so the test never touches disk. (The tree parsing itself, including
+ * the slug-collision precedence policy, is exercised in
+ * public.docs.tree.unit.tests.js — this file only verifies that the service
+ * wires `loadGuideEntries` → `resolveGuideEntries` → both `buildDocsTree` and
+ * `bySlug`, so the two endpoints are built from the SAME deduped list.)
  */
 import {
   jest, describe, test, expect, beforeEach,
@@ -34,9 +37,10 @@ jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
 
 const loadGuideEntries = jest.fn();
 const buildDocsTree = jest.fn();
+const resolveGuideEntries = jest.fn();
 
 jest.unstable_mockModule('../helpers/public.docs.tree.js', () => ({
-  default: { loadGuideEntries, buildDocsTree },
+  default: { loadGuideEntries, buildDocsTree, resolveGuideEntries },
 }));
 
 const PublicDocsService = (await import('../services/public.docs.service.js')).default;
@@ -56,12 +60,16 @@ describe('PublicDocsService', () => {
     jest.clearAllMocks();
     PublicDocsService.clearCache();
     loadGuideEntries.mockReturnValue(sampleEntries);
+    // Pass-through by default — collision resolution itself is unit-tested in
+    // public.docs.tree.unit.tests.js; here we only verify the wiring.
+    resolveGuideEntries.mockImplementation((entries) => entries);
     buildDocsTree.mockReturnValue(sampleTree);
   });
 
-  test('getTree builds the tree from the configured guide files + sections', () => {
+  test('getTree builds the tree from the configured guide files + sections, via resolveGuideEntries', () => {
     const tree = PublicDocsService.getTree();
     expect(loadGuideEntries).toHaveBeenCalledWith(guideFilesPaths);
+    expect(resolveGuideEntries).toHaveBeenCalledWith(sampleEntries);
     expect(buildDocsTree).toHaveBeenCalledWith(sampleEntries, guideSections);
     expect(tree).toBe(sampleTree);
   });
@@ -92,23 +100,26 @@ describe('PublicDocsService', () => {
     expect(loadGuideEntries).toHaveBeenCalledTimes(2);
   });
 
-  test('warns and last-wins when two entries share the same slug', () => {
-    const dupeEntries = [
+  test('bySlug is built from resolveGuideEntries output, not the raw loaded entries — tree and slug index cannot disagree', () => {
+    const rawEntries = [
       {
         slug: 'quickstart', title: 'Quickstart A', order: 0, summary: 'a', body: 'Body A',
       },
       {
         slug: 'quickstart', title: 'Quickstart B', order: 1, summary: 'b', body: 'Body B',
       },
     ];
-    loadGuideEntries.mockReturnValueOnce(dupeEntries);
+    // Collision already resolved upstream — resolveGuideEntries returns a
+    // single winner, same as the real helper would.
+    const deduped = [rawEntries[1]];
+    loadGuideEntries.mockReturnValueOnce(rawEntries);
+    resolveGuideEntries.mockReturnValueOnce(deduped);
 
     PublicDocsService.getTree();
 
-    expect(mockLogger.warn).toHaveBeenCalledWith(
-      expect.stringContaining('duplicate guide slug "quickstart"'),
-    );
-    // Last entry wins
+    // buildDocsTree (the listing) receives the deduped list, not the raw one.
+    expect(buildDocsTree).toHaveBeenCalledWith(deduped, guideSections);
+    // bySlug (the fetch endpoint) agrees with the winner buildDocsTree saw.
     expect(PublicDocsService.getMarkdown('quickstart')).toBe('Body B');
   });
 });