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

[thewrz]

This was written agentically; verify its assertions and edit accordingly:

Why

Every AgilePlace write bumps a card's resource version by exactly 1 — connection POSTs, dependency POSTs and comment writes included, not just card PATCHes (confirmed by arithmetic over the 2026-07-30 live smoke run; see the issue and the amended docs/API-VALIDATION.md section).

sync.main() flushed its queued PATCHes last, after writing each card's edges — so the run staled the very version those PATCHes carry. A card that both gained an edge and carried queued ops ate a guaranteed 409/428, a refetch and a retry, every run, reported under a version bumped by an unrelated change note that was not true: the run bumped it itself. The 2026-07-29 live run hit exactly this on four cards at once (an epic plus its three children).

What

The flush now runs before the child-connection and dependency steps, so no card is written between its own snapshot and its own flush. Nothing in those steps reads what the flush writes, and both of their inputs (poisoned, managed_card_ids) were already computable the moment step 2 finished. Comment sync bumps the version too, so it stays after the flush — where it already sat, for its own state-gate reason.

Issue #105's conflict retry stays exactly as it is: it remains the safety net for genuine concurrent edits by humans, which no ordering can prevent. This removes the conflicts the run inflicted on itself.

The op queue moves to agilesync/syncers/card_ops.py — sync.py was already at the repo's 800-line hard cap, and the queue, its lane-conflict poisoning and its single flush are one cohesive unit, which now carries the ordering contract in its own docstring. Behavior there is unchanged (same WARN text, same wholesale poisoned-card skip, same clean gate).

Not in this PR

