fix(sync): flush card PATCHes before the connection and dependency writes

agilesync/sync.py

@@ -30,8 +30,8 @@
 from agilesync.syncers import intake
 from agilesync.syncers import vetting_latch
 from agilesync.syncers.card_coherence import (contested_cards, fence_cid_index, fence_run_indices,
-                            filter_poisoned_edges, laneid_op_value, lane_conflict,
-                            poisoned_card_ids, same_card)
+                            filter_poisoned_edges, same_card)
+from agilesync.syncers.card_ops import CardOpQueue
 from agilesync.syncers.comment_sync import sync_comments
 from agilesync.config import STATE_FILE, env_config
 from agilesync.syncers.description_sync import sync_description
@@ -128,7 +128,7 @@ def sync_child_connections(cfg: dict, apply: bool, epics: list[dict], card_for,
                            by_number: dict, poisoned: frozenset[str],
                            managed_card_ids: set[str],
                            sub_issues: dict[int, list[int]] | None = None) -> None:
-    """Mirror GitHub sub-issues as native AgilePlace parent/child card connections (step 3):
+    """Mirror GitHub sub-issues as native AgilePlace parent/child card connections (step 4):
     authoritative native reads reconcile exactly (additions and removals); the [KEY] title-key
     fallback is add-only, because a heuristic must never authorize destructive reconciliation.
 
@@ -658,33 +658,16 @@ def card_for(issue):
         return _matching_card(issue, card_by_url, card_by_cid)
 
     # Issue #99: one bounded concurrent read phase completes the matched card snapshots; blocked_by
-    # resolves first (step 4 consumes it) so an unusable snapshot never prefetches dependency reads.
+    # resolves first (step 5 consumes it) so an unusable snapshot never prefetches dependency reads.
     blocked_by = ghkit_snapshot.resolve_blocked_by(
         cfg, graph, online, [i["number"] for i in syncable_issues])
     board_reads.hydrate_run_reads(cfg, online, syncable_issues, card_for, epics,
                                   prefetch_deps=blocked_by is not None)
 
-    card_ops: dict = {}
-
-    def queue(card, ops, note):
-        # Issue #70 Layer 2: two queue() calls for the same card can carry conflicting /laneId
-        # values (e.g. duplicate [KEY]-prefixed issue titles matching the same card through the
-        # customId fallback within one run). Detect and poison the entry rather than risk one
-        # issue's lane move clobbering another's -- the poisoned entry is skipped wholesale at
-        # flush (below), never partially applied.
-        cid = str(card["id"])
-        entry = card_ops.setdefault(
-            cid, {"card": card, "ops": [], "notes": [], "lane_id": None, "poisoned": False})
-        new_lane_id, conflict = lane_conflict(ops, entry["lane_id"])
-        if conflict:
-            entry["poisoned"] = True
-            conflicting_value = laneid_op_value(ops)
-            print(f"WARN  card {cid} poisoned: conflicting /laneId ops "
-                  f"({entry['lane_id']!r} vs {conflicting_value!r})")
-        else:
-            entry["lane_id"] = new_lane_id
-        entry["ops"].extend(ops)
-        entry["notes"].append(note)
+    # The run's card-op accumulator: every card-field mutation batches into ONE versioned PATCH
+    # (see card_ops.CardOpQueue for the batching, poisoning and flush-ordering contract).
+    card_op_queue = CardOpQueue()
+    queue = card_op_queue.queue
 
     # Retired issues (see _retire_matched_issues for the full contract).
     _retire_matched_issues(retired_issues, retired_card_by_url, all_card_by_cid, contested,
@@ -739,13 +722,20 @@ def queue(card, ops, note):
         sync_description(cfg, apply, issue, card, issues_state, queue)
         card_types.sync_card_type(cfg, apply, issue, card, resolved.by_name, issues_state, queue)
 
-    # 3) parent/child connections (see sync_child_connections for the full contract).
-    poisoned = poisoned_card_ids(card_ops)
+    # 3) flush: ONE versioned PATCH per card. Issue #107: this precedes the edge steps below
+    # because their POSTs bump each card's resource version, staling the version its own flush is
+    # about to send -- see card_ops.CardOpQueue's docstring. Nothing below reads what the flush
+    # writes (they reconcile edges, not card fields), and both of their inputs are computable the
+    # moment step 2 is done.
+    poisoned = card_op_queue.poisoned_ids()
     managed_card_ids = _managed_card_ids(syncable_issues, card_for, retired_card_by_url)
+    card_op_queue.flush(cfg, apply)
+
+    # 4) parent/child connections (see sync_child_connections for the full contract).
     sync_child_connections(cfg, apply, epics, card_for, by_key, by_number, poisoned, managed_card_ids,
                            sub_issues=graph.sub_issues if graph else None)
 
-    # 4) GitHub blocked-by edges -> native card dependencies (issue #57) -- all edges, managed
+    # 5) GitHub blocked-by edges -> native card dependencies (issue #57) -- all edges, managed
     # pairs only, retired Done blockers resolving through their URL-owned cards. Skip entirely
     # unless the whole blocked-by snapshot is complete. The card Blocked flag belongs to humans:
     # the sync never writes /isBlocked or /blockReason (the old flag-text mirror was retired in
@@ -762,21 +752,16 @@ def queue(card, ops, note):
                           _removal_authority_card_ids(syncable_issues, card_by_url, retired_card_by_url),
                           poisoned)
 
