Skip to content

fix(oas:sync): match the platform's OAS-upload output (tag index pages, root ordering, slug casing) #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rossrdme wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(oas:sync): match the platform's OAS-upload output (tag index pages, root ordering, slug casing) #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c738b2
fix(oas:sync): match the platform's OAS-upload output
rossrdme Jul 17, 2026
86bd1b4
fix(oas:sync): address review — reserve index.md, drop explicit hidden
rossrdme Jul 17, 2026
363d3de
fix(oas:sync): rework per review — reference-wide slugs, backfill ind…
rossrdme Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 129 additions & 9 deletions src/commands/oas-sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,96 @@ function buildPageContent({ oasFilename, operationId }) {
file: oasFilename,
operationId,
},
// Mirror the platform's OAS-upload behavior: a newly added endpoint is
// always written `hidden: false`, even when its tag and siblings are
// `hidden: true`. The backend does not infer this from a missing field, so
// it must be written explicitly.
//
// @todo Honor the `x-internal` OpenAPI extension for page visibility, to
// match gitto#2095 (RM-4616 / CX-3303): resolve `hidden` from operation-level
// `x-internal`, falling back to root-level, else false; and hide a tag's
// index page when all of its operations are `x-internal: true`. Deferred to
// keep oas:sync create-only — the resync-side rules (re-applying x-internal
// to existing pages, parent hide-ratchet) would require mutating existing
// pages, which this command intentionally never does.
hidden: false,
};

return matter.stringify('', frontmatter);
}

/**
* Build the category landing page for a tag (mirrors what the ReadMe platform
* generates on OAS upload): title from the tag name, excerpt from the tag's
* description in the spec's top-level `tags` array.
*/
function buildTagIndexContent(tagName, description) {
const frontmatter = { title: tagName };
if (description) frontmatter.excerpt = description;
// As with operation pages, upload always stamps hidden: false on new pages.
frontmatter.hidden = false;

return matter.stringify('', frontmatter);
}

