dev vouch dev on at. thats about it
atvouch.dev
prevent double commenting by accident
authored by
Luna
and committed by
tangled.org
d557ead6
617885d8
+199
-13
+19
-13
appview/lib/atvouch/tap_handler.ex
+19
-13
appview/lib/atvouch/tap_handler.ex
···
401
401
402
402
comment = Atvouch.Tangled.CommentBuilder.build_comment(author_did, author_handle, maintainer_routes, handle_map)
403
403
404
-
case Atvouch.Tangled.Client.post_comment(repo_handle, repo_name, pull_number, comment) do
405
-
:ok ->
406
-
now = DateTime.utc_now() |> DateTime.to_iso8601()
404
+
# Record the comment first to prevent races between duplicate events
405
+
now = DateTime.utc_now() |> DateTime.to_iso8601()
407
406
408
-
Atvouch.BotComment.create(%{
409
-
repo_at_uri: repo_at_uri,
410
-
pull_number: pull_number,
411
-
author_did: author_did,
412
-
commented_at: now
413
-
})
407
+
case Atvouch.BotComment.create(%{
408
+
repo_at_uri: repo_at_uri,
409
+
pull_number: pull_number,
410
+
author_did: author_did,
411
+
commented_at: now
412
+
}) do
413
+
{:ok, _} ->
414
+
case Atvouch.Tangled.Client.post_comment(repo_handle, repo_name, pull_number, comment) do
415
+
:ok ->
416
+
Logger.info("Posted vouch comment on #{repo_handle}/#{repo_name}##{pull_number}")
417
+
:ok
414
418
415
-
Logger.info("Posted vouch comment on #{repo_handle}/#{repo_name}##{pull_number}")
416
-
:ok
419
+
{:error, reason} ->
420
+
Logger.warning("Failed to post comment on #{repo_handle}/#{repo_name}##{pull_number}: #{inspect(reason)}")
421
+
:ok
422
+
end
417
423
418
-
{:error, reason} ->
419
-
Logger.warning("Failed to post comment on #{repo_handle}/#{repo_name}##{pull_number}: #{inspect(reason)}")
424
+
{:error, %Ecto.Changeset{} = changeset} ->
425
+
Logger.debug("Comment already recorded for #{repo_handle}/#{repo_name}##{pull_number}, skipping (#{inspect(changeset.errors)})")
420
426
:ok
421
427
end
422
428
end
+180
appview/test/atvouch/pull_handler_test.exs
+180
appview/test/atvouch/pull_handler_test.exs
···
433
433
refute_receive {:tangled_comment, _}, 1_000
434
434
end
435
435
436
+
test "duplicate race: pre-existing bot_comment record prevents crash and duplicate comment", %{
437
+
tap_port: tap_port,
438
+
state_agent: state_agent
439
+
} do
440
+
repo_at_uri = setup_vouch_graph()
441
+
442
+
# Pre-insert a BotComment to simulate a race where another process already recorded it
443
+
{:ok, _} =
444
+
Atvouch.BotComment.create(%{
445
+
repo_at_uri: repo_at_uri,
446
+
pull_number: 1,
447
+
author_did: "did:plc:author",
448
+
commented_at: "2026-03-19T10:00:00Z"
449
+
})
450
+
451
+
Atvouch.Test.FakeTangledServer.set_pulls_html(
452
+
state_agent,
453
+
"repoowner.test",
454
+
"testrepo",
455
+
pulls_html("repoowner.test", "testrepo")
456
+
)
457
+
458
+
{:ok, _pid} =
459
+
Atvouch.Tap.Socket.start_link(
460
+
uri: "ws://localhost:#{tap_port}/channel",
461
+
handler: Atvouch.TapHandler,
462
+
password: "123",
463
+
name: :"pull_race_test_#{tap_port}"
464
+
)
465
+
466
+
assert_receive {:ws_connected, ws_pid}, 5_000
467
+
468
+
Atvouch.Test.FakeTapServer.send_event(ws_pid, %{
469
+
"id" => 720,
470
+
"type" => "record",
471
+
"record" => %{
472
+
"action" => "create",
473
+
"did" => "did:plc:author",
474
+
"rev" => "3mgmqjki6sz2n",
475
+
"collection" => "sh.tangled.repo.pull",
476
+
"rkey" => "3pullrace",
477
+
"record" => %{
478
+
"$type" => "sh.tangled.repo.pull",
479
+
"title" => "Race condition PR",
480
+
"createdAt" => "2026-03-19T10:00:00Z",
481
+
"target" => %{
482
+
"repo" => repo_at_uri,
483
+
"branch" => "main"
484
+
}
485
+
},
486
+
"cid" => "bafypullrace",
487
+
"live" => true
488
+
}
489
+
})
490
+
491
+
# Should process without crashing
492
+
assert_receive {:ws_message, %{"type" => "ack", "id" => 720}}, 10_000
493
+
494
+
# Should NOT post a comment since it's already recorded
495
+
refute_receive {:tangled_comment, _}, 1_000
496
+
end
497
+
498
+
test "same pull number on different repos each get their own comment", %{
499
+
tap_port: tap_port,
500
+
state_agent: state_agent
501
+
} do
502
+
# Set up first repo (reuses setup_vouch_graph)
503
+
repo_at_uri_1 = setup_vouch_graph()
504
+
505
+
# Set up second repo with its own membership
506
+
{:ok, _} = Atvouch.Identity.create(%{did: "did:plc:repoowner2", handle: "repoowner2.test"})
507
+
508
+
repo_at_uri_2 = "at://did:plc:repoowner2/sh.tangled.repo/3def456"
509
+
510
+
{:ok, _} =
511
+
Atvouch.Membership.create(
512
+
%{
513
+
repo_at_uri: repo_at_uri_2,
514
+
source_did: "did:plc:repoowner2",
515
+
repo_did: "did:plc:repoowner2",
516
+
repo_name: "otherrepo",
517
+
remote_created_at: "2026-03-01T00:00:00Z",
518
+
received_at: "2026-03-01T00:00:00Z"
519
+
},
520
+
["did:plc:maintainer1"]
521
+
)
522
+
523
+
# Set up pulls pages for both repos
524
+
Atvouch.Test.FakeTangledServer.set_pulls_html(
525
+
state_agent,
526
+
"repoowner.test",
527
+
"testrepo",
528
+
pulls_html("repoowner.test", "testrepo")
529
+
)
530
+
531
+
Atvouch.Test.FakeTangledServer.set_pulls_html(
532
+
state_agent,
533
+
"repoowner2.test",
534
+
"otherrepo",
535
+
pulls_html("repoowner2.test", "otherrepo")
536
+
)
537
+
538
+
{:ok, _pid} =
539
+
Atvouch.Tap.Socket.start_link(
540
+
uri: "ws://localhost:#{tap_port}/channel",
541
+
handler: Atvouch.TapHandler,
542
+
password: "123",
543
+
name: :"pull_multi_repo_test_#{tap_port}"
544
+
)
545
+
546
+
assert_receive {:ws_connected, ws_pid}, 5_000
547
+
548
+
# Send pull event for first repo
549
+
Atvouch.Test.FakeTapServer.send_event(ws_pid, %{
550
+
"id" => 730,
551
+
"type" => "record",
552
+
"record" => %{
553
+
"action" => "create",
554
+
"did" => "did:plc:author",
555
+
"rev" => "3mgmqjki6sz2n",
556
+
"collection" => "sh.tangled.repo.pull",
557
+
"rkey" => "3pullmulti1",
558
+
"record" => %{
559
+
"$type" => "sh.tangled.repo.pull",
560
+
"title" => "PR on repo 1",
561
+
"createdAt" => "2026-03-19T10:00:00Z",
562
+
"target" => %{
563
+
"repo" => repo_at_uri_1,
564
+
"branch" => "main"
565
+
}
566
+
},
567
+
"cid" => "bafypullmulti1",
568
+
"live" => true
569
+
}
570
+
})
571
+
572
+
assert_receive {:ws_message, %{"type" => "ack", "id" => 730}}, 10_000
573
+
assert_receive {:tangled_comment, comment1}, 10_000
574
+
assert comment1.handle == "repoowner.test"
575
+
assert comment1.rkey == "testrepo"
576
+
assert comment1.number == 1
577
+
578
+
# Drain
579
+
drain_messages()
580
+
581
+
# Send pull event for second repo (same pull number #1)
582
+
Atvouch.Test.FakeTapServer.send_event(ws_pid, %{
583
+
"id" => 731,
584
+
"type" => "record",
585
+
"record" => %{
586
+
"action" => "create",
587
+
"did" => "did:plc:author",
588
+
"rev" => "3mgmqjki6sz2n",
589
+
"collection" => "sh.tangled.repo.pull",
590
+
"rkey" => "3pullmulti2",
591
+
"record" => %{
592
+
"$type" => "sh.tangled.repo.pull",
593
+
"title" => "PR on repo 2",
594
+
"createdAt" => "2026-03-19T10:00:00Z",
595
+
"target" => %{
596
+
"repo" => repo_at_uri_2,
597
+
"branch" => "main"
598
+
}
599
+
},
600
+
"cid" => "bafypullmulti2",
601
+
"live" => true
602
+
}
603
+
})
604
+
605
+
assert_receive {:ws_message, %{"type" => "ack", "id" => 731}}, 10_000
606
+
assert_receive {:tangled_comment, comment2}, 10_000
607
+
assert comment2.handle == "repoowner2.test"
608
+
assert comment2.rkey == "otherrepo"
609
+
assert comment2.number == 1
610
+
611
+
# Both repos should have their own dedup records
612
+
assert Atvouch.BotComment.exists?(repo_at_uri_1, 1)
613
+
assert Atvouch.BotComment.exists?(repo_at_uri_2, 1)
614
+
end
615
+
436
616
defp drain_messages do
437
617
receive do
438
618
_ -> drain_messages()