store: subtract what a group lends from a submitted card
A group lends the lines; the client sends them home.
Struck by their bytes, so the member stores its own.
An edit or a delete is seen and left in place,
and two of one name is a guess Sentry gets to face.

Assisted-by: Claude Opus 5 via Claude Code
change xouqukrvlkryvksnkqxxmtkpsvknmyrv
commit b4f61c0c179159259633443819b60a2e35705338
author Alpha Chen <alpha@kejadlen.dev>
date
parent lsotopxn
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index 0e3fe98..945e0a5 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -318,11 +318,11 @@ module ProTacts
     # Matched by the line's bytes, which is the same blindness the
     # editor's digests have (VCard::Parser::Line#digest): a stored
     # line whose bytes are a group's line reads as the group's here.
-    # That state is a member's rewrite carrying an inherited line back
-    # into its own card, which the write half subtracts
-    # (docs/plans/2026-08-24-vcard-storage-and-groups.md); until it
-    # lands, naming the group over both copies says something true
-    # about the value even where it is wrong about the byte.
+    # No write makes that state — Store#subtract_inherited takes the
+    # group's copy back out of a submission — but a member whose own
+    # card spells the line exactly as its group does still has one, and
+    # naming the group over both copies says something true about the
+    # value even where it is wrong about the byte.
     #: (VCard::Parser::Line line) -> String?
     def group_of(line)
       groups_by_line[line.verbatim.chomp]
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 5a2cb12..2420534 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -32,11 +32,11 @@ module ProTacts
   # and #rebuild_index will make it again from nothing.
   #
   # Every Contact this store hands out is composed, never the stored
-  # card alone: a birthday is subtracted out of a card on the way in,
-  # and Contact composes it back in on read, with the lines a contact
-  # inherits from its groups composed in beside it — so the vcard and
-  # the etag a caller sees, and the etag the change log records,
-  # describe the card a client downloads, not the bytes on disk.
+  # card alone: a birthday and the lines a contact inherits from its
+  # groups are both subtracted out of a card on the way in, and Contact
+  # composes them back in on read — so the vcard and the etag a caller
+  # sees, and the etag the change log records, describe the card a
+  # client downloads, not the bytes on disk.
   #
   # Sequel's transactions join one already open rather than failing on
   # SQLite's lack of nesting, which is what lets the group fan-out this
@@ -292,6 +292,12 @@ module ProTacts
       # reported rather than lost in silence.
       card = VCard.new(vcard)
       existing = birthday_of(id)
+      # The card as it stands before this write, read once for the two
+      # halves that need it: the rewrite arm below, which carries
+      # unrendered BDAY lines across, and the subtraction after the
+      # case, which accounts for the member's own lines before
+      # attributing any to a group.
+      own = stored_card(id)
       birthday, stored =
         case card.extract("BDAY")
         in [[line], rest]
@@ -303,7 +309,7 @@ module ProTacts
           # The rewrite arm: carry the unrendered lines out of the
           # stored card, report the unrecognized ones' loss, and keep
           # whatever an unseen model row holds.
-          carried, lost = carried_and_lost_bday_lines(stored_card(id))
+          carried, lost = carried_and_lost_bday_lines(own)
           kept = existing && !existing.served? ? existing : nil
           report_lost_bday_lines(lost)
           [kept, card.insert(carried)]
@@ -314,15 +320,18 @@ module ProTacts
           [nil, card]
         end
 
-      # The Contact this returns is the composed one — what the contact
-      # inherits included, read off membership as it stands at this
-      # write — and the logged etag is its hash, so a client's token
-      # describes the card it downloads. The submitted card is stored
-      # as it arrived: subtracting a group's lines back out of a
-      # member's submission is the write half's task
-      # (docs/plans/2026-08-24-vcard-storage-and-groups.md), so until it
-      # lands a member's rewrite carries the inherited lines within it.
-      contact = Contact.new(id:, stored:, birthday:, inherited: inherited_of(id))
+      # The other half of the split, the same shape as the birthday's:
+      # what the groups lend comes back out of the submission before it
+      # is stored, so a rewrite cannot materialize a group's lines into
+      # the member's own card.
+      #
+      # The Contact this returns is the composed one — those lines put
+      # back, read off membership as it stands at this write — and the
+      # logged etag is its hash, so a client's token describes the card
+      # it downloads.
+      inherited = inherited_of(id)
+      stored = subtract_inherited(stored, inherited, own)
+      contact = Contact.new(id:, stored:, birthday:, inherited:)
       @database.transaction do
         cards
           .insert_conflict(target: :id, update: {vcard: Sequel[:excluded][:vcard], updated_at: NOW})
