Skip to content

Commit 0fe1d3b

Browse files
iancooperclaude
andcommitted
docs(spec-0034): R5 design review (PASS) + Option B InMemory pump
Round-5 adversarial design review of ADR 0063: PASS (0 findings >= 60). Empirically confirmed the load-bearing claims — the success-branch orphan exists and the fix re-parents the MarkDispatched span (StartActivity with a null parentId falls back to ambient Activity.Current = S2). Fold in all 5 sub-threshold findings: - Pin InMemory pump surface: UseAsyncPublishConfirmation switch + PublishFailurePredicate hook (property-injection style), single worker, FIFO, unbounded channel, lazy start. - Address SendWithDelay/scheduler interaction (pump engages on Send re-entry). - Add deferred-bus-visibility consequence when the switch is on. - Correct "all four Send methods end in CancellationToken" -> only the 2 async. - Fix C-7 line ref :945 -> per-message async :1006. Adopt Option B for the pump (was single-worker serial): worker writes the bus in FIFO order, then dispatches each OnMessagePublished raise via Task.Run, so same-topic confirmations overlap and AC-10/NFR-3 concurrency is exercised in-process (mirrors Kafka). Trade-off: confirmation ordering non-deterministic (no AC asserts it). Two-stage drain on dispose (await worker, then await in-flight raise tasks via Interlocked count + TaskCompletionSource). Write review-design.md (round 5); refresh PROMPT.md (R5 PASS, R6 owed on the detailed pump). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d8b9308 commit 0fe1d3b

3 files changed

Lines changed: 88 additions & 46 deletions

File tree

PROMPT.md

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,23 @@
44
**Branch:** `issue-4179-failed-delivery-context` · **Spec dir:** `specs/0034-failed-delivery-context/`
55
**Issue:** #4179 · **PR:** #4180 (open)
66

7-
## ✅ STATUS: ADR 0063 committed + expanded; requirements & ADR tidied; **uncommitted working-tree edits** (symmetric decision + clarity pass)
7+
## ✅ STATUS: ADR 0063 design settled through **R5 PASS**; **R6 re-review owed** on the now-detailed InMemory pump. All committed + pushed.
88

99
Spec artifacts only so far; **no production code yet**. Workflow position:
10-
Issue → **Requirements ✅****ADR ✅ (committed, expanded, edited — uncommitted edits live)** ◀ HERE → re-review (R5) → approve → Tasks → Tests → Code.
10+
Issue → **Requirements ✅****ADR ✅ (R5 PASS, findings folded in; R6 owed on pump detail)** ◀ HERE → approve → Tasks → Tests → Code.
1111

1212
| Phase | State |
1313
|---|---|
14-
| Requirements | ✅ Approved (`.requirements-approved`), committed `d7c997512`, pushed, PR #4180. **Edited since (uncommitted)**see below. |
15-
| Design | ✅ ADR 0063 committed `d4dd4d793`, in `.adr-list`. Reviewed through **R4 PASS** (`5b96a2f4c`). THEN expanded (InMemory confirmation capability, `b8fceb3bc`). THEN **two uncommitted edit passes today** (clarity tidy + symmetric success/failure decision). **R5 re-review still owed** (must cover InMemory expansion **and** the symmetric change). |
14+
| Requirements | ✅ Approved (`.requirements-approved`), `d7c997512`, PR #4180. Edited since (FR-10 removed, FR-2 made symmetric, OOS-4 narrowed, AC-13 rewritten)committed `d8b930803`. |
15+
| Design | ✅ ADR 0063 (`.adr-list`). History: R1 Crit → R2 High → R3/R4 PASS (`5b96a2f4c`) → InMemory expansion (`b8fceb3bc`) → clarity tidy + symmetric success/failure decision (`d8b930803`) → **R5 PASS** (`review-design.md`, 0 findings ≥ 60) with all 5 sub-threshold findings folded in → **Option B InMemory pump** (single worker + `Task.Run`-per-raise, AC-10 now exercised in-memory). **R6 re-review owed**: scrutinize the detailed pump — drain correctness, the `Task.Run` concurrency claim, dispose ordering, no lost/double confirmations. |
1616
| Tasks | Not started |
1717
| Code | Not written |
1818

1919
## ▶️ FIRST THING NEXT SESSION
2020

21-
Working tree has **uncommitted edits** to `specs/0034-failed-delivery-context/requirements.md` and `docs/adr/0063-failed-delivery-context.md` (clarity pass + symmetric decision). Options:
22-
1. Eyeball the diffs, then commit them (the prior ADR/requirements work is already committed; these are follow-up edits).
23-
2. Run `/spec:review design` (round 5) — must cover the InMemory expansion **and** the new symmetric-span decision.
24-
3. When design is settled: `/spec:approve design` → then `/spec:tasks`.
21+
Everything is committed + pushed (working tree clean). Design is at **R5 PASS** but the InMemory pump gained substantial new detail (Option B concurrency + two-stage drain) that should be re-reviewed before approval. Options:
22+
1. Run `/spec:review design` (**round 6**) — focus on the InMemory pump: drain correctness (two-stage: await worker, then await in-flight `Task.Run` raises), the AC-10 concurrency claim, dispose ordering, no lost/double confirmations on shutdown. Also re-verify the symmetric orphan-fix (R5 confirmed it works via `StartActivity(parentId:null)` → ambient `Activity.Current` fallback).
23+
2. If R6 is clean: `/spec:approve design` → then `/spec:tasks`.
2524

