From f1e0452217c46e4af369574cf558167bdcab4b1c Mon Sep 17 00:00:00 2001 From: Dipesh Babu Date: Mon, 5 Oct 2026 00:20:45 -0400 Subject: [PATCH] fix: pull --rev confuses version numbers with hashes and strips valid hash characters --- packages/cli/src/commands/pull.ts | 22 ++++++++++++++-------- packages/cli/test/revisions.test.ts | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 packages/cli/test/revisions.test.ts diff --git a/packages/cli/src/commands/pull.ts b/packages/cli/src/commands/pull.ts index 0da11da..90fa8bf 100644 --- a/packages/cli/src/commands/pull.ts +++ b/packages/cli/src/commands/pull.ts @@ -123,6 +123,18 @@ function ownTarget(session: RemoteSession, version?: SessionDetail['versions'][n const normalizeId = (id: string) => id.trim().replace(/^#/, '').toLowerCase(); +export function selectVersion(versions: SessionDetail['versions'], rawRef: string): SessionDetail['versions'][number] { + const ref = rawRef.trim(); + const stored = versions.filter((v) => v.status === 'stored'); + let matches: SessionDetail['versions']; + if (/^id:[1-9]\d*$/.test(ref)) matches = stored.filter((v) => v.id === Number(ref.slice(3))); + else if (/^v?[1-9]\d*$/i.test(ref)) matches = stored.filter((v) => v.seq === Number(ref.replace(/^v/i, ''))); + else if (/^[A-Za-z0-9_-]+$/.test(ref)) matches = stored.filter((v) => v.contentHash.startsWith(ref)); + else matches = []; + if (matches.length !== 1) throw new Error(`--rev ${rawRef} matches ${matches.length} versions; use --versions to list them.`); + return matches[0]; +} + /** A pull id (own session or share), optionally a version by number `3` / `v3` or a content-hash prefix. */ async function resolveTarget(config: Config, id: string, versionRef: string | undefined): Promise { const { sessions, shared } = await fetchList(config); @@ -134,17 +146,11 @@ async function resolveTarget(config: Config, id: string, versionRef: string | un if (session) { if (!versionRef) return ownTarget(session); const detail = await fetchDetail(config, session.id); - const ref = versionRef.trim().replace(/^v/i, ''); - const matches = detail.versions.filter( - (v) => v.status === 'stored' && (byRowId !== null ? v.id === byRowId : String(v.seq) === ref || v.contentHash.startsWith(ref)) - ); - if (matches.length !== 1) throw new Error(`--rev ${versionRef} matches ${matches.length} versions; run \`vibi pull ${id} --versions\`.`); - return ownTarget(session, matches[0]); + return ownTarget(session, selectVersion(detail.versions, versionRef)); } const share = shared.find((s) => s.pullId === wanted); if (share) { - const refId = byRowId !== null ? byRowId : versionRef ? Number(versionRef.replace(/^v/i, '')) : null; - if (refId !== null && refId !== share.versionId) { + if (versionRef !== undefined && byRowId !== share.versionId) { throw new Error('shared sessions refer to one version; --rev does not apply.'); } return sharedTarget(share); diff --git a/packages/cli/test/revisions.test.ts b/packages/cli/test/revisions.test.ts new file mode 100644 index 0000000..10d70d1 --- /dev/null +++ b/packages/cli/test/revisions.test.ts @@ -0,0 +1,14 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { selectVersion } from '../src/commands/pull'; +import type { SessionDetail } from '@vibivibi/shared/sessions'; + +const versions = [{id: 80, seq: 1, status: 'stored', contentHash: 'vHash_ABC'}, {id: 90, seq: 2, status: 'stored', contentHash: '1different'}, {id: 91, seq: 3, status: 'pending', contentHash: 'pending'}] as SessionDetail['versions']; +test('version numbers cannot collide with hash prefixes and hashes retain their first character', () => { + assert.equal(selectVersion(versions, '1'), versions[0]); + assert.equal(selectVersion(versions, ' v2 '), versions[1]); + assert.equal(selectVersion(versions, 'vHash'), versions[0]); + assert.equal(selectVersion(versions, 'id:90'), versions[1]); + for (const ref of ['', ' ', '3', 'id:91', 'id:NaN', 'v0', 'v999', 'VHash']) assert.throws(() => selectVersion(versions, ref), /matches 0/); + assert.throws(() => selectVersion([...versions, {...versions[0], id: 95, contentHash: 'vHash_other'}], 'vHash'), /matches 2/); +});