@@ -573,6 +582,103 @@ module ProTacts
       )
     end
 
+    # The submitted card with the lines its groups lend it taken back
+    # out: served is stored plus inherited, so stored is submitted
+    # minus inherited and a client that PUTs back what it downloaded
+    # stores what it started with
+    # (docs/plans/2026-08-24-vcard-storage-and-groups.md, "Groups
+    # compose into cards"). Without this a member's rewrite would
+    # materialize the group's lines into its own card, and a later edit
+    # to the group would reach nobody.
+    #
+    # Each lent line is classified against the submission, and only one
+    # of the four shapes moves a byte. The line coming back as the same
+    # bytes is the group's, untouched, and it goes. One line of that
+    # name left over is an edit to the shared value and none is a
+    # deletion of it: both stay in the card as they arrived, because
+    # propagating either is the next task's (the plan's "Edits
+    # propagate to the group"), and storing the edit is what keeps it
+    # from being lost meanwhile. More than one left over is a line this
+    # cannot attribute at all, and reports rather than guesses.
+    #
+    # #substitute rather than #extract and #insert, which would move a
+    # member's own lines of the same name to the card's end: the lines
+    # this leaves keep their positions, so a submission that was the
+    # served card round-trips to the bytes it was composed from and the
+    # PUT can answer with a strong etag (RFC 6352 section 6.3.2.3).
+    #: (VCard card, Array[Contact::Inherited] inherited, VCard? own) -> VCard
+    def subtract_inherited(card, inherited, own)
+      return card if inherited.empty?
+
+      unaccounted = unaccounted_lines(card, own)
+      untouched = [] #: Array[VCard::Parser::Line]
+      ambiguous = 0
+
+      inherited.each do |lent|
+        candidates = unaccounted.select { it.names?(property_name(lent.line)) }
+        # Blind to identical bytes the way the editor's digests are
+        # (VCard::Parser::Line#digest): where a member's own card
+        # spells a line exactly as its group does, which copy this
+        # takes is undecidable and the bytes make it not matter.
+        match = candidates.find { it.verbatim.chomp == lent.line.chomp }
+        if match
+          unaccounted.delete_at(
+            unaccounted.index(match) #: Integer
+          )
+          untouched << match
+        elsif candidates.length > 1
+          ambiguous += 1
+        end
+      end
+
+      report_ambiguous_inherited_lines(ambiguous)
+      untouched.reduce(card) { |rest, line| rest.substitute(line.digest, []) }
+    end
+
+    # The submission's lines that the member's own stored card does not
+    # already explain — one struck per stored line of the same bytes,
+    # so a card that stores one of something and submits two leaves one
+    # over. What is left is what the groups lent plus whatever the
+    # client wrote beside it, which is the pool a lent line is
+    # attributed from. Everything for a card being created, which has
+    # no stored lines to explain anything.
+    #: (VCard card, VCard? own) -> Array[VCard::Parser::Line]
+    def unaccounted_lines(card, own)
+      stored = own ? own.lines.map { it.verbatim.chomp } : [] #: Array[String]
+      card.lines.reject { |line|
+        index = stored.index(line.verbatim.chomp)
+        stored.delete_at(index) if index
+        index
+      }
+    end
+
+    # A content line's property name: what stands before its first
+    # parameter or its value (RFC 2426 section 2.1.1). Read off the
+    # bytes rather than parsed, because the only lines asked are a
+    # group's, which carry no `item1.` prefix to strip — the schema
+    # refuses one (db/migrations/004_groups.rb).
+    #: (String line) -> String
+    def property_name(line)
+      line[/\A[^;:]*/].to_s
+    end
+
+    # The ambiguity report: a lent line that came back as neither its
+    # own bytes nor a single candidate is one this server cannot
+    # attribute — two lines of that name arrived that the member's card
+    # does not explain, and calling either the edit would be a guess.
+    # The card is stored as it arrived and the line is news, the same
+    # bargain report_unrecognized_bday_lines makes, and carries no card
+    # content for the same reason (ProTacts::SentryScrubber).
+    #: (Integer count) -> void
+    def report_ambiguous_inherited_lines(count)
+      return if count.zero?
+
+      Sentry.capture_message(
+        "a submitted card left #{count} inherited line(s) with more than one line of that name to attribute them to",
+        level: :warning,
+      )
+    end
+
     # The loss report, the rewrite's half of the arrival one: a stored
     # BDAY no client renders and no whitelist recognizes is about to be
     # dropped, and nobody would know.
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 6969e4a..10ed86d 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -670,14 +670,17 @@ module ProTacts
 
       stored = store.put(id, vcard)
       response.status = existing ? 204 : 201