-    # 5) flush: ONE versioned PATCH per card (optimistic concurrency)
-    for entry in card_ops.values():
-        if entry["poisoned"]:
-            continue  # Issue #70 Layer 2: conflicting /laneId ops -- discard, don't half-apply
-        agileplace.patch_card(cfg, apply, entry["card"], entry["ops"], "; ".join(entry["notes"]))
-
     # 6) comment sync (issue #66). Unlike every queued op above, sync_comments writes to GitHub AND
     # AgilePlace IMMEDIATELY (its comment endpoints aren't part of the card-PATCH queue), so an
     # applied comment write can't be rolled back by the poisoned-card hold below. Run it only when
     # this run will actually persist the resulting ledger -- a dry run (preview only, no writes/state)
     # or a CLEAN apply -- never on a poisoned apply where save_state is skipped: otherwise one issue's
     # poisoned card would strand ANOTHER issue's applied comment writes with no ledger, re-mirroring
     # them next run (issue #66 Codex P1 #4). Deferred here, after the flush, so it shares the atomic
-    # save's own gate.
-    clean = not any(entry["poisoned"] for entry in card_ops.values())
+    # save's own gate -- and, since issue #107, because a comment write bumps the card's resource
+    # version too: ahead of the flush it would stale every queued PATCH.
+    clean = card_op_queue.clean
     if not apply or clean:
         for issue in syncable_issues:
             card = card_for(issue)

agilesync/syncers/card_ops.py