2625
## ✅ RESOLVED DECISIONS (were open; now locked)
2726

@@ -64,10 +63,18 @@ perturb `Activity.Current`.
6463
3. **Binary-break + test migration**`Action<bool,string>``Action<PublishConfirmationResult>` breaks ~**10 in-repo test
6564
subscriber sites** (Kafka `delegate(bool,string)`; RMQ `(success,messageId)`/`(success,guid)`). Migrate them; confirm break
6665
fits target release line.
67-
4. **InMemory confirmation capability**`InMemoryMessageProducer` = 3rd `ISupportPublishConfirmation` implementer: opt-in
68-
async-confirm switch (default off = today's sync `(true,id)`), `Channel`+worker pump raising enriched callback off-thread,
69-
failure-injection hook (drives the `false` path), capture `Activity.Current` before enqueue + carry, dispose draining. Its
70-
orphaned `Action<bool,Id>` event (no external subscribers) → enriched `Action<PublishConfirmationResult>`.
66+
4. **InMemory confirmation capability**`InMemoryMessageProducer` = 3rd `ISupportPublishConfirmation` implementer. Pinned
67+
design (round-5 review folded in): settable props (property-injection style) **`bool UseAsyncPublishConfirmation`** (default
68+
`false` = today's sync write + `(true,id)`) and **`Func<Message,bool>? PublishFailurePredicate`** (default null = never fail;
69+
`true` ⇒ no bus write + raise `(false,…)`). When switch ON, **fire-and-forget**: `Send`/`SendAsync` capture `Activity.Current`,
70+
enqueue a work-item (carrying the `Message`) onto an **unbounded** `Channel` (SingleReader, SingleWriter=false) and return
71+
WITHOUT writing the bus; a **single** lazily-started worker drains **FIFO**, **writes `InternalBus`** (produce-order
72+
deterministic) **then dispatches the raise via `Task.Run`** (Option B — concurrent callbacks, mirrors Kafka `:373,381`). This
73+
**DOES** exercise AC-10/NFR-3 same-topic concurrency in-memory (cost: confirmation *ordering* non-deterministic, but no AC
74+
asserts it; single-msg ACs await 1 confirm, unaffected). **Two-stage drain** on dispose: complete writer → await worker → await
75+
in-flight raise tasks (`Interlocked` count + `TaskCompletionSource`). `SendWithDelay` → Scheduler re-enters `Send` later (pump
76+
engages then). Batch `SendAsync` = one work-item per message. Failure hook: `true` ⇒ no bus write + raise `(false,…)`. Orphaned
77+
`Action<bool,Id>` event (no external subscribers) → `Action<PublishConfirmationResult>`.
7178
5. **Symmetric span + orphan fix (NEW)** — mediator callback creates the confirmation span (S2) FIRST on every invocation, sets
7279
`Activity.Current = S2`, THEN branches. New `BrighterTracer` confirmation-span helper (outcome flag, both branches, sets
7380
`Activity.Current`). Success branch needs its OWN regression test (AC-13): asserts S2 + link to S1 + `MarkDispatched` DB span
@@ -136,8 +143,11 @@ When a confirmation-based producer (Kafka/RMQ) reports a failed publish via `OnM
136143

137144
## Review history
138145
- **Requirements:** 3 adversarial rounds (7→4→2 findings ≥ threshold, all resolved). `review-requirements.md` (round 3).
139-
- **Design:** R1 Critical → R2 High (concurrency) → R3/R4 PASS, all committed. `review-design.md` (round 4). **R5 owed**
140-
must cover (a) the InMemory expansion and (b) the symmetric success/failure span decision.
146+
- **Design:** R1 Critical → R2 High (concurrency) → R3/R4 PASS → **R5 PASS** (`review-design.md`, 0 findings ≥ 60; covered the
147+
InMemory expansion + symmetric span; empirically confirmed the orphan-fix re-parents via `StartActivity(parentId:null)`
148+
ambient fallback). All 5 R5 sub-threshold findings folded in (pump switch/hook names, single-worker FIFO, `SendWithDelay`,
149+
deferred-visibility consequence, the 2-async-only `CancellationToken` correction, `:1006` line fix) + Option B pump adopted.
150+
**R6 owed** — re-review the detailed pump (drain correctness, `Task.Run` concurrency, dispose ordering).
141151

142152
## ⚠️ Process reminders (CLAUDE.md)
143153
- **TDD MANDATORY**: TEST tasks use `/test-first <behavior>`; STOP for approval after each test.

0 commit comments

Comments
 (0)