From f4995691e960d4d92f89c949ae575ff5102fa130 Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 21 Aug 2026 01:21:48 -0400 Subject: [PATCH 1/3] fix: refuse a publish that would append to another project's history 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 since buildIndexEntry reads only the newest version's metadata, index.yaml attributed every prior version to whichever repo published last. Nothing surfaced the collision at publish time or afterwards. publishEntry now reads the source_repo recorded on the newest version and fails before writing when it denotes a different repository. Comparison is normalized so that re-publishing one repository spelled another way is unaffected: scheme or none, git@host:path SCP syntax, a www. prefix, a trailing .git, trailing slashes, backslash separators, and casing all collapse to the same form. Only spellings that are unambiguously the same target collapse, since the caller treats "different" as fatal. Three details worth noting for review: - The check sits ahead of the content-hash branch. Identical spec bytes take the metadata-only path, which overwrote the other project's source_repo and headline in place, so guarding only the new-version path would have left the quieter half of the bug. - forceNewVersion does not bypass it. That option means "another version of this entry", not "overwrite a different project". - Unreadable or malformed recorded metadata skips the check. There is nothing to compare, and refusing on unknown would turn a corrupt v1 into an unpublishable entry. A genuine repository move is the one legitimate reason to change the recorded value, so allowSourceRepoChange opts out, exposed on the MCP surface as allow_source_repo_change. docs/library-format.md promised auto-suffixing (-2, -3) for this case. That was never implemented, and silent suffixing is its own surprise for an agent-driven tool, so the doc now describes the refusal and the override. Eight tests. Three fail without the guard: the two-project collision, the metadata-only path, and forceNewVersion. The rest pin the normalization equivalences, keep genuinely distinct repos distinct, and hold the false-positive and malformed-metadata paths open. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 + core/library.ts | 81 ++++++++++++++++++++ docs/library-format.md | 38 +++++++++- mcp-server/server.ts | 10 ++- tests/library.test.mjs | 168 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 296 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4979ac1..4e87489 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] +### Fixed + +- **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, `git@host:path` SCP syntax, a `www.` prefix, a trailing `.git`, trailing slashes, separators and casing, so re-publishing the same repository spelled differently is unaffected. 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 The field-test round. Immediately after 0.15.0 shipped, the same 7-phase deepseek-harness analysis was re-run on a fresh worktree through the published binary — this time with the driving chat as orchestrator — and the run's own gaps became this release (#111–#114): the very first completion appended decision rows without their promised heading, both full runs ended with no dashboard ever rendered, the analysis→publish→synthesis library loop was unreachable from any served text, and the terminal completion message named nothing actionable while skills, amendments, a publishable spec, and the usage log all sat unused. diff --git a/core/library.ts b/core/library.ts index 26fac6d..6bd3e02 100644 --- a/core/library.ts +++ b/core/library.ts @@ -242,6 +242,58 @@ 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`, 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. + */ +export function normalizeSourceRepo(sourceRepo: string): string { + let s = sourceRepo.trim().replace(/\\/g, "/"); + // git@github.com:acme/tool -> github.com/acme/tool + const scp = /^[A-Za-z0-9._-]+@([^:/]+):(.+)$/.exec(s); + if (scp) s = `${scp[1]}/${scp[2]}`; + s = s.replace(/^[A-Za-z][A-Za-z0-9+.-]*:\/\//, ""); + s = s.replace(/^www\./i, ""); + s = s.replace(/\/+$/, ""); + s = s.replace(/\.git$/i, ""); + s = s.replace(/\/+$/, ""); + return s.toLowerCase(); +} + +/** 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 +361,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 +409,27 @@ 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. Pass an explicit, ` + + `distinct slug to shelve it separately, or set allowSourceRepoChange if the ` + + `repository itself moved.`, + ); + } + } + // 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..d090125 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,30 @@ 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://` or `git://` +scheme or none at all, `git@host:owner/name` SCP syntax, a `www.` host +prefix, a trailing `.git`, trailing slashes, backslash separators, and +any letter casing. + +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 8cba77d..2bd0797 100644 --- a/mcp-server/server.ts +++ b/mcp-server/server.ts @@ -656,7 +656,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 = [ @@ -1095,6 +1098,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..a85b56a 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,168 @@ 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 ", + ]) { + assert.equal(normalizeSourceRepo(variant), canonical, `variant: ${variant}`); + } +}); + +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); +}); + +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("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 { From 3d021af1828f33c6d8ba565ba773713e5a47f4e6 Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 21 Aug 2026 02:04:19 -0400 Subject: [PATCH 2/3] fix: normalize source_repo in the right order so SSH remotes still match Self-review of #127 found the guard refusing a legitimate re-publish, which is the failure direction the guard was supposed to avoid. normalizeSourceRepo ran the SCP branch before stripping the scheme and never stripped a userinfo prefix, so four common spellings of one repository each reduced to something different: ssh://git@github.com/acme/tool.git -> git@github.com/acme/tool ssh://git@github.com:22/acme/tool.git -> git@github.com:22/acme/tool https://git@github.com/acme/tool -> git@github.com/acme/tool https://user:token@github.com/acme/tool -> user:token@github.com/acme/tool Publishing once from an HTTPS clone and later from an SSH clone of the same repo was therefore refused as a different project. Reordered: scheme off first, then userinfo, then the SCP branch, which now runs only when there was no scheme, since that is the only place a colon means host:path rather than a port. Default ports 22, 80 and 443 are dropped; any other port is kept, because two services on one host can differ by port alone. The SCP branch also now requires a dotted host, so a Windows drive letter is not read as host:path. The original tests missed all of this: every URL form they used had either no scheme or no credentials. Widened to nine more equivalent spellings, and the distinctness assertions now pin non-default ports, drive letters, and a same-host different-path pair. Two tests fail against the previous normalization. 351 tests, suite green. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 +- core/library.ts | 26 ++++++++++++++++++++++---- tests/library.test.mjs | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e87489..819e0cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project are documented here. The format is based on ### Fixed -- **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, `git@host:path` SCP syntax, a `www.` prefix, a trailing `.git`, trailing slashes, separators and casing, so re-publishing the same repository spelled differently is unaffected. 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. +- **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`, trailing slashes, separators and casing, so re-publishing the same repository spelled differently is unaffected. A non-default port still distinguishes two services on one host. 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 6bd3e02..8fdd375 100644 --- a/core/library.ts +++ b/core/library.ts @@ -254,12 +254,30 @@ export function deriveSlug(sourceRepo: string): string { */ export function normalizeSourceRepo(sourceRepo: string): string { let s = sourceRepo.trim().replace(/\\/g, "/"); - // git@github.com:acme/tool -> github.com/acme/tool - const scp = /^[A-Za-z0-9._-]+@([^:/]+):(.+)$/.exec(s); - if (scp) s = `${scp[1]}/${scp[2]}`; + + // 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(/\/+$/, ""); s = s.replace(/\.git$/i, ""); s = s.replace(/\/+$/, ""); return s.toLowerCase(); diff --git a/tests/library.test.mjs b/tests/library.test.mjs index a85b56a..2ce9ded 100644 --- a/tests/library.test.mjs +++ b/tests/library.test.mjs @@ -528,6 +528,17 @@ test("normalizeSourceRepo collapses spellings of the same repository", () => { "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", ]) { assert.equal(normalizeSourceRepo(variant), canonical, `variant: ${variant}`); } @@ -538,6 +549,31 @@ test("normalizeSourceRepo keeps genuinely different repositories distinct", () = 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 () => { From ffc21c3252c25553b4a6957d6d6b75b3ae0011d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 08:58:28 +0000 Subject: [PATCH 3/3] fix: tighten source_repo comparison and drop remedies the caller may not have MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the collision guard. Case was folded across the whole reference, including absolute POSIX paths. /srv/Repos/tool and /srv/repos/tool are two directories on a case-sensitive filesystem, so folding them let a genuine two-project collision through the guard silently — the exact failure it exists to catch. Case is still folded for hosts, for the repository paths the forges serve over them, and for Windows drive paths, all of which are case-insensitive. This matters more than it reads: Pi records the analyzed directory as source_repo, so local paths are the common shape on that surface, not a curiosity. Repeated separators now collapse, so github.com//acme//tool no longer reads as a different repository from github.com/acme/tool. A leading // is preserved, since on Windows that is a UNC share rather than /server/share. The refusal message told the reader to "pass an explicit, distinct slug" or "set allowSourceRepoChange". Neither is reachable from Pi: /codecarto-publish takes no arguments and the override is MCP-only. It now describes the two remedies and names where each one lives, so the message is honest on whichever surface raised it. Closing the gap on Pi is separate UX work. Four tests, all failing against the previous normalization: the extended equivalence set, the case rules in both directions, an end-to-end refusal of two local directories differing only in case, and a check that the message names both override spellings and no longer prescribes one the caller may not expose. 354 total, suite green. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01LmPQsx1esS4uHDzVbbKVZg --- CHANGELOG.md | 2 +- core/library.ts | 35 ++++++++++++++++---- docs/library-format.md | 22 ++++++++++--- tests/library.test.mjs | 72 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 819e0cd..7e3bca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project are documented here. The format is based on ### Fixed -- **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`, trailing slashes, separators and casing, so re-publishing the same repository spelled differently is unaffected. A non-default port still distinguishes two services on one host. 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. +- **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 8fdd375..bd884a0 100644 --- a/core/library.ts +++ b/core/library.ts @@ -245,12 +245,13 @@ export function deriveSlug(sourceRepo: string): string { /** * 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`, trailing slashes, - * backslash separators, and case. + * 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. + * 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, "/"); @@ -279,8 +280,26 @@ export function normalizeSourceRepo(sourceRepo: string): string { 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(/\/+$/, ""); - return s.toLowerCase(); + + // 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. */ @@ -441,9 +460,11 @@ export async function publishEntry( 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. Pass an explicit, ` + - `distinct slug to shelve it separately, or set allowSourceRepoChange if the ` + - `repository itself moved.`, + `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.`, ); } } diff --git a/docs/library-format.md b/docs/library-format.md index d090125..103289b 100644 --- a/docs/library-format.md +++ b/docs/library-format.md @@ -346,10 +346,24 @@ 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://` or `git://` -scheme or none at all, `git@host:owner/name` SCP syntax, a `www.` host -prefix, a trailing `.git`, trailing slashes, backslash separators, and -any letter casing. +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 diff --git a/tests/library.test.mjs b/tests/library.test.mjs index 2ce9ded..7ac1e43 100644 --- a/tests/library.test.mjs +++ b/tests/library.test.mjs @@ -539,11 +539,34 @@ test("normalizeSourceRepo collapses spellings of the same repository", () => { "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); @@ -606,6 +629,55 @@ test("publish refuses a second project that derived the same slug", async () => } }); +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 {