@@ -0,0 +1,71 @@
+"""The run's per-card op queue: accumulate, poison, flush (issues #70, #107).
+
+Every mutation one run makes to a card's own fields is batched into a SINGLE versioned PATCH --
+that batching is what keeps a card's resource version from going stale between two writes of its
+own. This module owns that accumulator end to end: the `queue` callable every syncer is handed, the
+lane-conflict poisoning that guards it, and the one flush that sends it. sync.main() keeps the
+ordering decision (WHEN to flush); the shape of the queue lives here, so no caller has to reach
+into an entry's internals.
+
+WHERE THE FLUSH BELONGS IN A RUN (issue #107). Every AgilePlace write bumps a card's resource
+version by exactly 1 -- card PATCHes, connection POSTs, dependency POSTs and comment writes alike
+(measured live 2026-07-30, see docs/API-VALIDATION.md). Each queued PATCH carries the version from
+that card's own run snapshot, so ANY other write the run makes to that card first is a conflict the
+run inflicts on itself: a failed PATCH, a refetch and a retry, reported under a "version bumped by
+an unrelated change" note that is not true. So the flush runs BEFORE the connection/dependency
+steps, and the comment sync -- which bumps the version too -- stays after it. Prevention over
+recovery, the same reasoning as intake._card_for_link_write. The conflict retry (issue #105) stays
+the safety net for genuine concurrent edits by humans, which no ordering can prevent.
+
+Poisoning (issue #70 Layer 2) is unchanged by that ordering: an entry whose queued ops disagree
+about /laneId is skipped WHOLESALE at flush, never half-applied, and its card id is what
+`poisoned_ids()` reports to the connection/dependency steps so they leave that card's edges alone.
+
+Run: pytest -q
+"""
+from __future__ import annotations
+
+from agilesync.board import agileplace
+from agilesync.syncers.card_coherence import lane_conflict, laneid_op_value, poisoned_card_ids
+
+
+class CardOpQueue:
+    """One run's card-op accumulator, keyed by card id. Not thread-safe: the run queues serially."""
+
+    def __init__(self) -> None:
+        self.entries: dict[str, dict] = {}
+
+    def queue(self, card: dict, ops: list[dict], note: str) -> None:
+        """Add `ops` to this card's batch. Two queue() calls for the same card can carry conflicting
+        /laneId values (e.g. duplicate [KEY]-prefixed issue titles matching the same card through the
+        customId fallback within one run). Detect and poison the entry rather than risk one issue's
+        lane move clobbering another's -- the poisoned entry is skipped wholesale at flush."""
+        cid = str(card["id"])
+        entry = self.entries.setdefault(
+            cid, {"card": card, "ops": [], "notes": [], "lane_id": None, "poisoned": False})
+        new_lane_id, conflict = lane_conflict(ops, entry["lane_id"])
+        if conflict:
+            entry["poisoned"] = True
+            print(f"WARN  card {cid} poisoned: conflicting /laneId ops "
+                  f"({entry['lane_id']!r} vs {laneid_op_value(ops)!r})")
+        else:
+            entry["lane_id"] = new_lane_id
+        entry["ops"].extend(ops)
+        entry["notes"].append(note)
+
+    def poisoned_ids(self) -> frozenset[str]:
+        """Card ids whose queued ops conflict -- the edge steps must not touch these cards."""
+        return poisoned_card_ids(self.entries)
+
+    @property
+    def clean(self) -> bool:
+        """No card was poisoned this run, i.e. every queued op reached its PATCH."""
+        return not any(entry["poisoned"] for entry in self.entries.values())
+
+    def flush(self, cfg: dict, apply: bool) -> None:
+        """ONE versioned PATCH per card (optimistic concurrency). See this module's docstring for
+        why this must run before the run's connection/dependency writes."""
+        for entry in self.entries.values():
+            if entry["poisoned"]:
+                continue  # Issue #70 Layer 2: conflicting /laneId ops -- discard, don't half-apply
+            agileplace.patch_card(cfg, apply, entry["card"], entry["ops"], "; ".join(entry["notes"]))

docs/API-VALIDATION.md