-      # A strong ETag belongs on the answer only when what was stored is
-      # the submitted bytes, octet for octet — the one case RFC 6352
-      # section 6.3.2.3 lets a client rely on the tag it gets back, and
-      # anywhere else it forbids one outright. A birthday is subtracted
-      # from the card before storage and composed back in on read, so a
-      # PUT that carried one stores a different card than it was handed
-      # and the client refetches; a card with nothing to subtract still
-      # stores octet for octet and still gets the tag.
+      # A strong ETag belongs on the answer only when what the resource
+      # now serves is the submitted bytes, octet for octet — the one
+      # case RFC 6352 section 6.3.2.3 lets a client rely on the tag it
+      # gets back, and anywhere else it forbids one outright. Which is
+      # the composed card against the body, not the stored one: a
+      # birthday and a group's lines are both subtracted before storage
+      # and composed back in on read, so a member returning the card it
+      # downloaded stores fewer bytes than it sent and still gets the
+      # tag, while a PUT that carried a birthday somewhere other than
+      # where compose puts it back serves bytes that are not the ones
+      # it sent, and the client refetches.
       response["ETag"] = stored.etag if stored.vcard.to_s == vcard
 
       # A returned "" would land in the body and pin text/html and
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index 6a6930d..bd172f4 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1168,6 +1168,137 @@ class StoreTest < Minitest::Test
     end
   end
 
