diff --git a/CHANGELOG.md b/CHANGELOG.md index 4979ac1..819e0cd 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, 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 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..8fdd375 100644 --- a/core/library.ts +++ b/core/library.ts @@ -242,6 +242,76 @@ 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, "/"); + + // 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, ""); + 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 +379,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 +427,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..2ce9ded 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,204 @@ 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", + ]) { + 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); + // 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("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 {