@@ -530,12 +530,25 @@ AgilePlace PATCH /card/<id> failed: HTTP 428 ... {"op":"test","path":"/version",
   concurrency guard would buy one saved abort at the cost of an `agileplace` -> `markup` dependency
   and HTML parsing on the conflict path, with a lost-update as the failure mode if it ever
   over-normalized. Fail-closed and self-healing beats clever here.
-- **UNCONFIRMED (what bumped the version):** the run's own step 4 (`POST /card/dependency`) is the
-  only write that touched that card between its snapshot and the flush, which points at dependency
-  creation bumping the card's resource version -- but no live probe has measured a card's `version`
-  across a dependency POST, and an asynchronous server-side bump shortly after create is not ruled
-  out. If dependency/connection writes do bump it, the run is inflicting a conflict on itself every
-  time a card gains a dependency AND carries queued ops, and flushing before those writes (or
-  refetching the version for the affected cards) would avoid the wasted round trip -- the same
-  prevention-over-recovery reasoning as `intake._card_for_link_write`. Tracked separately; the
-  retry above is what keeps such a run correct meanwhile.
+- **CONFIRMED (what bumped the version), issue #107: EVERY write bumps a card's resource version
+  by exactly 1** -- card PATCHes, connection POSTs, dependency POSTs and comment writes alike.
+  Measured by arithmetic over a full live `python -m agilesync.tools.smoke` run (2026-07-30): the
+  throwaway parent card was created at version 1 (the create response carries no version) and step
+  21 read `actualValue: 15` after exactly 14 writes to it -- 7 PATCHes (steps 3, 4, 5, 6, 13, 14,
+  19), 2 connection POSTs (steps 9, 10), 2 dependency POSTs (steps 11, 12 -- the duplicate-create
+  409 wrote nothing) and 3 comment writes (steps 15, 17, 18). `1 + 14 = 15` exactly: any write that
+  did not bump would leave the total short, and the alternative explanation -- an asynchronous
+  server-side bump shortly after create -- would overshoot, so it is ruled out.
+  - **Residual caveat (still worth one cheap probe):** this is a TOTAL, not a per-call measurement.
+    It holds unless one write bumps twice while another does not bump at all. The direct probe
+    (read `version`, one `POST /card/dependency`, read `version`) costs two GETs and belongs in
+    `smoke.py`, which is blocked on the issue #108 extraction -- until it runs, treat the per-call
+    attribution as inferred from the total rather than individually measured.
+- **What that changed (issue #107):** the run was inflicting the conflict on itself. `sync.main()`
+  now flushes its card PATCHes BEFORE the child-connection and dependency steps, so no card is
+  written between its own snapshot and its own flush -- prevention over recovery, the same
+  reasoning as `intake._card_for_link_write`. Comment sync bumps the version too, so it stays
+  AFTER the flush (where it already sat, for its own state-gate reason). The retry above remains
+  the safety net for genuine concurrent edits by humans, which no ordering can prevent.
+  `tests/test_sync_flush_order.py` pins the ordering as an invariant; `tests/test_run.py` pins the
+  resulting write sequence at the HTTP boundary.

tests/test_run.py

@@ -346,8 +346,12 @@ def test_new_card_dry_run_plans_every_action_apply_executes(paired_runs):
     assert dry.process_writes == ()
     assert not dry.state_file.exists()
     assert Counter(_planned_actions(dry.output)) == Counter(_executed_actions(apply))
+    # Issue #107: the card PATCH precedes the connection/dependency POSTs, which bump the card's
+    # resource version -- flushing after them staled the version the PATCH itself carries. This is
+    # the same ordering tests/test_sync_flush_order.py pins as an invariant, seen here at the real
+    # HTTP boundary.
     assert [action[0] for action in _planned_actions(dry.output)] == [
-        "create", "gh", "connect", "depend", "patch",
+        "create", "gh", "patch", "connect", "depend",
     ]
     assert PLAN_ID_PREFIX not in json.dumps([
         {"path": write.path, "body": write.body} for write in apply.http_writes

tests/test_sync_comments_call_site.py

@@ -31,6 +31,7 @@
 
 sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
 
+from agilesync.syncers import card_ops  # noqa: E402
 from agilesync.syncers import comment_sync  # noqa: E402
 from agilesync import sync  # noqa: E402
 
@@ -90,7 +91,9 @@ def test_main_defers_comment_sync_and_holds_state_when_a_card_is_poisoned(tmp_pa
     stack.enter_context(patch("agilesync.sync.sync_description"))
     sync_comments_mock = stack.enter_context(patch("agilesync.sync.sync_comments"))
     save_state_mock = stack.enter_context(patch("agilesync.sync.save_state"))
-    monkeypatch.setattr(sync, "lane_conflict", lambda ops, lane_id: (None, True))
+    # The op queue owns the poisoning decision (issue #107 extraction), so lane_conflict is forced
+    # in card_ops' namespace -- main() no longer imports the name itself.
+    monkeypatch.setattr(card_ops, "lane_conflict", lambda ops, lane_id: (None, True))
 
     with stack, patch("agilesync.sync.env_config", return_value=cfg), patch("agilesync.sync.STATE_FILE", state_file), \
          patch("sys.argv", ["sync.py", "--apply"]):