diff --git a/.changeset/git-pr-create.md b/.changeset/git-pr-create.md index 6bf55b1..7d0a868 100644 --- a/.changeset/git-pr-create.md +++ b/.changeset/git-pr-create.md @@ -4,4 +4,6 @@ --- Add `git-remote-atproto pr create` to publish a pull request record from the -current checkout. +current checkout. The record lives at the key the branch's ref derives, and +creating refuses when a record already holds that key, so a second create +never clobbers the words already there. diff --git a/.changeset/git-pr-update.md b/.changeset/git-pr-update.md index 7748ef5..fa8d971 100644 --- a/.changeset/git-pr-update.md +++ b/.changeset/git-pr-update.md @@ -4,6 +4,12 @@ --- Add `git-remote-atproto pr update` to refresh the current branch's pull -request record with a new title, description and draft word, and the tip -commit at HEAD. The command appends a new record, which the reader takes as -the newest word for the branch. \ No newline at end of file +request with a new title, description and draft word, and the tip commit at +HEAD. The record lives at the key the branch derives, so an update rewrites +it in place rather than appending, with a compare-and-swap against the +version read. + +A pull request is one record at `dev.pdsjs.git.pull/:` (the +record key the branch's ref derives) rather than a newest-wins stack of +appended records. The reader, the CLI and the UI all agree on that key, so +an update is always the current word. \ No newline at end of file diff --git a/packages/git-ui/src/lib/collab.js b/packages/git-ui/src/lib/collab.js index ac3a7b1..d3719dd 100644 --- a/packages/git-ui/src/lib/collab.js +++ b/packages/git-ui/src/lib/collab.js @@ -14,7 +14,6 @@ import { byPull, fileSummary, fixesOf, - latestIntents, mergedRun, place, pullKey, @@ -28,7 +27,6 @@ export { byPull, fileSummary, fixesOf, - latestIntents, mergedRun, place, pullKey, diff --git a/packages/git-ui/src/lib/publish.js b/packages/git-ui/src/lib/publish.js index 39b4c2f..4d097b2 100644 --- a/packages/git-ui/src/lib/publish.js +++ b/packages/git-ui/src/lib/publish.js @@ -6,6 +6,7 @@ // see. The subject names the exact repository record version in view, so a // statement cannot be re-attributed to a later push. +import { pullRkey } from '@pdsjs/git/pull-requests'; import { authedFetch } from '#/lib/oauth.js'; import { repoRow } from './git.js'; @@ -95,6 +96,39 @@ async function createRecord(session, collection, record) { return response.json(); } +/** + * Write one record at a known key, the same way the CLI's pr create and pr + * update do. A compare-and-swap value guards a concurrent edit. + * @param {import('#/lib/oauth.js').Session} session + * @param {string} collection + * @param {string} rkey + * @param {Record} record + * @param {string|null} [swapRecord] + * @returns {Promise<{uri: string, cid: string}>} + */ +async function putRecord(session, collection, rkey, record, swapRecord) { + const response = await authedFetch( + session, + `${session.pds}/xrpc/com.atproto.repo.putRecord`, + { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + repo: session.did, + collection, + rkey, + record, + swapRecord, + }), + }, + ); + if (!response.ok) { + const body = await response.json().catch(() => ({})); + throw new Error(body.message || `The write failed (${response.status}).`); + } + return response.json(); +} + /** * Publish a review of the pull request in view. * @param {import('#/lib/oauth.js').Session} session @@ -138,9 +172,10 @@ export function publishReview(session, about) { export function publishPull(session, about) { const row = repoRow(about.repo); if (!row) throw new Error('The repository record is not in hand.'); - return createRecord( + return putRecord( session, PULL_COLLECTION, + pullRkey(about.repo, about.ref), buildPull({ ...about, subject: row }), ); } diff --git a/packages/git/src/cli.js b/packages/git/src/cli.js index 671cd3f..4c2e30d 100755 --- a/packages/git/src/cli.js +++ b/packages/git/src/cli.js @@ -18,6 +18,7 @@ import { AtprotoRemoteHelper } from './helper.js'; import { resolveRepoLocation } from './identity.js'; import { GIT_PULL_COLLECTION, GIT_REPO_COLLECTION } from './lexicon.js'; import { oauthLogin } from './oauth.js'; +import { pullRkey } from './pull-requests.js'; import { deleteSession, findSessionByDid, @@ -282,7 +283,9 @@ function buildPullRecord(about) { } /** - * Create a pull record for the current branch. + * Create the current branch's pull request: one record at the branch's key, + * which `pr update` rewrites in place. Nothing may hold the key yet, so a + * second `pr create` refuses rather than clobbering the words already there. * @param {string[]} args * @returns {Promise} */ @@ -290,27 +293,31 @@ async function createPull(args) { const options = parsePullOptions(args); const { branch, sha, location, client, subject, repoName } = await pullContext(options); + const ref = `refs/heads/${branch}`; const record = buildPullRecord({ subject, repo: repoName, - ref: `refs/heads/${branch}`, + ref, sha, title: options.title, status: options.draft ? 'draft' : '', note: options.note, }); - const created = await client.createRecord( + const created = await client.putRecord( location.did, GIT_PULL_COLLECTION, + pullRkey(repoName, ref), record, + null, ); process.stdout.write(`${created.uri}\n`); } /** - * Update the current branch's pull request: a new record that carries the - * newest title, note and status. The reader takes the newest record per - * branch, so this supersedes what `pr create` wrote and earlier updates. + * Update the current branch's pull request: rewrite the record at the + * branch's key with the newest words and the tip at HEAD, keeping what a + * flag did not set. The write compares against the version just read, so a + * concurrent edit surfaces instead of being silently overwritten. * @param {string[]} args * @returns {Promise} */ @@ -324,36 +331,37 @@ async function updatePull(args) { const { branch, sha, location, client, subject, repoName } = await pullContext(options); const ref = `refs/heads/${branch}`; - const rows = await client.listRecords(location.did, GIT_PULL_COLLECTION); - const existing = rows - .map((row) => /** @type {Record} */ (row.value)) - .find( - (value) => - /** @type {{uri?: unknown}} */ (value.subject)?.uri === subject.uri && - value.ref === ref, - ); + const rkey = pullRkey(repoName, ref); + const existing = await client.getRecord( + location.did, + GIT_PULL_COLLECTION, + rkey, + ); if (!existing) { throw new Error( `no pull request for ${branch}; run git-remote-atproto pr create first`, ); } + const held = /** @type {Record} */ (existing.value); const record = buildPullRecord({ subject, repo: repoName, ref, sha, - title: options.title ?? /** @type {string|undefined} */ (existing.title), + title: options.title ?? /** @type {string|undefined} */ (held.title), status: options.draft ? 'draft' - : /** @type {string|undefined} */ (existing.status), - note: options.note ?? /** @type {string|undefined} */ (existing.note), + : /** @type {string|undefined} */ (held.status), + note: options.note ?? /** @type {string|undefined} */ (held.note), }); - const created = await client.createRecord( + const updated = await client.putRecord( location.did, GIT_PULL_COLLECTION, + rkey, record, + existing.cid, ); - process.stdout.write(`${created.uri}\n`); + process.stdout.write(`${updated.uri}\n`); } async function main() { diff --git a/packages/git/src/pull-request-reader.js b/packages/git/src/pull-request-reader.js index 7e723e4..91fb475 100644 --- a/packages/git/src/pull-request-reader.js +++ b/packages/git/src/pull-request-reader.js @@ -21,10 +21,10 @@ import { mergeText } from './merge3.js'; import { byPull, fixesOf, - latestIntents, mergedRun, place, pullKey, + pullRkey, runsForCopies, shortRef, subjectOf, @@ -496,13 +496,16 @@ export function createPullRequestReader(ctx) { discovered(subjectUri, PULL_COLLECTION, collaborators), discovered(subjectUri, REPO_COLLECTION, collaborators, 'upstream'), ]); - /** @type {Map|null, words: Map}>} */ + /** @type {Map|null, words: Map}>} */ const byDid = new Map(); // A declared fork is read whole: null names every branch. for (const { who } of forks) { byDid.set(who.did, { who, refs: null, words: new Map() }); } - for (const { who, value } of records) { + // A pull request lives at the key its branch derives, and that record is + // the current word. Discovery may name several records for one branch; the + // one at the derived key wins, and where none is, the last read does. + for (const { who, uri, value } of records) { const { ref, note, status, title } = /** @type {Record} */ (value); if (typeof ref !== 'string') continue; @@ -513,8 +516,14 @@ export function createPullRequestReader(ctx) { words: new Map(), }; group.refs?.add(short); - if (note || status || title) - group.words.set(short, { note, status, title }); + const rkey = String(uri).split('/').pop() ?? ''; + if (note || status || title) { + const canonical = rkey === pullRkey(repo, ref); + const current = group.words.get(short); + if (!current || canonical || !current.canonical) { + group.words.set(short, { note, status, title, canonical }); + } + } byDid.set(who.did, group); } @@ -673,7 +682,7 @@ export function createPullRequestReader(ctx) { ) ).filter((who) => who !== null); - const [found, reviews, checks, forked, intents] = await Promise.all([ + const [found, reviews, checks, forked] = await Promise.all([ Promise.all( collaborators.map((who) => pullsFrom( @@ -690,30 +699,26 @@ export function createPullRequestReader(ctx) { statements(voices, subjectUri, REVIEW_COLLECTION), runs(config, name, dids), forkedPulls(name, subjectUri, main, canonicalBranch, new Set(dids)), - Promise.all( - collaborators.map( - /** @returns {Promise<[string, ReturnType]>} */ - async (who) => [ - who.did, - latestIntents( - await listRecords(who, PULL_COLLECTION), - subjectUri, - ), - ], - ), - ).then((pairs) => new Map(pairs)), ]); const mine = found.flat(); - for (const pull of mine) { - const intent = intents.get(pull.author.did)?.get(pull.branch); - if (intent) { - pull.status = intent.status; - pull.note = intent.note ?? ''; - pull.noteAt = intent.createdAt; - if (intent.title) pull.title = intent.title; - } - } + await Promise.all( + mine.map(async (pull) => { + const who = pull.author; + const record = await getRecord( + who, + PULL_COLLECTION, + pullRkey(name, `refs/heads/${pull.branch}`), + ); + if (!record) return; + const value = /** @type {Record} */ (record.value); + if (typeof value.title === 'string') pull.title = value.title; + if (typeof value.status === 'string') pull.status = value.status; + if (typeof value.note === 'string') pull.note = value.note; + if (typeof value.createdAt === 'string') + pull.noteAt = value.createdAt; + }), + ); const pulls = [...mine, ...forked].sort((a, b) => b.when - a.when); const open = pulls.filter((pull) => !pull.merged); diff --git a/packages/git/src/pull-requests.js b/packages/git/src/pull-requests.js index 046928f..d579dc3 100644 --- a/packages/git/src/pull-requests.js +++ b/packages/git/src/pull-requests.js @@ -206,48 +206,20 @@ export function fileSummary(paths) { return { count: paths.length, dirs }; } +/** Characters a record key may hold; anything else becomes `_`. */ +const RKEY_SAFE = /[^A-Za-z0-9._~:-]/g; + /** - * The newest record per branch among one account's pull request records about - * one repository. What decorates a collaborator's branches with the words only - * the author can say: a title, a draft or withdrawn status, or a note. - * - * Keyed by short branch name, which with the account is what identifies a pull - * request. - * @param {unknown[]} rows - listRecords entries from that account - * @param {string} subjectUri - the canonical repository record's AT-URI - * @returns {Map} + * The record key one branch's pull request lives at: the repository name and + * the full ref, which together name the pull request. The CLI writes it with + * `putRecord` and the reader reads it back, so both must agree or an update + * is never seen. + * @param {string} repoName + * @param {string} ref - full ref name + * @returns {string} */ -export function latestIntents(rows, subjectUri) { - const records = rows - .map( - (row) => - /** @type {Record} */ ( - /** @type {{value?: unknown}|null} */ (row)?.value ?? {} - ), - ) - .filter( - (value) => - value.subject?.uri === subjectUri && typeof value.ref === 'string', - ) - .sort((a, b) => - String(b.createdAt ?? '').localeCompare(String(a.createdAt ?? '')), - ); - /** @type {Map>} */ - const index = new Map(); - for (const record of records) { - const short = shortRef(record.ref); - if (!index.has(short)) { - index.set(short, { - status: record.status, - note: record.note, - title: record.title, - createdAt: record.createdAt, - }); - } - } - return index; -} +export const pullRkey = (repoName, ref) => + `${repoName}:${ref}`.replace(RKEY_SAFE, '_'); /** * Every reviewer's records about this repository, indexed by what they name. diff --git a/packages/git/src/xrpc.js b/packages/git/src/xrpc.js index 66fabf9..16eeb0b 100644 --- a/packages/git/src/xrpc.js +++ b/packages/git/src/xrpc.js @@ -246,30 +246,6 @@ export class XrpcClient { ); } - /** - * The records in one collection of one repo, newest first. - * @param {string} repo - * @param {string} collection - * @param {number} [limit] - * @returns {Promise>} - */ - async listRecords(repo, collection, limit = 100) { - const params = new URLSearchParams({ - repo, - collection, - limit: String(limit), - }); - const res = await fetch( - `${this.service}/xrpc/com.atproto.repo.listRecords?${params}`, - ); - if (!res.ok) await throwXrpcError(res); - const body = - /** @type {{records: Array<{uri: string, cid: string, value: unknown}>}} */ ( - await res.json() - ); - return body.records; - } - /** * Write a record with compare-and-swap semantics. The swap has three * states, and null is not the permissive one: diff --git a/packages/git/test/cli-pr.test.js b/packages/git/test/cli-pr.test.js index 9040454..ac57467 100644 --- a/packages/git/test/cli-pr.test.js +++ b/packages/git/test/cli-pr.test.js @@ -20,9 +20,9 @@ afterEach(async () => { }); describe('git-remote-atproto pr create', () => { - it('writes a pull record for the current branch against its upstream', async () => { + it('writes the pull record at the branch key, requiring none exists', async () => { /** @type {Record|null} */ - let created = null; + let put = null; const server = createServer((request, response) => { const url = new URL(request.url || '/', 'http://127.0.0.1'); if (request.method === 'GET' && url.pathname.endsWith('repo.getRecord')) { @@ -45,17 +45,18 @@ describe('git-remote-atproto pr create', () => { } if ( request.method === 'POST' && - url.pathname.endsWith('repo.createRecord') + url.pathname.endsWith('repo.putRecord') ) { let body = ''; request.on('data', (chunk) => { body += chunk; }); request.on('end', () => { - created = JSON.parse(body).record; + put = JSON.parse(body); + const rkey = /** @type {string} */ (put?.rkey); response.setHeader('content-type', 'application/json').end( JSON.stringify({ - uri: 'at://did:plc:alice/dev.pdsjs.git.pull/pull-1', + uri: `at://did:plc:alice/dev.pdsjs.git.pull/${rkey}`, cid: 'pull-cid', }), ); @@ -146,26 +147,33 @@ describe('git-remote-atproto pr create', () => { await execFileAsync('git', ['rev-parse', 'HEAD'], { cwd: repo }) ).stdout.trim(); expect(stdout.trim()).toBe( - 'at://did:plc:alice/dev.pdsjs.git.pull/pull-1', + 'at://did:plc:alice/dev.pdsjs.git.pull/project:refs_heads_feature_cli', ); - expect(created).toMatchObject({ - $type: 'dev.pdsjs.git.pull', - subject: { - uri: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', - cid: 'canonical-cid', + expect(put).toMatchObject({ + collection: 'dev.pdsjs.git.pull', + repo: 'did:plc:alice', + rkey: 'project:refs_heads_feature_cli', + swapRecord: null, + record: { + $type: 'dev.pdsjs.git.pull', + subject: { + uri: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', + cid: 'canonical-cid', + }, + repo: 'project', + ref: 'refs/heads/feature/cli', + sha, + title: 'CLI change', + status: 'draft', + note: 'Created from the checkout.', }, - repo: 'project', - ref: 'refs/heads/feature/cli', - sha, - title: 'CLI change', - status: 'draft', - note: 'Created from the checkout.', }); expect( - /** @type {Record} */ ( - /** @type {unknown} */ (created) - ).createdAt, - ).toEqual(expect.any(String)); + /** @type {Record} */ (/** @type {unknown} */ (put)) + .record, + ).toMatchObject({ + createdAt: expect.any(String), + }); } finally { rmSync(root, { recursive: true, force: true }); } @@ -220,13 +228,14 @@ describe('git-remote-atproto pr update', () => { }); } - it('appends a record carrying the newest words and the current HEAD', async () => { - /** @type {Array>} */ - const stored = []; + it('rewrites the record at the branch key, keeping what a flag did not set', async () => { + /** @type {Array<{swapRecord: string|null, record: Record}>} */ + const puts = []; const server = createServer((request, response) => { const url = new URL(request.url || '/', 'http://127.0.0.1'); if (request.method === 'GET' && url.pathname.endsWith('repo.getRecord')) { const owner = url.searchParams.get('repo'); + const rkey = url.searchParams.get('rkey'); const value = owner === 'did:plc:alice' ? { @@ -234,6 +243,29 @@ describe('git-remote-atproto pr update', () => { upstream: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', } : { name: 'project' }; + // The copy's repo record (rkey `project`) or the pull record (the + // branch's derived key). + if (rkey === 'project:refs_heads_feature_cli') { + response.setHeader('content-type', 'application/json').end( + JSON.stringify({ + uri: `at://${owner}/dev.pdsjs.git.pull/${rkey}`, + cid: 'pull-cid-1', + value: { + subject: { + uri: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', + }, + repo: 'project', + ref: 'refs/heads/feature/cli', + sha: 'oldsha', + title: 'First title', + status: 'draft', + note: 'First note', + createdAt: '2026-08-01T00:00:00Z', + }, + }), + ); + return; + } response.setHeader('content-type', 'application/json').end( JSON.stringify({ uri: `at://${owner}/dev.pdsjs.git.repo/project`, @@ -243,35 +275,20 @@ describe('git-remote-atproto pr update', () => { ); return; } - if ( - request.method === 'GET' && - url.pathname.endsWith('repo.listRecords') - ) { - response.setHeader('content-type', 'application/json').end( - JSON.stringify({ - records: stored.map((value, index) => ({ - uri: `at://did:plc:alice/dev.pdsjs.git.pull/pull-${index + 1}`, - cid: `pull-cid-${index + 1}`, - value, - })), - }), - ); - return; - } if ( request.method === 'POST' && - url.pathname.endsWith('repo.createRecord') + url.pathname.endsWith('repo.putRecord') ) { let body = ''; request.on('data', (chunk) => { body += chunk; }); request.on('end', () => { - stored.push(JSON.parse(body).record); + puts.push(JSON.parse(body)); response.setHeader('content-type', 'application/json').end( JSON.stringify({ - uri: `at://did:plc:alice/dev.pdsjs.git.pull/pull-${stored.length}`, - cid: `pull-cid-${stored.length}`, + uri: `at://did:plc:alice/dev.pdsjs.git.pull/${JSON.parse(body).rkey}`, + cid: 'pull-cid-2', }), ); }); @@ -301,11 +318,6 @@ describe('git-remote-atproto pr update', () => { ATPROTO_GIT_CONFIG_DIR: config, ATPROTO_GIT_SERVICE: service, }; - await execFileAsync( - process.execPath, - [cliPath, 'pr', 'create', '--title', 'First title', '--draft'], - { cwd: repo, env }, - ); writeFileSync(join(repo, 'change.txt'), 'change again\n'); execFileSync('git', ['add', 'change.txt'], { cwd: repo }); @@ -330,21 +342,27 @@ describe('git-remote-atproto pr update', () => { ); expect(stdout.trim()).toBe( - 'at://did:plc:alice/dev.pdsjs.git.pull/pull-2', + 'at://did:plc:alice/dev.pdsjs.git.pull/project:refs_heads_feature_cli', ); - expect(stored).toHaveLength(2); - expect(stored[1]).toMatchObject({ - $type: 'dev.pdsjs.git.pull', - subject: { - uri: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', - cid: 'canonical-cid', + expect(puts).toHaveLength(1); + expect(puts[0]).toMatchObject({ + collection: 'dev.pdsjs.git.pull', + repo: 'did:plc:alice', + rkey: 'project:refs_heads_feature_cli', + swapRecord: 'pull-cid-1', + record: { + $type: 'dev.pdsjs.git.pull', + subject: { + uri: 'at://did:plc:canonical/dev.pdsjs.git.repo/project', + cid: 'canonical-cid', + }, + repo: 'project', + ref: 'refs/heads/feature/cli', + sha, + title: 'Second title', + status: 'draft', + note: 'Second body', }, - repo: 'project', - ref: 'refs/heads/feature/cli', - sha, - title: 'Second title', - status: 'draft', - note: 'Second body', }); } finally { rmSync(root, { recursive: true, force: true }); @@ -400,24 +418,23 @@ describe('git-remote-atproto pr update', () => { const server = createServer((request, response) => { const url = new URL(request.url || '/', 'http://127.0.0.1'); if (request.method === 'GET' && url.pathname.endsWith('repo.getRecord')) { + const owner = url.searchParams.get('repo'); + const rkey = url.searchParams.get('rkey'); + // The branch's pull record does not exist yet; the copy's repo record + // does. + if (rkey === 'project:refs_heads_feature_cli') { + response.writeHead(404).end(); + return; + } response.setHeader('content-type', 'application/json').end( JSON.stringify({ - uri: 'at://did:plc:alice/dev.pdsjs.git.repo/project', + uri: `at://${owner}/dev.pdsjs.git.repo/project`, cid: 'fork-cid', value: { name: 'project' }, }), ); return; } - if ( - request.method === 'GET' && - url.pathname.endsWith('repo.listRecords') - ) { - response - .setHeader('content-type', 'application/json') - .end(JSON.stringify({ records: [] })); - return; - } response.writeHead(404).end(); }); await new Promise((resolve) => diff --git a/packages/git/test/pull-request-reader.test.js b/packages/git/test/pull-request-reader.test.js index e96e3ce..89005c1 100644 --- a/packages/git/test/pull-request-reader.test.js +++ b/packages/git/test/pull-request-reader.test.js @@ -102,7 +102,7 @@ function world({ [BOB]: { 'dev.pdsjs.git.pull': [ { - uri: `at://${BOB}/dev.pdsjs.git.pull/1`, + uri: `at://${BOB}/dev.pdsjs.git.pull/demo:refs_heads_feature`, cid: 'b1', value: { subject: { uri: SUBJECT }, @@ -161,6 +161,21 @@ function world({ }, ], 'dev.pdsjs.git.pull': [ + { + // The current word, at the key the branch derives. Legacy records + // with server-generated keys are stale. + uri: `at://${DAN}/dev.pdsjs.git.pull/demo:refs_heads_fix`, + cid: 'd4', + value: { + subject: { uri: SUBJECT }, + repo: REPO, + ref: 'refs/heads/fix', + sha: sha('d'), + title: 'Dan renamed it', + note: 'the update', + createdAt: '2026-08-05T00:00:00Z', + }, + }, { uri: `at://${DAN}/dev.pdsjs.git.pull/3d`, cid: 'd1', @@ -171,6 +186,7 @@ function world({ sha: sha('d'), title: 'Dan named it', status: 'draft', + createdAt: '2026-08-01T00:00:00Z', }, }, { @@ -396,11 +412,19 @@ describe('createPullRequestReader', () => { it('reads a stranger the discovery port names, and only the branch their record names', async () => { const { reader, walked } = world({ about: [ - { did: DAN, collection: 'dev.pdsjs.git.pull', rkey: '3d' }, + { + did: DAN, + collection: 'dev.pdsjs.git.pull', + rkey: 'demo:refs_heads_fix', + }, // A record about another repository, which the record itself refutes. { did: DAN, collection: 'dev.pdsjs.git.pull', rkey: 'elsewhere' }, // A collaborator, already read whole, is not read twice. - { did: BOB, collection: 'dev.pdsjs.git.pull', rkey: '1' }, + { + did: BOB, + collection: 'dev.pdsjs.git.pull', + rkey: 'demo:refs_heads_feature', + }, ], }); const pulls = @@ -412,8 +436,8 @@ describe('createPullRequestReader', () => { expect(fix).toMatchObject({ id: `${DAN} fix`, fork: true, - title: 'Dan named it', - status: 'draft', + title: 'Dan renamed it', + note: 'the update', ahead: 1, behind: 0, base: sha('3'), @@ -427,6 +451,31 @@ describe('createPullRequestReader', () => { expect(pulls.open.map((pull) => pull.branch)).toEqual(['fix', 'feature']); }); + it("prefers the record at the branch's derived key over another discovery names", async () => { + // Discovery names the derived-key record and an older one. Either order, + // the record the branch's key derives is the current word. + const { reader } = world({ + about: [ + { did: DAN, collection: 'dev.pdsjs.git.pull', rkey: '3d' }, + { + did: DAN, + collection: 'dev.pdsjs.git.pull', + rkey: 'demo:refs_heads_fix', + }, + ], + }); + const pulls = + /** @type {NonNullable>>} */ ( + await reader.listPulls(ALICE, REPO) + ); + const fix = pulls.open.find((pull) => pull.author.did === DAN); + expect(fix).toMatchObject({ + title: 'Dan renamed it', + note: 'the update', + status: undefined, + }); + }); + it('reads every branch of a copy that names this repository as its upstream', async () => { const { reader, walked } = world({ about: [{ did: DAN, collection: 'dev.pdsjs.git.repo', rkey: REPO }], diff --git a/packages/git/test/pull-requests.test.js b/packages/git/test/pull-requests.test.js index 2ecd98b..8f0aae0 100644 --- a/packages/git/test/pull-requests.test.js +++ b/packages/git/test/pull-requests.test.js @@ -3,10 +3,10 @@ import { byPull, fileSummary, fixesOf, - latestIntents, mergedRun, place, pullKey, + pullRkey, replacedTips, runsForCopies, subjectOf, @@ -161,44 +161,14 @@ describe('runsForCopies', () => { }); }); -describe('latestIntents', () => { - const SUBJECT = 'at://did:plc:owner/dev.pdsjs.git.repo/project'; - /** @param {Partial>} value */ - const row = (value) => ({ value: { subject: { uri: SUBJECT }, ...value } }); - - it('keys the newest record by branch', () => { - const index = latestIntents( - [ - row({ - ref: 'refs/heads/agent/docs', - title: 'Document the reader', - status: 'draft', - createdAt: '2026-08-02T00:00:00Z', - }), - row({ - ref: 'refs/heads/agent/docs', - createdAt: '2026-08-01T00:00:00Z', - }), - ], - SUBJECT, +describe('pullRkey', () => { + it('derives a safe record key from the repository and the full ref', () => { + expect(pullRkey('project', 'refs/heads/feature/x')).toBe( + 'project:refs_heads_feature_x', ); - expect(index.get('agent/docs')?.status).toBe('draft'); - expect(index.get('agent/docs')?.title).toBe('Document the reader'); - }); - - it('drops records about another repository, and malformed rows', () => { - const index = latestIntents( - [ - row({ - subject: { uri: 'at://did:plc:owner/dev.pdsjs.git.repo/other' }, - ref: 'refs/heads/x', - }), - row({ ref: 42 }), - {}, - ], - SUBJECT, + expect(pullRkey('pds.js', 'refs/heads/main')).toBe( + 'pds.js:refs_heads_main', ); - expect(index.size).toBe(0); }); });