Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
handleIssue,
handlePRReviewComment,
} from '@/lib/integrations/platforms/github/webhook-handlers';
import { upsertSessionPullRequestsFromWebhook } from '@/lib/integrations/platforms/github/webhook-handlers/upsert-session-pull-request';
import { PLATFORM, GITHUB_EVENT, GITHUB_ACTION } from '@/lib/integrations/core/constants';
import { logExceptInTest } from '@/lib/utils.server';
import { logWebhookEvent, updateWebhookEvent } from '@/lib/integrations/db/webhook-events';
Expand Down Expand Up @@ -391,7 +392,14 @@ export async function handleGitHubWebhook(

const action = parseResult.data.action;

// Filter out closed events - we don't log or process them
// Side-effect: keep cli_session_pull_requests in sync with the PR's head
// repo + branch. Runs BEFORE the closed-event early-return so that
// merged/closed state is persisted. Fires for opened / reopened /
// edited / synchronize / closed; no-op for other actions. Errors are
// captured internally and never throw.
await upsertSessionPullRequestsFromWebhook(parseResult.data);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Duplicate webhooks can still repeat this side effect

This upsert now runs before logWebhook, so the existing duplicate-delivery guard is bypassed for pull_request events. Replayed or retried webhooks will execute the database lookup/upsert again and refresh pr_last_synced_at/updated_at even when logWebhook would have returned isDuplicate. If duplicate suppression should continue to apply to all webhook side effects, log/check the delivery before calling upsertSessionPullRequestsFromWebhook, while still placing the closed-event early return after the upsert for non-duplicates.


// Filter out closed events - we don't log or process them further
if (action === GITHUB_ACTION.CLOSED) {
return NextResponse.json({ message: 'Event received' }, { status: 200 });
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { normalizeGitUrl } from './normalize-git-url';

describe('normalizeGitUrl', () => {
it('trims trailing .git from https URLs', () => {
expect(normalizeGitUrl('https://github.com/acme/widgets.git')).toBe(
'https://github.com/acme/widgets'
);
});

it('returns the canonical https form when already canonical', () => {
expect(normalizeGitUrl('https://github.com/acme/widgets')).toBe(
'https://github.com/acme/widgets'
);
});

it('converts git@github.com: ssh form into https', () => {
expect(normalizeGitUrl('git@github.com:acme/widgets.git')).toBe(
'https://github.com/acme/widgets'
);
});

it('converts ssh form without .git suffix', () => {
expect(normalizeGitUrl('git@github.com:acme/widgets')).toBe('https://github.com/acme/widgets');
});

it('lower-cases host and owner/repo path', () => {
expect(normalizeGitUrl('https://GitHub.com/ACME/Widgets.git')).toBe(
'https://github.com/acme/widgets'
);
});

it('treats https and ssh variants of the same repo as equal', () => {
const a = normalizeGitUrl('https://github.com/acme/widgets.git');
const b = normalizeGitUrl('git@github.com:acme/widgets.git');
const c = normalizeGitUrl('https://github.com/acme/widgets');
expect(a).toBe(b);
expect(a).toBe(c);
});

it('strips a trailing slash', () => {
expect(normalizeGitUrl('https://github.com/acme/widgets/')).toBe(
'https://github.com/acme/widgets'
);
});

it('returns empty string for empty input', () => {
expect(normalizeGitUrl('')).toBe('');
expect(normalizeGitUrl(' ')).toBe('');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Normalize a git remote URL to a canonical lower-cased https form so that
* equivalent https/ssh/`.git`-suffixed variants of the same repo compare equal.
*
* - Strips a trailing `.git`
* - Lower-cases host + owner/repo path
* - Converts `git@host:owner/repo` ssh form into `https://host/owner/repo`
* - Leaves non-http(s) URLs that it cannot parse as-is (lower-cased, `.git` stripped)
*
* Pure helper; no IO.
*/
export function normalizeGitUrl(url: string): string {
const trimmed = url.trim();
if (trimmed === '') return '';

const sshMatch = trimmed.match(/^git@([^:]+):(.+)$/);
const httpsForm = sshMatch ? `https://${sshMatch[1]}/${sshMatch[2]}` : trimmed;

try {
const parsed = new URL(httpsForm);
const host = parsed.host.toLowerCase();
let pathname = parsed.pathname.toLowerCase();
if (pathname.endsWith('.git')) {
pathname = pathname.slice(0, -'.git'.length);
}
while (pathname.endsWith('/')) {
pathname = pathname.slice(0, -1);
}
return `${parsed.protocol.toLowerCase()}//${host}${pathname}`;
} catch {
const lower = httpsForm.toLowerCase();
return lower.endsWith('.git') ? lower.slice(0, -'.git'.length) : lower;
}
}
Loading