+  ## Subtracting what a group lends
+
+  # The member's own address, deliberately unlike the group's so a
+  # stored line and a lent one are told apart by their bytes.
+  OWN_ADDRESS = "ADR:;;1 Long Road;;;;" #: String
+  # The group's address as a client sends it back changed — one digit,
+  # so nothing but the value moved.
+  EDITED_ADDRESS = HOUSEHOLD_ADDRESS.sub("7 Calculus", "8 Calculus") #: String
+
+  # The round trip the whole composition rests on: a member PUTs back
+  # what it downloaded, and what is stored is the card it was composed
+  # from — the group's lines in the served card, in the index nowhere
+  # (docs/plans/2026-08-24-vcard-storage-and-groups.md).
+  def test_a_members_put_of_its_served_card_stores_what_it_started_with
+    with_store({"aiden" => AIDEN}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS, HOUSEHOLD_NOTE])
+      served = store.contact("aiden").vcard.to_s
+
+      contact = store.put("aiden", served)
+
+      assert_equal AIDEN, card_row(store, "aiden").fetch(:vcard)
+      assert_equal served, contact.vcard.to_s
+      refute_includes indexed_names(store, "aiden"), "ADR"
+    end
+  end
+
+  # Both subtractions in one write, and the composition puts them back
+  # in the order it always does: the group's lines, then the birthday.
+  def test_a_put_subtracts_the_birthday_and_the_lent_lines_together
+    born = AIDEN.sub("FN:Aiden\r\n", "FN:Aiden\r\nBDAY:1985-12-10\r\n")
+
+    with_store({"aiden" => born}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+
+      store.put("aiden", served)
+
+      assert_equal AIDEN, card_row(store, "aiden").fetch(:vcard)
+      assert_equal served, store.contact("aiden").vcard.to_s
+    end
+  end
+
+  # A member's own line of the name its group lends keeps its place in
+  # the card: the subtraction takes the lent copy and moves nothing
+  # else, which is what lets the round trip hold for a member that
+  # carries an address of its own.
+  def test_a_members_own_line_survives_the_subtraction_in_place
+    own = AIDEN.sub("FN:Aiden\r\n", "#{OWN_ADDRESS}\r\nFN:Aiden\r\n")
+
+    with_store({"aiden" => own}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+
+      store.put("aiden", served)
+
+      assert_equal own, card_row(store, "aiden").fetch(:vcard)
+      assert_equal served, store.contact("aiden").vcard.to_s
+    end
+  end
+
+  # An edit to a shared value is detected and left where it landed:
+  # the member's card keeps it, the group keeps its own, and the
+  # member serves both until the propagation task takes the edit to
+  # the group (the plan's "Edits propagate to the group").
+  def test_an_edited_lent_line_stays_in_the_members_own_card
+    with_store({"aiden" => AIDEN}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+
+      store.put("aiden", served.sub(HOUSEHOLD_ADDRESS, EDITED_ADDRESS))
+
+      assert_includes card_row(store, "aiden").fetch(:vcard), EDITED_ADDRESS
+      composed = store.contact("aiden").vcard.to_s
+      assert_includes composed, EDITED_ADDRESS
+      assert_includes composed, HOUSEHOLD_ADDRESS
+      assert_empty sentry_messages
+    end
+  end
+
+  # A deletion is detected and takes nothing with it: the group still
+  # lends the line, so the next read composes it back in. Whether a
+  # delete should reach the group at all is the plan's open question,
+  # and nothing here decides it.
+  def test_a_deleted_lent_line_comes_straight_back
+    with_store({"aiden" => AIDEN}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+
+      store.put("aiden", served.sub("#{HOUSEHOLD_ADDRESS}\r\n", ""))
+
+      assert_equal AIDEN, card_row(store, "aiden").fetch(:vcard)
+      assert_equal served, store.contact("aiden").vcard.to_s
+      assert_empty sentry_messages
+    end
+  end
+
+  # Two lines of the lent name, neither of them its bytes: the edit
+  # cannot be told from the addition, so the card is stored as it
+  # arrived and the ambiguity is reported rather than guessed at.
+  def test_two_candidates_for_one_lent_line_are_stored_and_reported
+    with_store({"aiden" => AIDEN}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+      submitted = served.sub(HOUSEHOLD_ADDRESS, "#{EDITED_ADDRESS}\r\n#{OWN_ADDRESS}")
+
+      store.put("aiden", submitted)
+
+      assert_equal submitted, card_row(store, "aiden").fetch(:vcard)
+      assert_equal 1, sentry_messages.length
+    end
+  end
+
+  # A member whose own card spells a line exactly as its group does
+  # keeps one copy of it: the stored lines are struck against the
+  # submission before anything is attributed to a group, so what is
+  # left over is the lent copy and only that one goes.
+  def test_a_line_the_member_and_its_group_both_spell_keeps_one_copy
+    own = AIDEN.sub("END:VCARD\r\n", "#{HOUSEHOLD_ADDRESS}\r\nEND:VCARD\r\n")
+
+    with_store({"aiden" => own}) do |store|
+      add_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      served = store.contact("aiden").vcard.to_s
+      assert_equal 2, served.scan(HOUSEHOLD_ADDRESS).length
+
+      store.put("aiden", served)
+
+      assert_equal own, card_row(store, "aiden").fetch(:vcard)
+      assert_equal served, store.contact("aiden").vcard.to_s
+    end
+  end
+
   ## The migration
 
   # A database at the old action vocabulary — put and delete only — is
diff --git a/test/pro_tacts/test_web.rb b/test/pro_tacts/test_web.rb
index a2004e9..0fddd86 100644
--- a/test/pro_tacts/test_web.rb
+++ b/test/pro_tacts/test_web.rb
@@ -336,6 +336,31 @@ class WebTest < Minitest::Test
     end
   end
 
+  # The same rule where a group lends the member a line: what comes
+  # back is subtracted before storage and composed again on read, so
+  # the resource serves the submitted bytes exactly and the tag is
+  # honest — even though what was stored is fewer bytes than arrived
+  # (Store#subtract_inherited).
+  def test_a_members_put_of_its_served_card_keeps_the_strong_etag
+    address = "ADR;TYPE=home:;;7 Calculus Close;London;England;NW1 1AB;United Kingdom"
+
+    with_contacts({"aiden" => "Aiden"}) do |store|
+      lend(store, "aiden", address)
+
+      get "/dav/addressbook/aiden.vcf"
+      served = last_response.body
+      assert_includes served, address
+
+      put_request "aiden", served, "CONTENT_TYPE" => VCARD
+
+      assert_equal 204, last_response.status
+      assert_equal %("#{Digest::SHA256.hexdigest(served)}"), last_response["ETag"]
+
+      get "/dav/addressbook/aiden.vcf"
+      assert_equal served, last_response.body
+    end
+  end
+
   # The same omission for the rewrite that carries a birthday across:
   # the stored card is the submission plus the BDAY line macOS dropped,
   # so the answer cannot claim the tag — the client refetches, and the
@@ -710,6 +735,17 @@ class WebTest < Minitest::Test
     end
   end
 
+  # Puts a card in a group of its own that lends it one line. A group's
+  # properties and members have no write path yet — authoring is the
+  # admin UI's task — so they land through the store's own database,
+  # the way the fixture seeder's do (test/fixture_data.rb).
+  def lend(store, id, line)
+    database = store.instance_variable_get(:@database)
+    group = store.create_group(name: "Booles")
+    database[:group_properties].insert(group_id: group, position: 0, line:)
+    database[:group_members].insert(group_id: group, card_id: id)
+  end
+
   def etag_only_propfind
     <<~XML
       <?xml version="1.0" encoding="UTF-8"?>