/**
* Collect every slug already used across the entire reference/ tree. Reference
* page slugs share one flat namespace (docs/ is a separate namespace and is not
* consulted), so a generated operation slug must be unique against all of them.
* A page's slug is its filename without `.md`; a category page's slug (a folder
* containing `index.md`) is the folder name. Comparison is case-insensitive.
*/
function collectReferenceSlugs(refDir) {
const slugs = new Set();

function walk(dir) {
for (const entry of fs.readdirSync(dir)) {
const full = path.join(dir, entry);
let stat;
try {
stat = fs.statSync(full);
} catch {
continue;
}
if (stat.isDirectory()) {
walk(full);
} else if (entry.endsWith('.md')) {
// A folder's index.md contributes the folder name as a slug; any other
// page contributes its own filename.
const slug = entry === 'index.md' ? path.basename(dir) : path.basename(entry, '.md');
slugs.add(slug.toLowerCase());
}
}
}

walk(refDir);
return slugs;
Comment on lines +227 to +251

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve slug ownership when deleting one of several duplicate pages.

Set collapses duplicate legacy slugs. Line 311 therefore releases a slug even when another page or category still owns it, allowing a later operation to reuse that supposedly reserved slug. This is especially relevant when migrating trees previously permitted to reuse slugs across tags.

Track reference counts per lowercase slug, decrementing on deletion and removing only at zero. Add a regression with two existing pages sharing a slug, one stale and one retained.

Also applies to: 303-311

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/oas-sync.js` around lines 227 - 251, Replace the Set-based slug
tracking in collectReferenceSlugs with lowercase-slug reference counts so
duplicate pages/categories retain their ownership. Update the deletion logic
around the slug release at line 311 to decrement the count and remove the slug
only when it reaches zero. Add a regression covering two existing pages with the
same slug where deleting one leaves that slug reserved by the retained page.

@erunion erunion Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rossrdme What do you think about this? If they have duplicate slugs in their directory this isn't going to support that. Probably fine since we don't support that in bidi anyways?

}

/**
* Reserve a unique reference slug. `index` is never usable by an operation (it's
* reserved for the tag category page), and any slug already present in the
* reference namespace gets a numeric suffix (`-1`, `-2`, ...) until it's free.
* The chosen slug is added to `takenSlugs` so later operations see it.
*/
function reserveSlug(takenSlugs, base) {
let chosen = base;
if (base === 'index' || takenSlugs.has(base)) {
let n = 1;
while (takenSlugs.has(`${base}-${n}`)) n += 1;
chosen = `${base}-${n}`;
}
takenSlugs.add(chosen);
return chosen;
}

/**
* Run the sync for a single OAS file. Returns changes for that file.
*
* `takenSlugs` is the reference-wide set of slugs already in use; it is read and
* mutated so slugs stay unique across every spec processed in one sync run.
*/
function syncOneOas(refDir, oasFilename, spec) {
function syncOneOas(refDir, oasFilename, spec, takenSlugs) {
const specOps = extractOperations(spec);
const infoTitle = safeSegment(
spec.info?.title || path.basename(oasFilename, path.extname(oasFilename)),
Expand All @@ -211,6 +292,14 @@ function syncOneOas(refDir, oasFilename, spec) {

const changes = { added: [], deleted: [], skipped: [] };

// Tag descriptions from the spec's top-level `tags` array, used for the
// per-tag category landing page (index.md).
const tagDescriptions = new Map(
(Array.isArray(spec.tags) ? spec.tags : [])
.filter((t) => t && t.name)
.map((t) => [t.name, t.description || null]),
);

// Deletes: pages referencing operations that no longer exist.
for (const [opId, page] of pagesByOpId) {
if (!specOps.has(opId)) {
Expand All @@ -219,24 +308,53 @@ function syncOneOas(refDir, oasFilename, spec) {
const pageDir = path.dirname(page.filePath);
const slug = path.basename(page.filePath, '.md');
removeFromOrder(path.join(pageDir, '_order.yaml'), slug);
takenSlugs.delete(slug.toLowerCase());

changes.deleted.push(page.relativePath);
}
}

// Adds: operations with no page yet. Title/excerpt are owned by the OAS spec
// at render time, so generated pages carry only the api reference.
// Ensure every tag present in the spec has its category landing page (index.md)
// and is ordered — independent of whether its operation pages are new. Doing
// this as its own pass (rather than only when creating a new op page) backfills
// category pages for references first synced by a CLI version that didn't
// generate them, and recreates one that was deleted.
const specTags = new Set([...specOps.values()].map((op) => op.tag || 'Other'));
for (const rawTag of specTags) {
const tag = safeSegment(rawTag, 'Other');
const pageDir = path.join(refDir, infoTitle, tag);
if (!isWithin(refDir, pageDir)) continue;

const indexPath = path.join(pageDir, 'index.md');
if (!fs.existsSync(indexPath)) {
// Never overwrite an existing index.md — it may be a hand-written category.
fs.mkdirSync(pageDir, { recursive: true });
fs.writeFileSync(indexPath, buildTagIndexContent(rawTag, tagDescriptions.get(rawTag)));
changes.added.push(path.relative(refDir, indexPath));
}
// The category page's slug is the tag folder name; reserve it so no operation
// takes it. Ordering entries are idempotent, so this is a no-op when present.
takenSlugs.add(tag.toLowerCase());
addToOrder(path.join(refDir, infoTitle, '_order.yaml'), tag);
addToOrder(path.join(refDir, '_order.yaml'), infoTitle);
}

// Adds: operation pages with no page yet. Title/excerpt are owned by the OAS
// spec at render time, so generated pages carry only the api reference. Slugs
// are lowercased to match the platform's OAS-upload output.
for (const [opId, op] of specOps) {
if (pagesByOpId.has(opId)) continue;

const tag = safeSegment(op.tag || 'Other', 'Other');
const slug = safeSegment(opId, 'operation');
const pageDir = path.join(refDir, infoTitle, tag);
// Reference slugs share one flat namespace, so uniquify against every slug
// already in reference/ — a collision (or the reserved `index` slug) gets a
// numeric suffix rather than being skipped.
const slug = reserveSlug(takenSlugs, safeSegment(opId, 'operation').toLowerCase());
const pagePath = path.join(pageDir, `${slug}.md`);

// Never overwrite an existing file: it belongs to a manual page, another
// spec, or a different operation whose sanitized name collides with this
// one. Skipping (rather than clobbering) keeps repeated syncs stable.
// Guard against a spec-crafted name escaping reference/, or a stale slug set
// vs. disk. reserveSlug already prevents slug collisions.
if (!isWithin(refDir, pagePath) || fs.existsSync(pagePath)) {
changes.skipped.push({ path: path.relative(refDir, pagePath), operationId: opId });
continue;
Expand All @@ -247,7 +365,6 @@ function syncOneOas(refDir, oasFilename, spec) {
fs.writeFileSync(pagePath, content);

addToOrder(path.join(pageDir, '_order.yaml'), slug);
addToOrder(path.join(refDir, infoTitle, '_order.yaml'), tag);

changes.added.push(path.relative(refDir, pagePath));
}
Expand Down Expand Up @@ -275,11 +392,14 @@ export function syncOas(input) {
const oasFiles = findOasFiles(refDir);
if (oasFiles.length === 0) return null;

// Reference slugs share one flat namespace across every spec, so build the set
// of in-use slugs once and let each spec read/extend it.
const takenSlugs = collectReferenceSlugs(refDir);
const allChanges = [];

for (const { filename, spec } of oasFiles) {
const ops = extractOperations(spec);
const changes = syncOneOas(refDir, filename, spec);
const changes = syncOneOas(refDir, filename, spec, takenSlugs);
allChanges.push({ filename, spec, opCount: ops.size, changes });
}

Expand Down
Loading