diff --git a/CHANGELOG.md b/CHANGELOG.md index c1bc4d1..df5545e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes to this project are documented here. The format is based on - **`acquireLock` closes the lock descriptor when `writeFile` throws** (#131). A non-EEXIST write failure previously leaked the file handle until GC. - **Phase continuation no longer burns the full compaction settle timeout** (#129). When a phase run ends without a compaction event, the pending compaction promise is now settled with `false`, so `waitForCompaction` returns immediately instead of waiting out `COMPACTION_SETTLE_TIMEOUT_MS` on every continuation. - **Completion re-validates under the status lock** (#132). `completeValidatedPhase` now re-runs `validatePhaseOutput` inside the atomic status update; a stale PASS whose output changed since the caller's validation refuses to complete (and leaves status untouched) instead of completing a phase on evidence that no longer holds. Validations that never touched a file on disk (no `outputPath`) keep the legacy path, matching the synthetic-validation contract in unit tests. +- **Publish refuses to append one project's spec to another project's history** (#123). Slugs derive from the trailing path segment of `source_repo`, so `acme/whisper` and `openai/whisper` both produce `whisper`. Publishing the second wrote it as `v2` of the first: the entry's version history then spanned two unrelated codebases, and because `index.yaml` carries only the newest version's metadata, the index attributed the whole entry to whichever repo published last. `publishEntry` now compares the incoming `source_repo` against the one recorded on the newest version and fails before writing anything. The comparison is normalized across scheme, embedded credentials (`git@`, `user:token@`), `git@host:path` SCP syntax, a default port, a `www.` prefix, a trailing `.git`, repeated and trailing slashes, and separators, so re-publishing the same repository spelled differently is unaffected. A non-default port still distinguishes two services on one host. Casing is folded for hosts, forge-served repository paths and Windows drive paths, but not for absolute POSIX paths, where `/srv/Repos/tool` and `/srv/repos/tool` are two directories — Pi records the analyzed directory as `source_repo`, so local paths are the common shape on that surface. It is checked ahead of the content-hash branch, because a metadata-only update overwrote the wrong entry just as quietly. Unreadable or malformed recorded metadata skips the check rather than blocking the publish. `force_new_version` does not bypass it. A genuine repository move opts out with `allow_source_repo_change` (`allowSourceRepoChange` in `PublishOptions`). `docs/library-format.md` previously promised auto-suffixing (`-2`, `-3`) for this case, which was never implemented; it now documents the refusal instead. ## [0.16.0] — 2026-08-17 diff --git a/core/library.ts b/core/library.ts index 26fac6d..bd884a0 100644 --- a/core/library.ts +++ b/core/library.ts @@ -242,6 +242,95 @@ export function deriveSlug(sourceRepo: string): string { return RESERVED_SLUGS.has(safe) ? `${safe}-entry` : safe; } +/** + * Reduce a repo reference to a comparable form so that spellings of the same + * repository do not read as different projects. Handles scheme, `git@host:path` + * SCP syntax, a `www.` host prefix, a trailing `.git`, repeated and trailing + * slashes, backslash separators, and case. + * + * This is deliberately conservative: it only collapses spellings that are + * unambiguously the same target. Anything it cannot prove equivalent stays + * distinct, because the caller treats "different" as a hard error. Case is the + * one place that cuts the other way — see the note above the return. + */ +export function normalizeSourceRepo(sourceRepo: string): string { + let s = sourceRepo.trim().replace(/\\/g, "/"); + + // Order matters here. The scheme comes off first so that the SCP branch + // below sees only genuine `host:path` syntax, and the userinfo strip runs + // before either interpretation of a colon. Getting this order wrong makes + // `ssh://git@host/acme/tool` and `https://host/acme/tool` read as two + // different repositories, which would refuse a legitimate re-publish. + const hadScheme = /^[A-Za-z][A-Za-z0-9+.-]*:\/\//.test(s); + s = s.replace(/^[A-Za-z][A-Za-z0-9+.-]*:\/\//, ""); + + // git@, user:token@, oauth2:x-oauth-basic@ ... + s = s.replace(/^[^/@]+@/, ""); + + // SCP syntax (git@github.com:acme/tool) only ever appears without a scheme, + // where the colon separates host from path rather than naming a port. The + // dot requirement keeps a Windows drive letter (C:/repos/tool) out of this + // branch. + if (!hadScheme) s = s.replace(/^([^:/]+\.[^:/]+):(.+)$/, "$1/$2"); + + // A default port for the transports in play is not a distinguishing part of + // the address. Any other port is left alone, since two services on one host + // may genuinely differ by port. + s = s.replace(/^([^/]+):(?:22|80|443)(?=\/|$)/, "$1"); + + s = s.replace(/^www\./i, ""); + s = s.replace(/\.git$/i, ""); + + // Repeated separators name the same location. A leading `//` is the one + // exception: on Windows that is a UNC share (\\server\share), which is not + // the same place as /server/share. + s = s.startsWith("//") ? `/${s.replace(/\/{2,}/g, "/")}` : s.replace(/\/{2,}/g, "/"); + s = s.replace(/\/+$/, ""); + + // Case folding is only safe where the target is case-insensitive. Hosts are, + // as are the repository paths the major forges serve over them, and so are + // Windows drive paths. A POSIX absolute path is not: /srv/Repos/tool and + // /srv/repos/tool are two directories on Linux, and folding them together + // would hide exactly the cross-project collision this comparison exists to + // catch. Pi records the analyzed directory as source_repo, so local paths + // are a common case here rather than a curiosity. + return isCaseSensitivePath(s) ? s : s.toLowerCase(); +} + +/** An absolute POSIX path (or a `~` home reference), where case is significant. */ +function isCaseSensitivePath(s: string): boolean { + return s.startsWith("/") || s === "~" || s.startsWith("~/"); +} + +/** True when two repo references denote the same repository. */ +export function sameSourceRepo(a: string, b: string): boolean { + return normalizeSourceRepo(a) === normalizeSourceRepo(b); +} + +/** + * The `source_repo` recorded on an entry's newest version, or null when it + * cannot be determined (no metadata, unreadable, or malformed). Null means + * "unknown", and callers treat unknown as permission to proceed rather than + * as a mismatch. + */ +async function readRecordedSourceRepo( + libraryRoot: string, + namespace: string | undefined, + slug: string, + version: number, +): Promise { + const metaPath = join(versionDir(libraryRoot, namespace, slug, version), METADATA_FILE); + if (!(await pathExists(metaPath))) return null; + try { + const raw = parseSimpleYaml(await readFile(metaPath, "utf8")); + if (!isPlainObject(raw)) return null; + const recorded = raw.source_repo; + return typeof recorded === "string" && recorded.trim() !== "" ? recorded : null; + } catch { + return null; + } +} + // ─── Path helpers ─────────────────────────────────────────────────────────── function entryRoot(libraryRoot: string, namespace: string | undefined, slug: string): string { @@ -309,6 +398,14 @@ export interface PublishOptions { forceNewVersion?: boolean; /** Skip the regen of index.yaml + INDEX.md (caller will batch). */ skipReindex?: boolean; + /** + * Permit publishing when the target entry's recorded `source_repo` differs + * from the incoming one. Off by default: a mismatch usually means two + * different projects derived the same slug, and continuing would append + * one project's spec to the other's version history. Set this only when + * the repository genuinely moved (rename, org transfer, host change). + */ + allowSourceRepoChange?: boolean; } export interface PublishResult { @@ -349,6 +446,29 @@ export async function publishEntry( const latestVersion = existingVersions.length === 0 ? 0 : existingVersions[existingVersions.length - 1]!; const newSpecHash = sha256(spec); + // Collision guard. Slugs derive from the trailing path segment of the source + // repo, so two unrelated projects (acme/whisper and openai/whisper) collapse + // onto one slug. Without this check the second publish would append its spec + // to the first project's version history, and the index would then report the + // newcomer's source_repo as though it owned every prior version. Checked + // before the idempotence branch below, because a metadata-only update would + // overwrite the wrong entry just as silently. + if (latestVersion > 0 && !opts.allowSourceRepoChange) { + const recorded = await readRecordedSourceRepo(libraryRoot, namespace, input.slug, latestVersion); + if (recorded !== null && !sameSourceRepo(recorded, input.source_repo)) { + const label = namespace ? `${namespace}/${input.slug}` : input.slug; + throw new Error( + `Refusing to publish: entry "${label}" v${latestVersion} records source_repo ` + + `"${recorded}", but this publish carries "${input.source_repo}". Publishing would ` + + `append this spec to a different project's version history. Publish this project ` + + `under a distinct slug to shelve it separately, or — if the repository itself ` + + `moved (rename, org transfer, host change) — re-publish with the source-repo ` + + `change allowed: allow_source_repo_change on codecarto_publish, ` + + `allowSourceRepoChange in PublishOptions.`, + ); + } + } + // Content-hash idempotence: if the latest version's spec matches bytes-for-bytes, // update metadata in place and return without bumping the version. if (latestVersion > 0 && !opts.forceNewVersion) { diff --git a/docs/library-format.md b/docs/library-format.md index e990dd0..103289b 100644 --- a/docs/library-format.md +++ b/docs/library-format.md @@ -133,9 +133,17 @@ codecarto-library/ - Must not equal any reserved name (`latest`, `index`, `entries`). Slugs are derived from the source repo name by default (`my-cool-tool` -from `github.com/acme/my-cool-tool`), with collision-handling via a -trailing `-2`, `-3`, etc. If the namespace differs, the same slug is -permitted across namespaces. +from `github.com/acme/my-cool-tool`). If the namespace differs, the same +slug is permitted across namespaces. + +Derivation uses only the trailing path segment, so two unrelated +repositories can land on one slug (`acme/whisper` and `openai/whisper` +both give `whisper`). CodeCartographer does not auto-suffix these. +Instead, publish refuses when the target entry already records a +different `source_repo`, because the alternative is appending one +project's spec to another project's version history. To shelve the +second project, pass an explicit distinct slug. See "Source repo +conflicts" below. ### Version directories @@ -331,6 +339,44 @@ as a recoverable error: `codecarto library-reindex` repairs it. rename), `latest` is repointed, and `index.yaml` + `INDEX.md` are regenerated. +## Source repo conflicts + +Before either branch above, publish compares the incoming `source_repo` +against the one recorded on the entry's newest version. If they denote +different repositories, publish fails and writes nothing. + +Comparison is normalized, so these are all the same repository and none +of them trip the check: a `https://`, `http://`, `ssh://`, `git://` or +`file://` scheme or none at all, `git@host:owner/name` SCP syntax, a +`www.` host prefix, a trailing `.git`, repeated and trailing slashes, +backslash separators, and any letter casing in a host, a forge-served +repository path, or a Windows drive path. + +Case in an absolute POSIX path is *not* folded, because `/srv/Repos/tool` +and `/srv/repos/tool` are two directories on a case-sensitive filesystem. +Folding them would hide the collision the check exists to catch, and the +Pi surface records the analyzed directory as `source_repo`, so local +paths are the common shape there rather than an edge case. + +Normalization is deliberately conservative: it collapses only spellings +that are unambiguously the same target. Host aliases (`ssh.github.com` +for `github.com`) and provider-specific SSH path layouts (Azure DevOps +`v3/org/proj/name` against the HTTPS `org/proj/_git/name`) are left +distinct, so re-publishing through one of those will need the override +below. + +The check is skipped when the recorded `source_repo` cannot be read at +all (absent, unreadable, or malformed metadata), since there is nothing +to compare. It is *not* skipped by the force-new-version override, which +means "another version of this entry", not "overwrite a different +project". + +A repository that genuinely moved (rename, org transfer, host change) is +the one legitimate case for changing the recorded value. Override it +with `allow_source_repo_change` on `codecarto_publish`, or +`allowSourceRepoChange` in `PublishOptions` when calling the core +directly. + ## Git interaction CodeCartographer is conservative about git operations on the library: diff --git a/mcp-server/server.ts b/mcp-server/server.ts index e990da5..64f9aab 100644 --- a/mcp-server/server.ts +++ b/mcp-server/server.ts @@ -684,7 +684,10 @@ export async function handlePublish(args: Record) { confidentiality, generation, }, - { forceNewVersion: args.force_new_version === true }, + { + forceNewVersion: args.force_new_version === true, + allowSourceRepoChange: args.allow_source_repo_change === true, + }, ); const lines = [ @@ -1244,6 +1247,11 @@ const TOOLS = [ }, }, force_new_version: { type: "boolean" }, + allow_source_repo_change: { + type: "boolean", + description: + "Permit publishing when the target entry already records a different source_repo. Off by default, because a mismatch usually means two projects derived the same slug and the spec would land in the wrong version history. Set only when the repository itself moved.", + }, }, required: ["source_repo", "headline"], }, diff --git a/tests/library.test.mjs b/tests/library.test.mjs index 64f7022..7ac1e43 100644 --- a/tests/library.test.mjs +++ b/tests/library.test.mjs @@ -5,7 +5,9 @@ // content-hash idempotence, force-new-version override, namespacing // (on and off), slug validation, readEntry (latest + specific version), // listEntries filters, reindex from a hand-edited tree, malformed -// metadata graceful fallback, and commitPublish in a non-git directory. +// metadata graceful fallback, commitPublish in a non-git directory, and the +// source_repo collision guard that stops one project's spec landing in +// another's version history. import { test } from "node:test"; import assert from "node:assert/strict"; @@ -34,6 +36,8 @@ const { listEntries, reindex, commitPublish, + normalizeSourceRepo, + sameSourceRepo, } = lib; async function makeLibrary({ namespaced = true, name = "test-library" } = {}) { @@ -508,6 +512,276 @@ test("listEntries regenerates index when it is missing", async () => { // ─── Git ────────────────────────────────────────────────────────────────── +// ─── source_repo collision guard ─────────────────────────────────────────── + +test("normalizeSourceRepo collapses spellings of the same repository", () => { + const canonical = "github.com/acme/tool"; + for (const variant of [ + "https://github.com/acme/tool", + "https://github.com/acme/tool.git", + "https://github.com/acme/tool/", + "http://github.com/acme/tool", + "ssh://github.com/acme/tool.git", + "git@github.com:acme/tool.git", + "https://www.github.com/acme/tool", + "github.com/acme/tool", + "https://GitHub.com/Acme/Tool", + "github.com\\acme\\tool", + " https://github.com/acme/tool.git ", + // Scheme plus credentials. These have to survive the userinfo strip, and + // the scheme has to come off before the SCP branch sees the colon. + "ssh://git@github.com/acme/tool.git", + "ssh://git@github.com:22/acme/tool.git", + "https://git@github.com/acme/tool", + "https://user:token@github.com/acme/tool.git", + "https://oauth2:x-oauth-basic@github.com/acme/tool.git", + "git+https://github.com/acme/tool.git", + "git://github.com/acme/tool.git", + "https://github.com:443/acme/tool", + "http://github.com:80/acme/tool", + // Repeated separators name the same location. + "https://github.com//acme//tool", + "github.com/acme//tool/", + ]) { + assert.equal(normalizeSourceRepo(variant), canonical, `variant: ${variant}`); + } +}); + +test("normalizeSourceRepo folds case only where the target is case-insensitive", () => { + // Hosts and the repository paths the forges serve over them are + // case-insensitive, so these collapse. + assert.equal(sameSourceRepo("https://GitHub.com/Acme/Tool", "https://github.com/acme/tool"), true); + assert.equal(sameSourceRepo("git@GitHub.com:Acme/Tool.git", "github.com/acme/tool"), true); + // Windows drive paths are case-insensitive too. + assert.equal(sameSourceRepo("C:\\Repos\\Tool", "c:/repos/tool"), true); + + // A POSIX absolute path is not. /srv/Repos/tool and /srv/repos/tool are two + // directories on Linux, and folding them would hide the very collision this + // comparison exists to catch. Pi records the analyzed directory as + // source_repo, so this is the common shape there, not a curiosity. + assert.equal(sameSourceRepo("/srv/Repos/tool", "/srv/repos/tool"), false); + assert.equal(sameSourceRepo("~/Work/tool", "~/work/tool"), false); + // Same path, same case, reached via file:// — still one location. + assert.equal(sameSourceRepo("file:///srv/Repos/tool", "/srv/Repos/tool"), true); + // A leading `//` is a UNC share on Windows, not /server/share. + assert.equal(sameSourceRepo("//server/share/tool", "/server/share/tool"), false); +}); + +test("normalizeSourceRepo keeps genuinely different repositories distinct", () => { + assert.equal(sameSourceRepo("https://github.com/openai/whisper", "https://github.com/acme/whisper"), false); + assert.equal(sameSourceRepo("https://github.com/acme/tool", "https://gitlab.com/acme/tool"), false); + assert.equal(sameSourceRepo("https://github.com/acme/tool", "https://github.com/acme/tool-2"), false); + assert.equal(sameSourceRepo("/home/a/tool", "/home/b/tool"), false); + // A non-default port distinguishes two services on one host, so it survives + // normalization even though 22/80/443 do not. + assert.equal(sameSourceRepo("https://git.internal:8080/a/tool", "https://git.internal:9090/a/tool"), false); + // Windows drive letters must not be read as SCP host:path syntax. + assert.equal(sameSourceRepo("C:/repos/tool", "D:/repos/tool"), false); + assert.equal(sameSourceRepo("git@github.com:acme/tool", "git@github.com:acme/other"), false); + assert.equal(sameSourceRepo("https://github.com/acme/tool", "https://github.com/acme"), false); +}); + +test("publish is not refused when the same repo is re-published over SSH", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + // The realistic shape of the false positive: one clone uses HTTPS, a later + // one uses an ssh:// remote carrying a git@ user. Same repository. + await publishEntry(libraryRoot, "# v1\n", sampleInput({ + source_repo: "https://github.com/myorg/hexbridge", + })); + const result = await publishEntry(libraryRoot, "# v2\n", sampleInput({ + source_repo: "ssh://git@github.com/myorg/hexbridge.git", + })); + assert.equal(result.version, 2); + assert.equal(result.isNewVersion, true); + } finally { + await cleanup(); + } +}); + +test("publish refuses a second project that derived the same slug", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + // Both repos end in "whisper", so deriveSlug produces one slug for both. + assert.equal(deriveSlug("https://github.com/openai/whisper"), "whisper"); + assert.equal(deriveSlug("https://github.com/acme/whisper"), "whisper"); + + await publishEntry(libraryRoot, "# spec A\n", sampleInput({ + slug: "whisper", + source_repo: "https://github.com/openai/whisper", + })); + + await assert.rejects( + () => publishEntry(libraryRoot, "# spec B\n", sampleInput({ + slug: "whisper", + source_repo: "https://github.com/acme/whisper", + })), + /Refusing to publish.*openai\/whisper.*acme\/whisper/s, + ); + + // The first project's entry is untouched: still v1, still its own repo. + const entry = await readEntry(libraryRoot, { slug: "whisper", namespace: "james" }); + assert.equal(entry.metadata.version, 1); + assert.equal(entry.metadata.source_repo, "https://github.com/openai/whisper"); + assert.equal(entry.spec, "# spec A\n"); + } finally { + await cleanup(); + } +}); + +test("publish refuses two local directories that differ only in case", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + // The Pi shape of the bug: source_repo is the analyzed directory, and slug + // derives from the same string, so sibling trees collide. Case is the part + // a naive fold would throw away. + await publishEntry(libraryRoot, "# spec A\n", sampleInput({ + slug: "tool", + source_repo: "/srv/Repos/tool", + })); + await assert.rejects( + () => publishEntry(libraryRoot, "# spec B\n", sampleInput({ + slug: "tool", + source_repo: "/srv/repos/tool", + })), + /Refusing to publish/, + ); + } finally { + await cleanup(); + } +}); + +test("the refusal names where the override actually lives", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + await publishEntry(libraryRoot, "# spec A\n", sampleInput({ + slug: "whisper", + source_repo: "https://github.com/openai/whisper", + })); + await assert.rejects( + () => publishEntry(libraryRoot, "# spec B\n", sampleInput({ + slug: "whisper", + source_repo: "https://github.com/acme/whisper", + })), + (err) => { + // Both spellings, so the reader can act on whichever surface they are + // on. The message must not tell them to "pass" an option the caller + // may not expose: /codecarto-publish in Pi takes no arguments. + assert.match(err.message, /allow_source_repo_change/); + assert.match(err.message, /allowSourceRepoChange/); + assert.doesNotMatch(err.message, /Pass an explicit/); + return true; + }, + ); + } finally { + await cleanup(); + } +}); + +test("the collision guard also covers the metadata-only path", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + const spec = "# identical bytes\n"; + await publishEntry(libraryRoot, spec, sampleInput({ + slug: "whisper", + source_repo: "https://github.com/openai/whisper", + })); + + // Identical spec bytes would otherwise take the in-place metadata update + // branch and silently rewrite the other project's source_repo. + await assert.rejects( + () => publishEntry(libraryRoot, spec, sampleInput({ + slug: "whisper", + source_repo: "https://github.com/acme/whisper", + headline: "A different project entirely.", + })), + /Refusing to publish/, + ); + + const entry = await readEntry(libraryRoot, { slug: "whisper", namespace: "james" }); + assert.equal(entry.metadata.source_repo, "https://github.com/openai/whisper"); + assert.equal(entry.metadata.headline, sampleInput().headline); + } finally { + await cleanup(); + } +}); + +test("publish accepts the same repository spelled differently", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + await publishEntry(libraryRoot, "# v1\n", sampleInput({ + source_repo: "https://github.com/myorg/hexbridge", + })); + // A later run reporting the SCP form plus .git must not read as a new project. + const result = await publishEntry(libraryRoot, "# v2\n", sampleInput({ + source_repo: "git@github.com:myorg/hexbridge.git", + })); + assert.equal(result.version, 2); + assert.equal(result.isNewVersion, true); + } finally { + await cleanup(); + } +}); + +test("allowSourceRepoChange permits a genuine repository move", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + await publishEntry(libraryRoot, "# v1\n", sampleInput({ + source_repo: "https://github.com/oldorg/hexbridge", + })); + const result = await publishEntry( + libraryRoot, + "# v2\n", + sampleInput({ source_repo: "https://github.com/neworg/hexbridge" }), + { allowSourceRepoChange: true }, + ); + assert.equal(result.version, 2); + const entry = await readEntry(libraryRoot, { slug: "hexbridge", namespace: "james" }); + assert.equal(entry.metadata.source_repo, "https://github.com/neworg/hexbridge"); + } finally { + await cleanup(); + } +}); + +test("the collision guard stays out of the way when metadata is unreadable", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + await publishEntry(libraryRoot, "# v1\n", sampleInput()); + // Corrupt the recorded metadata: source_repo becomes undeterminable, so the + // guard has nothing to compare and must not block the publish. + const metaPath = join(libraryRoot, ENTRIES_DIR, "james", "hexbridge", "v1", METADATA_FILE); + await writeFile(metaPath, ":::not valid yaml:::\n", "utf8"); + + const result = await publishEntry(libraryRoot, "# v2\n", sampleInput({ + source_repo: "https://github.com/someoneelse/hexbridge", + })); + assert.equal(result.version, 2); + } finally { + await cleanup(); + } +}); + +test("forceNewVersion does not bypass the collision guard", async () => { + const { libraryRoot, cleanup } = await makeLibrary(); + try { + await publishEntry(libraryRoot, "# spec A\n", sampleInput({ + slug: "whisper", + source_repo: "https://github.com/openai/whisper", + })); + await assert.rejects( + () => publishEntry( + libraryRoot, + "# spec B\n", + sampleInput({ slug: "whisper", source_repo: "https://github.com/acme/whisper" }), + { forceNewVersion: true }, + ), + /Refusing to publish/, + ); + } finally { + await cleanup(); + } +}); + test("commitPublish returns not-a-git-repo when .git is missing", async () => { const { libraryRoot, cleanup } = await makeLibrary(); try {