diff --git a/docs/sync/README.md b/docs/sync/README.md index 84558dde..64540b83 100644 --- a/docs/sync/README.md +++ b/docs/sync/README.md @@ -120,9 +120,9 @@ A pseudonymous `device_id` distinguishes your machines without revealing hostnam | `git.in_main` | `true` | Whether the commit landed in the main branch | | `git.was_reverted` | `false` | Whether a later commit reverted it | -Attribution is **inferred** (timestamp-window correlation, the same heuristic as `codeburn yield`); the resource attribute `codeburn.attribution_methodology: timestamp-window` marks it as such. State transitions (a commit merging to main, or being reverted) are re-sent automatically on later pushes — receivers should upsert by `(git.repo, git.sha)`. +Attribution is **inferred** (timestamp-window correlation, the same heuristic as `codeburn yield`); the resource attribute `codeburn.attribution_methodology: timestamp-window` marks it as such. State transitions (a commit merging to main, or being reverted) are re-sent automatically on later pushes — receivers should upsert commits by `(git.repo, git.sha)` and session spans by `ai.session_id` (latest state wins). When a commit migrates to a later-parsed session with a tighter window, the losing session re-emits with `git.commit_count: 0` (a retraction), so summing `git.commit_count` across upserted session rows never double-counts. Retractions fire only when the commit was won by another session — commits that merely age out of the `--since` window are not retracted, so a previously-synced count stays correct. Session spans also re-emit when an ongoing session's window grows, keeping the span end time current. -With `--attribution`, normalized repo remote URLs, commit SHAs, commit timestamps (span start times), PR URLs, and the merged/reverted booleans leave your machine — plus the same pseudonymous `codeburn.device_id` resource attribute the usage spans carry. PR links are shape-checked client-side (https, `/org/repo/pull/N` path, bounded length, max 20 per session) before sending. Precisely what is and is not sent: +With `--attribution`, normalized repo remote URLs, commit SHAs, commit timestamps (span start times), PR URLs, and the merged/reverted booleans leave your machine — plus the same pseudonymous `codeburn.device_id` resource attribute the usage spans carry. PR links are rebuilt client-side from scheme + host + path only (userinfo, query strings, and fragments are dropped; https, `/org/repo/pull/N` path, bounded length, max 20 per session), and the repo identity itself passes a strict hostname/path allow-list before sending — malformed or transport-helper remotes (`ext::…`, `codecommit::…`) are rejected outright rather than parsed. Precisely what is and is not sent: - **Commits**: only from repos with a network `origin` remote, and only for sessions whose own project path resolved to that repo. Local-only repos, `file://` remotes, and Windows filesystem paths are never emitted as repo identities. A session whose project path no longer resolves never inherits the repo of the directory you happen to push from. - **PR links**: sent whenever a session captured them, even when the session's repo could not be identified — the PR URL itself names the repo, so this adds no information beyond the link the session already recorded. diff --git a/src/sync/cli.ts b/src/sync/cli.ts index a48561c5..f6f28db6 100644 --- a/src/sync/cli.ts +++ b/src/sync/cli.ts @@ -371,6 +371,12 @@ export function registerSyncCommands(program: Command): void { process.stderr.write('Auth rejected by server during attribution push. Run `codeburn sync setup` to re-authenticate.\n') process.exit(1) } + if (attrResult.outcome === 'rate-limited') { + process.stderr.write(`Rate limited during attribution push — gave up after repeated retries. Remaining facts will be sent on the next push.\n`) + } + if (attrResult.outcome === 'server-error') { + process.stderr.write(`Server error (HTTP ${attrResult.httpStatus}) during attribution push. Remaining facts will be sent on the next push.\n`) + } } else { process.stderr.write(`Skipping attribution push (${attributionUnsent.length} facts) — will retry on next push.\n`) } @@ -382,7 +388,10 @@ export function registerSyncCommands(program: Command): void { // Summary process.stderr.write(`\nSynced ${result.totalSent} calls ($${result.totalCostSent.toFixed(2)}) to ${config.baseUrl}\n`) if (attrResult) { - process.stderr.write(` Attribution: ${attrResult.totalSent} facts synced${attrResult.totalRejected > 0 ? `, ${attrResult.totalRejected} rejected (will retry)` : ''}\n`) + const attrSuffix = attrResult.outcome !== 'complete' + ? ` (push incomplete — remainder retries next push)` + : attrResult.totalRejected > 0 ? `, ${attrResult.totalRejected} rejected (will retry)` : '' + process.stderr.write(` Attribution: ${attrResult.totalSent} facts synced${attrSuffix}\n`) } if (result.totalRejected > 0) { process.stderr.write(` ${result.totalRejected} spans rejected (will retry on next push)\n`) diff --git a/src/sync/otlp.ts b/src/sync/otlp.ts index d6a042e0..8df36c59 100644 --- a/src/sync/otlp.ts +++ b/src/sync/otlp.ts @@ -189,7 +189,22 @@ export function sessionAttributionKey(record: SessionAttributionRecord): string const commitStates = record.commits .map(c => `${c.sha}:${c.inMain ? 1 : 0}${c.wasReverted ? 1 : 0}`) .sort() - return `attr:s:${record.sessionId}:${stateHash([record.repo ?? '', ...record.prLinks, ...commitStates])}` + // Project and both window timestamps are part of the state: an ongoing + // session whose window grew (or whose project resolution changed) re-emits + // with the corrected span times instead of freezing at first send. + return `attr:s:${record.sessionId}:${stateHash([ + record.repo ?? '', + record.project, + record.firstTimestamp, + record.lastTimestamp, + ...record.prLinks, + ...commitStates, + ])}` +} + +/** Ledger-key prefix for a session's attribution facts (any state). */ +export function sessionAttributionKeyPrefix(sessionId: string): string { + return `attr:s:${sessionId}:` } /** Flatten attribution records into ledger-able items (one session item + one per commit). */ @@ -234,9 +249,11 @@ export function buildAttributionOtlpPayload(items: AttributionItem[]): OtlpPaylo const spans: OtlpSpan[] = items.map(item => { const startNano = toUnixNano(item.timestamp) - const endNano = item.endTimestamp - ? toUnixNano(item.endTimestamp) - : (BigInt(startNano) + 1_000_000n).toString() + // Clamp like the usage builder: end is never 0 (malformed timestamp) and + // never earlier than start + 1ms (out-of-order session timestamps). + const minEndNano = BigInt(startNano) + 1_000_000n + const rawEndNano = item.endTimestamp ? BigInt(toUnixNano(item.endTimestamp)) : 0n + const endNano = (rawEndNano > minEndNano ? rawEndNano : minEndNano).toString() const attributes: OtlpAttribute[] = [ { key: 'ai.session_id', value: { stringValue: item.sessionId } }, diff --git a/src/sync/push.ts b/src/sync/push.ts index a0ffc016..7a22420e 100644 --- a/src/sync/push.ts +++ b/src/sync/push.ts @@ -209,8 +209,24 @@ export function collectUnsentAttribution(records: SessionAttributionRecord[]): { allItems: AttributionItem[] unsent: AttributionItem[] } { - const allItems = flattenAttributionRecords(records) const sent = ledgerKeySet() + + // Empty records (no commits, no PR links) exist only to RETRACT a session + // span whose commits migrated to another session. Send one only when a + // PRIOR state for that session was already ledgered — a session that was + // never sent has nothing to retract. + const sessionsWithPriorState = new Set() + for (const key of sent) { + if (key.startsWith('attr:s:')) { + const sessionId = key.slice('attr:s:'.length, key.lastIndexOf(':')) + sessionsWithPriorState.add(sessionId) + } + } + const sendable = records.filter(r => + r.commits.length > 0 || r.prLinks.length > 0 || sessionsWithPriorState.has(r.sessionId), + ) + + const allItems = flattenAttributionRecords(sendable) const unsent = allItems.filter(i => !sent.has(i.dedupKey)) return { allItems, unsent } } diff --git a/src/yield.ts b/src/yield.ts index 33e435af..79239b99 100644 --- a/src/yield.ts +++ b/src/yield.ts @@ -146,6 +146,24 @@ function getMainBranch(cwd: string): string { * and a trailing `.git` / `/` is removed. Local paths and `file://` remotes * return null — a repo with no network remote has no server-side identity. */ +/** Max length of an emitted repo identity (`host/org/repo`). */ +const MAX_REPO_IDENTITY_LENGTH = 200 + +/** + * Positive validation (allow-list) of a composed repo identity — the final + * gate EVERY branch passes through before anything is returned. The host must + * look like a hostname and every path segment like a repo path segment, so no + * upstream parsing quirk (transport-helper remotes like `ext::…` or + * `codecommit::…`, credentials that survived a malformed URL, oversized + * strings) can reach the wire. Rejecting is always safe: an unrecognizable + * remote simply has no server-side identity. + */ +function isValidRepoIdentity(host: string, segments: string[]): boolean { + if (!/^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$/.test(host)) return false + if (segments.length === 0) return false + return segments.every(s => /^[A-Za-z0-9._~-]+$/.test(s)) +} + export function normalizeRemoteUrl(url: string): string | null { const trimmed = url.trim() if (!trimmed) return null @@ -159,12 +177,6 @@ export function normalizeRemoteUrl(url: string): string | null { let host: string let path: string - // scp-like syntax: [user@]host:path. Host must be at least 2 chars — a - // single-character "host" is a Windows drive-relative path (`C:repo`), - // never a real remote host. `@` is excluded from the host class so a - // rejected single-char host can't backtrack into `user@C` matching as - // host "user@C". - const scpLike = /^(?:[^@/]+@)?([^:/\\@]{2,}):(?!\/\/)(.+)$/.exec(trimmed) if (/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(trimmed)) { let parsed: URL try { @@ -176,22 +188,34 @@ export function normalizeRemoteUrl(url: string): string | null { if (!parsed.hostname) return null host = parsed.hostname path = parsed.pathname - } else if (scpLike) { - // scp-like syntax: [user@]host:path — not parseable by URL + } else { + // scp-like syntax: [user@]host:path. Credentials (userinfo) are split off + // at the FIRST `@` BEFORE any host matching — expressing userinfo as an + // optional regex group lets backtracking abandon the group and re-parse a + // credential prefix as `host:path`, dumping the token into the path + // (e.g. `x-access-token:ghp_…@github.com/org/repo`). Host must be at + // least 2 chars: a single-character "host" is a Windows drive-relative + // path (`C:repo`), never a real remote host. + const at = trimmed.indexOf('@') + const rest = at >= 0 ? trimmed.slice(at + 1) : trimmed + const scpLike = /^([^:/\\]{2,}):(?!\/\/)(.+)$/.exec(rest) + if (!scpLike) return null host = scpLike[1] path = scpLike[2] - } else { - // Bare local path (or something unrecognizable) — no remote identity. - return null } const cleanPath = path + .replace(/\/+/g, '/') // collapse doubled slashes → one join key, not two .replace(/^\/+/, '') .replace(/\/+$/, '') - .replace(/\.git$/, '') + .replace(/\.git$/i, '') // case-insensitive: Repo.GIT joins with repo.git if (!cleanPath) return null - return `${host.toLowerCase()}/${cleanPath}` + const identity = `${host.toLowerCase()}/${cleanPath}` + if (identity.length > MAX_REPO_IDENTITY_LENGTH) return null + if (!isValidRepoIdentity(host.toLowerCase(), cleanPath.split('/'))) return null + + return identity } /** `git remote get-url origin`, normalized. Null when absent or local-only. */ @@ -569,21 +593,6 @@ export type SessionAttributionRecord = { lastTimestamp: string } -/** - * Compute per-session attribution records for sync. Reuses the exact yield - * repo grouping + tightest-window commit attribution (`methodology: - * timestamp-window`), then joins in each repo group's normalized origin - * remote and the session's PR links. - * - * Inclusion rules: - * - Sessions with neither attributed commits nor PR links are omitted. - * - Commits are only included when the repo has a normalized remote; a SHA - * without a repo identity has no server-side meaning. Such sessions still - * emit a record when they carry PR links (the PR URL embeds the repo). - * - * Takes already-parsed projects (sync push has them in hand) instead of - * re-parsing like computeYield does. - */ /** Max PR links retained per session attribution record. */ export const MAX_PR_LINKS_PER_SESSION = 20 @@ -592,11 +601,16 @@ export const MAX_PR_LINKS_PER_SESSION = 20 * verify truthiness, so arbitrary strings can land in `session.prLinks`. * Keep only https URLs shaped like a PR (`/org/repo/pull/N` — GitHub and * GitHub Enterprise), bounded in length, capped per session, sorted. + * + * Links are REBUILT from `origin + pathname`, never passed through verbatim: + * userinfo (`https://alice:token@…`), query strings (copy-pasted GitHub + * links routinely carry `?notification_referrer_id=…`), and fragments are + * all dropped. Rebuilt links that collapse to the same URL dedupe. */ export function sanitizePrLinks(links: string[]): string[] { - const valid: string[] = [] + const valid = new Set() for (const link of links) { - if (typeof link !== 'string' || link.length === 0 || link.length > 256) continue + if (typeof link !== 'string' || link.length === 0 || link.length > 512) continue let url: URL try { url = new URL(link) @@ -605,11 +619,34 @@ export function sanitizePrLinks(links: string[]): string[] { } if (url.protocol !== 'https:') continue if (!/^\/[^/]+\/[^/]+\/pull\/\d+$/.test(url.pathname)) continue - valid.push(link) + const rebuilt = `${url.origin}${url.pathname}` + if (rebuilt.length > 256) continue + valid.add(rebuilt) } - return valid.sort().slice(0, MAX_PR_LINKS_PER_SESSION) + return [...valid].sort().slice(0, MAX_PR_LINKS_PER_SESSION) } +/** + * Compute per-session attribution records for sync. Reuses the exact yield + * repo grouping + tightest-window commit attribution (`methodology: + * timestamp-window`), then joins in each repo group's normalized origin + * remote and the session's sanitized PR links. + * + * Inclusion rules: + * - Only sessions whose OWN project path resolved to a repo participate in + * commit attribution; cwd-fallback sessions never carry a repo or commits + * (privacy gate — see below) but still emit a record when they have PR links. + * - Commits require a normalized remote; a SHA without a repo identity has + * no server-side meaning. + * - A session with no commits and no PR links is emitted ONLY when it lost a + * commit to a tighter-window session in THIS computation (`lostCandidacy`) + * — a retraction candidate. A session that merely aged its commits out of + * the range lost them to nobody, and emitting an empty record for it would + * permanently retract a still-correct server-side count. + * + * Takes already-parsed projects (sync push has them in hand) instead of + * re-parsing like computeYield does. + */ export function computeAttributionRecords( projects: ProjectSummary[], range: DateRange, @@ -636,8 +673,10 @@ export function computeAttributionRecords( // Keyed by object reference: session objects are unique per group entry, // whereas sessionId strings could collide across projects. const attributionBySession = new Map() + const lostCandidacyBySession = new Map() for (const [i, session] of ownSessions.entries()) { attributionBySession.set(session, attributions[i]?.commits ?? []) + lostCandidacyBySession.set(session, attributions[i]?.lostCandidacy ?? false) } for (const [index, session] of group.sessions.entries()) { @@ -649,7 +688,17 @@ export function computeAttributionRecords( ? (attributionBySession.get(session) ?? []) : [] const prLinks = sanitizePrLinks(session.prLinks ?? []) - if (attributedCommits.length === 0 && prLinks.length === 0) continue + // Empty sessions are retraction candidates ONLY when they lost a commit + // to a tighter-window session in THIS run: that commit's server-side + // attribution is migrating, so the loser must re-emit commit_count=0. + // An empty session whose commits merely aged out of the --since range + // (rolling window, or a narrower window than a previous push) lost them + // to NOBODY — emitting a retraction for it would permanently zero a + // still-correct server-side count, because the original state key stays + // ledgered and is never re-sent. + const lostToTighterSession = sessionRemote !== null && + (lostCandidacyBySession.get(session) ?? false) + if (attributedCommits.length === 0 && prLinks.length === 0 && !lostToTighterSession) continue records.push({ sessionId: session.sessionId, diff --git a/tests/sync-attribution.test.ts b/tests/sync-attribution.test.ts index 4012b4c4..d4b8f6c1 100644 --- a/tests/sync-attribution.test.ts +++ b/tests/sync-attribution.test.ts @@ -133,6 +133,40 @@ describe('normalizeRemoteUrl', () => { expect(normalizeRemoteUrl('gitserver:team/repo.git')).toBe('gitserver/team/repo') expect(normalizeRemoteUrl('git@gitbox:org/repo.git')).toBe('gitbox/org/repo') }) + + it('never leaks credentials via scp-branch backtracking on malformed remotes', () => { + // Credential-prefixed remotes: the userinfo split happens BEFORE host + // matching, so a token can never be re-parsed as host:path. + expect(normalizeRemoteUrl('x-access-token:ghp_LIVETOKEN_abcdefghijklmnop@github.com/acme/private-repo.git')).toBeNull() + expect(normalizeRemoteUrl('oauth2:glpat-TOKEN@gitlab.com/org/repo.git')).toBeNull() + // One dropped slash: not a URL, must not fall through as host "https" + expect(normalizeRemoteUrl('https:/user:ghp_TOKEN@github.com/org/repo.git')).toBeNull() + // Multiple @: split at the first, residual @ fails the allow-list + expect(normalizeRemoteUrl('a@b@github.com:org/repo.git')).toBeNull() + expect(normalizeRemoteUrl('user@host:path@with-at')).toBeNull() + }) + + it('rejects transport-helper remotes and enforces shape + length on the identity', () => { + // git-remote-ext: embeds a local SSH key path + expect(normalizeRemoteUrl('ext::ssh -i /Users/me/.ssh/id_ed25519_work git@github.com %S /acme/private.git')).toBeNull() + expect(normalizeRemoteUrl('ext::sh -c whatever')).toBeNull() + // git-remote-codecommit: embeds an AWS profile name + expect(normalizeRemoteUrl('codecommit::us-east-1://MyAwsProfile@MyRepo')).toBeNull() + expect(normalizeRemoteUrl('codecommit::us-east-1://MyRepo')).toBeNull() + // Length bound + expect(normalizeRemoteUrl(`git@github.com:org/${'a'.repeat(300)}.git`)).toBeNull() + // Path segments must be repo-shaped (no spaces, colons, @) + expect(normalizeRemoteUrl('gitserver:has space/repo.git')).toBeNull() + // Legit multi-segment (GitLab subgroup) paths survive the allow-list + expect(normalizeRemoteUrl('https://gitlab.example.com/group/sub/repo.git')).toBe('gitlab.example.com/group/sub/repo') + }) + + it('normalizes .GIT case-insensitively and collapses doubled slashes to one join key', () => { + expect(normalizeRemoteUrl('git@github.com:acme/Repo.GIT')).toBe('github.com/acme/Repo') + expect(normalizeRemoteUrl('https://github.com/acme/Repo.Git')).toBe('github.com/acme/Repo') + expect(normalizeRemoteUrl('https://github.com/acme//repo.git')).toBe('github.com/acme/repo') + expect(normalizeRemoteUrl('git@github.com:acme//repo.git')).toBe('github.com/acme/repo') + }) }) // ── computeAttributionRecords ───────────────────────────────────────── @@ -172,13 +206,16 @@ describe('computeAttributionRecords', () => { } }) - it('omits sessions with no commits and no PR links', async () => { + it('omits empty sessions that lost nothing — commits aged out of range are NOT retracted', async () => { const repoDir = await mkdtemp(join(tmpdir(), 'codeburn-attr-empty-')) try { initRepo(repoDir) git(repoDir, ['remote', 'add', 'origin', 'git@github.com:acme/widget.git']) await writeFile(join(repoDir, 'file.txt'), 'hello\n') - // Commit outside every session window + // Commit outside every session window: no session competes for it, so + // nobody "lost" it. Even if this session previously synced a commit + // (now outside the --since range), emitting an empty record here would + // permanently zero a still-correct server-side count. commitAt(repoDir, 'feat: unrelated', '2026-01-01T20:00:00Z') const session = makeSession({ sessionId: 'sess-idle' }) @@ -187,6 +224,44 @@ describe('computeAttributionRecords', () => { ] expect(computeAttributionRecords(projects, range, repoDir)).toEqual([]) + + // …and therefore nothing can be sent, even with prior ledger state for + // this session (simulating an earlier wider---since push). + const { writeLedger } = await import('../src/sync/ledger.js') + writeLedger([{ key: 'attr:s:sess-idle:0123456789abcdef', ts: '2026-01-01T10:00:00.000Z' }]) + const { collectUnsentAttribution } = await import('../src/sync/push.js') + expect(collectUnsentAttribution(computeAttributionRecords(projects, range, repoDir)).unsent).toEqual([]) + } finally { + await rm(repoDir, { recursive: true, force: true }) + } + }) + + it('emits a retraction candidate for a session that lost its commit to a tighter window', async () => { + const repoDir = await mkdtemp(join(tmpdir(), 'codeburn-attr-lost-')) + try { + initRepo(repoDir) + git(repoDir, ['remote', 'add', 'origin', 'git@github.com:acme/widget.git']) + await writeFile(join(repoDir, 'file.txt'), 'hello\n') + commitAt(repoDir, 'feat: contested', '2026-01-01T10:30:00Z') + + const tight = makeSession({ + sessionId: 'sess-tight', + firstTimestamp: '2026-01-01T10:15:00.000Z', + lastTimestamp: '2026-01-01T10:45:00.000Z', + }) + const broadLoser = makeSession({ sessionId: 'sess-broad-loser' }) + const projects = [ + { project: 'app', projectPath: repoDir, sessions: [tight, broadLoser] } as ProjectSummary, + ] + + const records = computeAttributionRecords(projects, range, repoDir) + const loser = records.find(r => r.sessionId === 'sess-broad-loser')! + // The loser IS emitted (retraction candidate: lostCandidacy) with zero + // commits — the sync layer decides whether a prior state warrants + // actually sending it. + expect(loser).toBeDefined() + expect(loser.commits).toEqual([]) + expect(loser.repo).toBe('github.com/acme/widget') } finally { await rm(repoDir, { recursive: true, force: true }) } @@ -326,9 +401,13 @@ describe('computeAttributionRecords', () => { const records = computeAttributionRecords(projects, range, repoDir) - expect(records).toHaveLength(1) - expect(records[0]!.sessionId).toBe('sess-tight') - expect(records[0]!.commits).toHaveLength(1) + // Both sessions get records (the loser is a retraction candidate), + // but the commit is awarded exactly once — to the tighter window. + expect(records).toHaveLength(2) + const tightRecord = records.find(r => r.sessionId === 'sess-tight')! + const broadRecord = records.find(r => r.sessionId === 'sess-broad')! + expect(tightRecord.commits).toHaveLength(1) + expect(broadRecord.commits).toEqual([]) } finally { await rm(repoDir, { recursive: true, force: true }) } @@ -372,6 +451,14 @@ describe('attribution dedup keys', () => { expect(sessionAttributionKey(mutated)).not.toBe(sessionAttributionKey(record)) }) + it('session key changes when the window or project changes (ongoing sessions re-emit)', () => { + const base = makeRecord() + const grown = makeRecord({ lastTimestamp: '2026-01-01T12:00:00.000Z' }) + const renamed = makeRecord({ project: 'app-renamed' }) + expect(sessionAttributionKey(grown)).not.toBe(sessionAttributionKey(base)) + expect(sessionAttributionKey(renamed)).not.toBe(sessionAttributionKey(base)) + }) + it('flattens one session item plus one item per commit', () => { const items = flattenAttributionRecords([makeRecord()]) expect(items).toHaveLength(2) @@ -404,9 +491,36 @@ describe('sanitizePrLinks', () => { ]) }) - it('drops oversized strings and caps the count per session', () => { - const huge = `https://github.com/acme/widget/pull/1?x=${'a'.repeat(300)}` + it('rebuilds links from origin + pathname: userinfo, query, and fragment never survive', () => { + expect(sanitizePrLinks([ + 'https://alice:ghp_TOKEN@github.com/acme/widget/pull/5', + 'https://github.com/acme/widget/pull/7?notification_referrer_id=xyz', + 'https://github.com/acme/widget/pull/6#pullrequestreview-123', + ])).toEqual([ + 'https://github.com/acme/widget/pull/5', + 'https://github.com/acme/widget/pull/6', + 'https://github.com/acme/widget/pull/7', + ]) + }) + + it('dedupes links that collapse to the same rebuilt URL', () => { + expect(sanitizePrLinks([ + 'https://github.com/acme/widget/pull/9', + 'https://github.com/acme/widget/pull/9?ref=a', + 'https://github.com/acme/widget/pull/9#comment', + ])).toEqual(['https://github.com/acme/widget/pull/9']) + }) + + it('drops oversized inputs and caps the count per session', () => { + // A long referrer query is stripped, so the link survives... + const longQuery = `https://github.com/acme/widget/pull/1?x=${'a'.repeat(300)}` + expect(sanitizePrLinks([longQuery])).toEqual(['https://github.com/acme/widget/pull/1']) + // ...but pathological inputs beyond the input bound are dropped outright + const huge = `https://github.com/acme/widget/pull/1?x=${'a'.repeat(600)}` expect(sanitizePrLinks([huge])).toEqual([]) + // And a rebuilt link that is itself oversized is dropped + const longPath = `https://github.com/${'o'.repeat(150)}/${'r'.repeat(80)}/pull/1` + expect(sanitizePrLinks([longPath])).toEqual([]) const many = Array.from({ length: 30 }, (_, i) => `https://github.com/acme/widget/pull/${i + 1}`) expect(sanitizePrLinks(many)).toHaveLength(MAX_PR_LINKS_PER_SESSION) @@ -472,6 +586,22 @@ describe('buildAttributionOtlpPayload', () => { const items = flattenAttributionRecords([makeRecord(), makeRecord({ sessionId: 'sess-2' })]) expect(batchAttributionItems(items, 3).map(b => b.length)).toEqual([3, 1]) }) + + it('clamps span end time: never 0, never earlier than start + 1ms', () => { + // Session window ends BEFORE it starts (out-of-order provider timestamps) + const outOfOrder = flattenAttributionRecords([makeRecord({ + firstTimestamp: '2026-01-01T11:00:00.000Z', + lastTimestamp: '2026-01-01T10:00:00.000Z', + })]) + const span1 = buildAttributionOtlpPayload(outOfOrder).resourceSpans[0]!.scopeSpans[0]!.spans[0]! + expect(BigInt(span1.endTimeUnixNano)).toBe(BigInt(span1.startTimeUnixNano) + 1_000_000n) + + // Malformed end timestamp (toUnixNano -> 0) + const malformed = flattenAttributionRecords([makeRecord({ lastTimestamp: 'not-a-date' })]) + const span2 = buildAttributionOtlpPayload(malformed).resourceSpans[0]!.scopeSpans[0]!.spans[0]! + expect(span2.endTimeUnixNano).not.toBe('0') + expect(BigInt(span2.endTimeUnixNano)).toBe(BigInt(span2.startTimeUnixNano) + 1_000_000n) + }) }) // ── Send + ledger pipeline ──────────────────────────────────────────── @@ -566,6 +696,42 @@ describe('sendAttributionBatches + collectUnsentAttribution', () => { } }) + it('retracts a session span when its commit migrates to a tighter-window session', async () => { + const { sendAttributionBatches, collectUnsentAttribution } = await import('../src/sync/push.js') + + const sha = 'b'.repeat(40) + const commit = { sha, timestamp: '2026-01-01T10:30:00.000Z', inMain: true, wasReverted: false } + + // Push 1: session A (broad window) owns the commit + const push1 = collectUnsentAttribution([makeRecord({ sessionId: 'sess-A', prLinks: [], commits: [commit] })]) + expect(push1.unsent).toHaveLength(2) + const mock = await startMockOtlp([{ status: 200 }]) + try { + await sendAttributionBatches({ endpoint: mock.url, accessToken: 't', batches: [push1.unsent] }) + + // Push 2: a later-parsed tighter session B now wins the commit; A is empty + const push2 = collectUnsentAttribution([ + makeRecord({ sessionId: 'sess-A', prLinks: [], commits: [] }), // loser: retraction candidate + makeRecord({ sessionId: 'sess-B', prLinks: [], commits: [commit] }), // winner + ]) + + // A re-emits with commit_count 0 (retraction), B emits session + commit + const kinds = push2.unsent.map(i => `${i.sessionId}:${i.kind}`).sort() + expect(kinds).toEqual(['sess-A:session', 'sess-B:commit', 'sess-B:session']) + const retraction = push2.unsent.find(i => i.sessionId === 'sess-A')! + expect(retraction.commitCount).toBe(0) + expect(retraction.dedupKey).not.toBe(push1.unsent.find(i => i.kind === 'session')!.dedupKey) + + // A session that was NEVER sent stays excluded when empty + const neverSent = collectUnsentAttribution([ + makeRecord({ sessionId: 'sess-never', prLinks: [], commits: [] }), + ]) + expect(neverSent.unsent).toEqual([]) + } finally { + mock.server.close() + } + }) + it('does not ledger on server error', async () => { const { sendAttributionBatches } = await import('../src/sync/push.js') const { readLedger } = await import('../src/sync/ledger.js')