Skip to content

Commit 8bfd39a

Browse files
ralphstodomingoclaude
ralphstodomingo
and
claude
committed
fix: key only public-host remotes, so a self-hosted X.git stays apart from X
A cached remote is either the service's value or the one git printed, and `x.git` names a different repo in each. github.com, gitlab.com and bitbucket.org don't hold `x` and `x.git` side by side, so only their remotes are keyed, dropping every trailing `.git` in one pass; other remotes compare exactly. A self-hosted server can hold both, and the reaper could otherwise remove one project's block for the other's archived record. The drift check also accepts the service's canonical form of the local remote, which is what it stores off the public hosts. A record whose remote key isn't a string matches no project, as before. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01LmLXkY5NUqPL7nH2exe9zb
1 parent 3a9af3c commit 8bfd39a

4 files changed

Lines changed: 121 additions & 63 deletions

File tree

‎packages/opencode/src/altimate/workspace/memory-sync.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,14 @@ function mirrorIdentityOf(record: CloudMemoryRecord, binding: CachedBinding | nu
429429
if (!blockId || !scope) return undefined
430430
if (scope === "project") {
431431
if (String(m.datamate_id ?? "") !== String(binding?.datamateId ?? "")) return undefined
432-
const recordProject = recordProjectIdentity(m)
433-
if (recordProject?.value && binding && !sameProjectIdentity(recordProject, projectIdentityFor(binding)))
432+
// The record's key as written. One that isn't a string names no project of
433+
// ours, so it never matches.
434+
const recordKey = m.repo_remote ?? m.project_path
435+
if (
436+
recordKey &&
437+
binding &&
438+
!(typeof recordKey === "string" && sameProjectIdentity(recordProjectIdentity(m), projectIdentityFor(binding)))
439+
)
434440
return undefined
435441
}
436442
return `${scope}:${blockId}`

‎packages/opencode/src/altimate/workspace/remote-identity.ts‎

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -148,67 +148,68 @@ function authorityHost(authority: string): string | null {
148148
return hostname.toLowerCase()
149149
}
150150

151-
/** `https://<host>/<path>` for a repo on github.com, gitlab.com or
152-
* bitbucket.org, whatever URL form `canonical` is in; otherwise `canonical`
153-
* unchanged. `canonical` is a `canonicalRepoRemote` value: the service's
154-
* `_unify_public_host`. */
155-
export function unifyPublicHost(canonical: string): string {
151+
/** The host and path of a repo on github.com, gitlab.com or bitbucket.org,
152+
* whatever URL form `canonical` is in, or undefined where the service's rule
153+
* leaves the value as it is. `canonical` is a `canonicalRepoRemote` value. */
154+
function publicHostRepo(canonical: string): { host: string; path: string } | undefined {
156155
let [scheme, sep, rest] = partition(canonical, "://")
157156
let host: string
158157
let path: string
159158
if (sep) {
160159
scheme = scheme.toLowerCase()
161-
if (!REMOTE_SCHEMES.has(scheme)) return canonical
160+
if (!REMOTE_SCHEMES.has(scheme)) return undefined
162161
let authority: string
163162
;[authority, , path] = partition(rest, "/")
164163
const parsed = authorityHost(authority)
165-
if (parsed === null) return canonical
164+
if (parsed === null) return undefined
166165
host = parsed
167166
} else {
168167
// scp-like `host:path` with no user. Git reads a value as scp-like only
169168
// when no `/` precedes the first `:`; anything else is a local path.
170169
const [authority, colon, after] = partition(canonical, ":")
171-
if (!colon || authority.includes("/")) return canonical
170+
if (!colon || authority.includes("/")) return undefined
172171
scheme = "ssh"
173172
host = rpartition(authority, "@")[2].toLowerCase()
174173
path = after
175174
}
176175
if (SSH_SCHEMES.has(scheme)) host = SSH_HOST_ALIASES[host] ?? host
177176
else if (WEB_SCHEMES.has(scheme)) host = WEB_HOST_ALIASES[host] ?? host
178-
if (!CASE_INSENSITIVE_HOSTS.has(host) || !isAscii(path)) return canonical
177+
if (!CASE_INSENSITIVE_HOSTS.has(host) || !isAscii(path)) return undefined
179178
path = stripChar(path, "/", "both").toLowerCase()
180-
const unified = `https://${host}/${path}`
181-
if (!path || new TextEncoder().encode(unified).length > IDENTIFIER_MAX_LENGTH) return canonical
182-
return unified
179+
if (!path || new TextEncoder().encode(`https://${host}/${path}`).length > IDENTIFIER_MAX_LENGTH) return undefined
180+
return { host, path }
183181
}
184182

185-
/** The value two remotes of one repo share: the service's unified value of the
186-
* remote's canonical form, applied until it stops changing.
183+
/** `https://<host>/<path>` for a repo on github.com, gitlab.com or
184+
* bitbucket.org, whatever URL form `canonical` is in; otherwise `canonical`
185+
* unchanged: the service's `_unify_public_host`. */
186+
export function unifyPublicHost(canonical: string): string {
187+
const repo = publicHostRepo(canonical)
188+
return repo ? `https://${repo.host}/${repo.path}` : canonical
189+
}
190+
191+
/** The key every form of a repo on github.com, gitlab.com or bitbucket.org
192+
* shares, or undefined for any other remote, which compares exactly.
187193
*
188-
* Inputs come in three shapes: a remote as git prints it, a value the service
189-
* stored (canonical, or unified), and a raw remote the client cached before
190-
* the service answered. One pass maps all of them to the service's value,
191-
* except where a pass is not idempotent: canonicalization takes one trailing
192-
* `.git` off a stored value that still ends in one. Repeating reaches the same
193-
* value from every shape. The cost is that a repo whose name ends in `.git`
194-
* compares equal to the one without it; the service already stores several
195-
* forms of such a repo under the shorter name. */
196-
export function repoRemoteKey(value: string): string {
197-
// Only the first pass can lengthen the value (`git@` to `ssh://git@`, `http`
198-
// to `https`). Every later pass that changes it removes at least one
199-
// character, so the value settles within this bound.
200-
const bound = value.length + 16
201-
let key = value
202-
for (let i = 0; i < bound; i++) {
203-
const next = unifyPublicHost(canonicalRepoRemote(key))
204-
if (next === key) break
205-
key = next
206-
}
207-
return key
194+
* A cached value is either the service's or, when the service gave none (a
195+
* workspace pinned from the IDE), the remote as git printed it. Read as git
196+
* prints it, a remote ending in `.git` names the repo without it; as the
197+
* service stores it, the repo with it. These hosts don't let the two exist
198+
* side by side (GitHub and GitLab refuse a name ending in `.git`, and Bitbucket
199+
* resolves `x.git` to `x`), so the key drops every trailing `.git`. Other hosts
200+
* may, so their remotes get no key. */
201+
export function publicRepoKey(value: string): string | undefined {
202+
const repo = publicHostRepo(canonicalRepoRemote(value))
203+
if (!repo) return undefined
204+
const path = repo.path.replace(/(?:\.git|\/)+$/, "") || repo.path
205+
return `${repo.host}/${path}`
208206
}
209207

208+
/** Do two cached remotes name the same repo? */
210209
export function sameRepoRemote(a: string, b: string): boolean {
211-
return a === b || repoRemoteKey(a) === repoRemoteKey(b)
210+
if (a === b) return true
211+
const key = publicRepoKey(a)
212+
return key !== undefined && key === publicRepoKey(b)
212213
}
213214

214215
/** A project's identity as the binding system records it: its git remote when
@@ -224,7 +225,7 @@ export function projectIdentity(remote: unknown, projectPath: unknown): ProjectI
224225
}
225226

226227
/** Do two identities name the same project? Remotes compare by
227-
* `repoRemoteKey`; paths compare exactly, since a path is never a remote and
228+
* `sameRepoRemote`; paths compare exactly, since a path is never a remote and
228229
* must not be rewritten (`/srv/x.git`, `C:\x.git`); a remote never equals a
229230
* path. A missing identity matches nothing. */
230231
export function sameProjectIdentity(a: ProjectIdentity | undefined, b: ProjectIdentity | undefined): boolean {
@@ -237,5 +238,8 @@ export function sameProjectIdentity(a: ProjectIdentity | undefined, b: ProjectId
237238
* service or the local cache, and `current` is this project's, with the remote
238239
* as git prints it (`[email protected]:acme/x.git`). */
239240
export function sameIdentifier(kind: "remote" | "path", stored: string, current: string): boolean {
240-
return kind === "remote" ? sameRepoRemote(stored, current) : stored === current
241+
if (kind === "path") return stored === current
242+
// The service found the binding by the canonical form of `current`, and off
243+
// the public hosts that form is the value it stores.
244+
return sameRepoRemote(stored, current) || canonicalRepoRemote(current) === stored
241245
}

‎packages/opencode/test/altimate/workspace/memory-sync.test.ts‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,8 @@ describe("toBlock", () => {
959959
kind: "remote",
960960
value: own,
961961
})
962-
expect(b?.origin).toBeUndefined()
962+
expect(b).not.toBeNull()
963+
expect(b!.origin).toBeUndefined()
963964
})
964965

965966
test("a record written under another URL form of this repo is not labelled", () => {
@@ -968,14 +969,16 @@ describe("toBlock", () => {
968969
const project = (repo_remote: string) =>
969970
record({ source: MIRROR_SOURCE, block_id: "x", block_scope: "project", repo_remote })
970971
const https = { kind: "remote" as const, value: "https://github.com/acme/analytics" }
971-
expect(toBlock(project("ssh://[email protected]/acme/analytics"), https)?.origin).toBeUndefined()
972-
expect(toBlock(project("[email protected]:Acme/Analytics.git"), https)?.origin).toBeUndefined()
973-
expect(
974-
toBlock(project("https://github.com/acme/analytics"), {
975-
kind: "remote",
976-
value: "ssh://[email protected]/acme/analytics",
977-
})?.origin,
978-
).toBeUndefined()
972+
const ssh = { kind: "remote" as const, value: "ssh://[email protected]/acme/analytics" }
973+
for (const [repoRemote, own] of [
974+
["ssh://[email protected]/acme/analytics", https],
975+
["[email protected]:Acme/Analytics.git", https],
976+
["https://github.com/acme/analytics", ssh],
977+
] as const) {
978+
const b = toBlock(project(repoRemote), own)
979+
expect(b).not.toBeNull()
980+
expect(b!.origin).toBeUndefined()
981+
}
979982
})
980983

981984
test("self-hosted forms, other repos and paths still label a sibling", () => {
@@ -1186,6 +1189,21 @@ describe("reaping blocks archived elsewhere", () => {
11861189
expect(idsOf(removed)).toEqual(["ours"])
11871190
})
11881191

1192+
test("on a self-hosted host, an archived record for repo X.git is not reaped for repo X", async () => {
1193+
// Such a server can hold both repos; only the public hosts' forms are one repo.
1194+
syncInternals.resolveBinding = async () => ({ ...BINDING, repoRemote: "https://git.acme.internal/acme/X" }) as any
1195+
const removed = seeThrough({ shared: { updated: "2026-05-01T00:00:00.000Z" } })
1196+
listResponse = [
1197+
archived("shared", {
1198+
block_scope: "project",
1199+
datamate_id: String(BINDING.datamateId),
1200+
repo_remote: "https://git.acme.internal/acme/X.git",
1201+
}),
1202+
]
1203+
await refresh(`${SES}-reap-5e`)
1204+
expect(idsOf(removed)).toEqual([])
1205+
})
1206+
11891207
test("an archived record for THIS project under an older URL form is reaped", async () => {
11901208
// The binding now holds the repo's https value; the record was archived
11911209
// when it still held the SSH form.
@@ -2193,6 +2211,23 @@ describe("a repo's URL forms", () => {
21932211
expect(callsTo("/datamates/memory/", "POST").length).toBe(1)
21942212
})
21952213

2214+
test("a record whose remote key isn't a string is not this project's", async () => {
2215+
boundTo(HTTPS)
2216+
const record = projectRecord("mem-numeric", "forms-numeric", HTTPS)
2217+
listResponse = [{ ...record, metadata: { ...record.metadata, repo_remote: 42 } }]
2218+
await mirrorBlock(edited("forms-numeric"))
2219+
expect(callsTo("/datamates/memory/mem-numeric", "PATCH").length).toBe(0)
2220+
expect(callsTo("/datamates/memory/", "POST").length).toBe(1)
2221+
})
2222+
2223+
test("a self-hosted record under the binding's own value is updated", async () => {
2224+
boundTo("https://git.acme.internal/data/analytics")
2225+
listResponse = [projectRecord("mem-self-same", "forms-self-same", "https://git.acme.internal/data/analytics")]
2226+
await mirrorBlock(edited("forms-self-same"))
2227+
expect(callsTo("/datamates/memory/", "POST").length).toBe(0)
2228+
expect(callsTo("/datamates/memory/mem-self-same", "PATCH").length).toBe(1)
2229+
})
2230+
21962231
test("self-hosted SSH and https forms stay separate projects", async () => {
21972232
boundTo("https://git.acme.internal/data/analytics")
21982233
listResponse = [projectRecord("mem-selfhosted", "forms-self", "ssh://[email protected]/data/analytics")]

‎packages/opencode/test/altimate/workspace/remote-identity.test.ts‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import vectors from "./fixtures/remote-identity-vectors.json"
1010
import {
1111
canonicalRepoRemote,
1212
projectIdentity,
13-
repoRemoteKey,
13+
publicRepoKey,
1414
sameIdentifier,
1515
sameProjectIdentity,
1616
sameRepoRemote,
@@ -50,7 +50,7 @@ describe("parity with the workspace service", () => {
5050
})
5151
})
5252

53-
describe("repoRemoteKey", () => {
53+
describe("sameRepoRemote", () => {
5454
const X = "https://github.com/acme/x"
5555

5656
test("every form of a public-host repo shares one key", () => {
@@ -62,30 +62,31 @@ describe("repoRemoteKey", () => {
6262
"ssh://[email protected]:443/acme/x",
6363
"github.com:acme/x",
6464
X,
65-
])
66-
expect(repoRemoteKey(form)).toBe(X)
65+
]) {
66+
expect(publicRepoKey(form)).toBe("github.com/acme/x")
67+
expect(sameRepoRemote(form, X)).toBe(true)
68+
}
6769
})
6870

69-
test("a stored value ending in .git reaches the key its raw remote does", () => {
71+
test("a stored value ending in .git matches the remote git prints for it", () => {
7072
// The service stores `https://github.com/acme/x.git` for a remote ending
71-
// in `x.git.git`; one canonicalization pass on each side disagrees.
73+
// in `x.git.git`.
7274
expect(sameRepoRemote("https://github.com/acme/x.git", "[email protected]:acme/x.git.git")).toBe(true)
73-
})
74-
75-
test("the key settles however many .git suffixes a remote carries", () => {
7675
for (const n of [1, 33, 200]) {
77-
const raw = "https://github.com/acme/x" + ".git".repeat(n)
78-
expect(repoRemoteKey(raw)).toBe("https://github.com/acme/x")
76+
const raw = X + ".git".repeat(n)
7977
expect(sameRepoRemote(raw, canonicalRepoRemote(raw))).toBe(true)
78+
expect(publicRepoKey(raw)).toBe("github.com/acme/x")
8079
}
81-
const selfHosted = "ssh://[email protected]/data/x" + ".git".repeat(200)
82-
expect(repoRemoteKey(selfHosted)).toBe("ssh://[email protected]/data/x")
8380
})
8481

85-
test("self-hosted and GitHub Enterprise forms stay distinct", () => {
82+
test("other hosts compare exactly, so .git keeps two repos apart", () => {
83+
// A self-hosted server can hold repos `X` and `X.git` side by side. The
84+
// service stores `.../X.git` for the second and `.../X` for the first.
85+
expect(publicRepoKey("https://git.acme.internal/acme/X.git")).toBeUndefined()
86+
expect(sameRepoRemote("https://git.acme.internal/acme/X", "https://git.acme.internal/acme/X")).toBe(true)
87+
expect(sameRepoRemote("https://git.acme.internal/acme/X.git", "https://git.acme.internal/acme/X")).toBe(false)
8688
expect(sameRepoRemote("ssh://[email protected]/data/x", "https://git.acme.internal/data/x")).toBe(false)
8789
expect(sameRepoRemote("[email protected]:team/repo.git", "https://github.acme.com/team/repo")).toBe(false)
88-
// Case still matters off the public hosts.
8990
expect(sameRepoRemote("https://gitea.example.com/Foo/Bar", "https://gitea.example.com/foo/bar")).toBe(false)
9091
})
9192

@@ -137,6 +138,18 @@ describe("sameIdentifier (the link dialog's drift check)", () => {
137138
)
138139
})
139140

141+
test("off the public hosts, the service's canonical form of the local remote is no drift", () => {
142+
expect(sameIdentifier("remote", "ssh://[email protected]/data/x", "[email protected]:data/x.git")).toBe(
143+
true,
144+
)
145+
expect(sameIdentifier("remote", "https://git.acme.internal/data/X", "https://git.acme.internal/data/X.git/")).toBe(
146+
true,
147+
)
148+
expect(sameIdentifier("remote", "https://git.acme.internal/data/X", "https://git.acme.internal/data/Y.git")).toBe(
149+
false,
150+
)
151+
})
152+
140153
test("paths compare exactly", () => {
141154
expect(sameIdentifier("path", "/work/x", "/work/x")).toBe(true)
142155
expect(sameIdentifier("path", "/work/x.git", "/work/x")).toBe(false)

0 commit comments

Comments
 (0)