• The direct per-call live probe (read version, one POST /card/dependency, read version). The measurement in hand is a total over 14 writes, not per-call; it holds unless one write bumps twice while another does not bump at all. That probe belongs in smoke.py, which is blocked on the smoke.py is at 776/800 lines — extract before the next live probe (blocks the #105 conflict-retry probe) #108 extraction — the doc records it as inferred-from-the-total rather than individually measured, and the caveat is written down there.
• A smoke step for this change, for the same smoke.py is at 776/800 lines — extract before the next live probe (blocks the #105 conflict-retry probe) #108 reason. This reorders existing writes rather than adding a write shape.

Testing

• Unit tests pass — full suite green, 1657 passed (4 new)
• New invariant test tests/test_sync_flush_order.py: no card is written between its snapshot and its flush PATCH, driven through the real main(). Watched it fail first (writes landed connect, dependency, patch, patch)
• tests/test_run.py's end-to-end HTTP-boundary sequence updated to create, gh, patch, connect, depend — the oracle change this requirement drives
• Poisoned-card hold and the comment-sync clean gate unchanged (test_sync_comments_call_site.py, test_sync_lane_conflict.py, test_sync_contested_cards.py green; the poisoning seam's monkeypatch target moved with the extraction)
• Live --apply run shows no version bumped note where none is warranted — needs the maintainer: this checkout has no .env, so I could not run it
• CI green

🤖 Co-authored by Claude Opus 5. Closes #107.

Summary by CodeRabbit

• Improvements

  • Card updates are now batched and applied in a consistent order, reducing version conflicts during synchronization.
  • Conflicting card updates are safely skipped to prevent partial changes.
  • Card updates are completed before connections, dependencies, and comments are synchronized.
  • Comments and saved sync state are only updated after a clean card synchronization.

• Documentation

  • Clarified resource-version behavior and synchronization write ordering.

• Tests

  • Added coverage to verify update ordering and conflict handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 572a4a06-5b10-4104-808a-da32ad6e8da6

📥 Commits

Reviewing files that changed from the base of the PR and between 327a359 and 6a8a90b.

📒 Files selected for processing (6)

• agilesync/sync.py
• agilesync/syncers/card_ops.py
• docs/API-VALIDATION.md
• tests/test_run.py
• tests/test_sync_comments_call_site.py
• tests/test_sync_flush_order.py

---

📝 Walkthrough

Walkthrough

Card mutations now use CardOpQueue for batching, lane-conflict poisoning, and single-card PATCHes. sync.main() flushes patches before connection and dependency writes, then gates comments and state persistence on queue cleanliness. Tests and validation docs record the updated ordering and version behavior.

Changes

Card synchronization ordering

Layer / File(s)	Summary
Card operation queue agilesync/syncers/card_ops.py	Adds per-card mutation batching, lane-conflict poisoning, poisoned-card tracking, clean-state reporting, and one PATCH per non-poisoned card.
Sync orchestration integration agilesync/sync.py	Replaces local mutation bookkeeping with CardOpQueue, flushes patches before edge synchronization, and uses queue cleanliness for comment and state gating.
Ordering validation and documentation tests/test_sync_flush_order.py, tests/test_run.py, tests/test_sync_comments_call_site.py, docs/API-VALIDATION.md	Pins PATCH-before-edge/comment ordering, updates poisoned-card patching, and documents measured resource-version behavior.

Sequence Diagram(s)

sequenceDiagram
  participant sync.main
  participant CardOpQueue
  participant AgilePlace
  participant EdgeSync
  participant CommentSync
  sync.main->>CardOpQueue: queue card mutations
  sync.main->>CardOpQueue: flush queued patches
  CardOpQueue->>AgilePlace: PATCH cards
  sync.main->>EdgeSync: sync connections and dependencies
  sync.main->>CommentSync: sync comments when clean

Loading

Possibly related PRs

• wrzonance/agile-github-sync#74: Earlier poison and queued-operation handling that this PR centralizes in CardOpQueue.
• wrzonance/agile-github-sync#80: Related poisoned-card filtering for connection and dependency synchronization.
• wrzonance/agile-github-sync#89: Related ordering and gating of comment synchronization after card PATCH operations.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: flushing card PATCHes before connection and dependency writes.
Linked Issues check	✅ Passed	The PR satisfies #107 by confirming the version bump behavior in docs and moving card PATCH flushes before connection, dependency, and comment writes.
Out of Scope Changes check	✅ Passed	The changes stay scoped to the sync-ordering fix, the queue refactor, and supporting tests/docs needed for #107.

---

_{Comment @coderabbitai help to get the list of available commands.}

[thewrz]

This was written agentically; verify its assertions and edit accordingly:

Adversarial cross-harness review — Codex gpt-5.6-sol, effort xhigh

Run once, read-only, against origin/main (codex exec review --base origin/main). Cross-harness by design: a Claude-authored change reviewed by the other harness.

Verdict: no findings

The extraction preserves the existing batching and poisoning semantics, while the new flush position correctly precedes version-bumping edge and comment writes. The full test suite passes with 1657 tests.

The new queue preserves the prior batching/poisoning behavior, and the reordered flush occurs after all queue producers but before every version-bumping edge/comment write.

No [P1] or [P2] items were raised, so there is nothing to fix, decline, or defer from this pass.

What it actually did (not just read the diff)

• Traced the changed code through its call sites: agilesync/board/agileplace.py's write functions (patch_card, connect_children/disconnect_children, create_dependencies/delete_dependencies, get_card) and comment_sync.sync_comments.
• Diffed the new main() flow against the base version of sync.py side by side, and confirmed the flush now sits after every queue producer and before every version-bumping write.
• Specifically hunted the failure-ordering question this reorder raises — it grepped the suite for connect_children/create_dependencies failure injection and save_state interactions.
• Checked the package layout for the new module (agilesync/syncers/card_ops.py) and read docs/API-VALIDATION.md and smoke.py against the version-bump claim.
• Ran the suite itself: pytest -q -> 1657 passed in 18.29s, independently reproducing the result claimed in the PR body.

Still open, unchanged by this review

Both are recorded in the PR description and neither is a code defect:

1. The live --apply verification still needs a maintainer with tenant credentials — this checkout has no .env.
2. The direct per-call version probe remains blocked on smoke.py is at 776/800 lines — extract before the next live probe (blocks the #105 conflict-retry probe) #108; docs/API-VALIDATION.md records the per-call attribution as inferred from the 14-write total rather than individually measured.

🤖 Co-authored by Claude Opus 5; review performed by Codex gpt-5.6-sol (xhigh).

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.