Add asynchronous posting (post_async/post_await, proposal 2 of #15)

[eric-descourtis-thenvoi]

Summary

Adds asynchronous posting — pobox:post_async/2 returns a request-id promise
immediately, and pobox:post_await/1,2 collects its ok/full result later. This
implements proposal 2 of #15 (the rpc:async_call/rpc:yield pattern), so a burst of
messages can be submitted concurrently instead of one blocking post_sync round-trip at
a time. Purely additive.

Scope

The last of #15's three enhancements, a sibling to weighting (#16), calls (#17) and
preflight (#18). Branched off master; versioned 1.6.0, sequenced after those.

What's added

-spec post_async(name(), term()) -> gen_statem:request_id().
-spec post_await(gen_statem:request_id()) -> ok | full.
-spec post_await(gen_statem:request_id(), timeout()) -> ok | full | timeout | {error, term()}.

• post_async/2 fires a post without blocking and returns a promise.
• post_await/1,2 collects the ok/full result; timeout if it hasn't arrived (the
  promise stays valid and can be awaited again); {error, noproc} if the box is gone.

Usage — fire everything, then gather:

ReqIds  = [pobox:post_async(Box, M) || M <- Messages],   %% none blocks
Results = [pobox:post_await(R, 5000) || R <- ReqIds].    %% each is ok | full

Design

These are thin client-side wrappers over gen_statem:send_request/2 and
receive_response/2 — no server-side change, because the existing {post, Msg} call
handler (shared with post_sync) already replies ok | full. This keeps the whole
feature to three small functions with no new state or protocol.

Backward compatibility

Strictly additive — no existing function, message, or behavior changes. All 67 original
Common Test cases and 3 properties pass unchanged.

Test plan

• 70 Common Test cases (67 original + 3 new): post_async/post_await happy path
  (ok + full), 100-message pipelined submission collected in order, and box-death →
  {error, noproc}.
• 3 PropEr properties unchanged.
• rebar3 dialyzer clean; compile warnings-as-errors clean.

rebar3 ct       # 70 passed
rebar3 proper   # 3/3 properties
rebar3 dialyzer # 0 warnings

Files changed

 src/pobox.erl              | 27 +++++++-
 test/pobox_async_SUITE.erl | 53 ++++++++++++++++
 README.md                  | 15 +++++
 src/pobox.app.src          |  2 +-

Note

Fire-and-forget (calling post_async and never awaiting) leaves a lingering monitor and
reply in the caller's mailbox — inherent to gen_statem:send_request; the contract is to
await (or discard) each promise, same as gen_server:send_request.

Implements proposal 2 (async post) of #15.

🤖 Generated with Claude Code

[eric-descourtis-thenvoi]

@ferd — external review of PR #19 (async posting: post_async/post_await). Small, cleanly-scoped diff; the code is correct against the OTP primitives, but the documented timeout contract is false and one spec is unsound — both are doc/contract issues, not crashes.

Important

Verdict: COMMENT — no HIGH. 3 MED (all doc/contract/coverage, PR-introduced) + 3 LOW. The wrapper implementation is sound; the blockers-to-clear are the re-await documentation (M1), the post_await/1 spec (M2), and the matching test gap (M3). Nothing here is a runtime crash on the happy path.

At a glance

Sev	ID	Title	Site
🟠 MED	M1	Docs promise re-await after timeout, but receive_response/2 abandons the request → silent loss of the ok/full result	README.md:241,243 · src/pobox.erl:238
🟠 MED	M2	post_await/1 spec -> ok | full is unsound: box death under infinity returns {error,noproc}	src/pobox.erl:235
🟠 MED	M3	post_await/1 arity + the timeout/re-await path have zero test coverage	test/pobox_async_SUITE.erl
🟡 LOW	L1	Return type mixes bare atom timeout with {error, Reason} tuple (ergonomics wart)	src/pobox.erl:236
🟡 LOW	L2	Guard asymmetry (post_async guarded, post_await not) — verified correct-by-design	src/pobox.erl:234-240
🟡 LOW	L3	vsn 1.2.0 → 1.6.0 skips 1.3–1.5 (sibling PRs) — merge-train note	src/pobox.app.src:3

PR: #19 · branch: feat/issue-15-async-post @ 01f88e9 · base: master · +95 / −2, 4 files · CI: none configured (ran locally, below).

Methodology: fresh multi-pass review — 6 parallel lanes (llm-hardening, correctness, test-adequacy, a cross-model codex lane, plus general-review on both Opus 4.8 and codex, run last). Every finding verified against the branch tip and the OTP 27 stdlib source (gen.erl / gen_statem.erl), plus 3 live escript probes (re-await abandonment, malformed-ReqId, noproc). Two dry-round convergence. The author's "purely additive / backward-compatible" framing was treated as a hypothesis and confirmed (no server-side change, existing {post,Msg} handler reused, exports appended not reordered). Not a runtime-fan-out diff (no DB write / broadcast / channel / GenServer-side change), so no live-tracing gate — but the doc/spec claims were proven live anyway.

---

MED (PR-introduced)

M1 — Docs promise re-await after timeout, but receive_response/2 abandons the request

README.md:241,243 · src/pobox.erl:238

The README ("the promise stays valid and can be awaited again") and the post_await/2 doc-comment assert a re-await guarantee OTP does not provide: gen_statem:receive_response/2 abandons the request on timeout (demonitor(ReqId,[flush])). A caller who follows the documented "await again" instruction after a timeout silently loses the post's ok/full result forever.

Blast radius: contained — silent loss of the result on a documented, invited path; no crash, no server-side corruption (the message may still be buffered, but the outcome is unobservable).
Scope of cause: three sites — README prose (:243), README example comment (:241 %% each is ok | full, which also understates the reachable timeout/{error,noproc} on the finite-timeout collect), and the post_await/2 doc-comment (src/pobox.erl:238).
Fix: correct all three sites — drop "can be awaited again"; fix the %% each is ok | full comment. If re-await is genuinely wanted, implement it with a reqids collection + check_response/3 (which does not abandon), not receive_response/2.

flowchart TD
  A["post_async/2 -> ReqId"] --> B["post_await ReqId T1"]
  B -->|reply in time| OK["returns ok or full"]
  B -->|T1 elapses| AB["gen:receive_response after-branch: demonitor flush, request ABANDONED"]
  AB --> R1["returns timeout"]
  R1 --> C["docs say: await again"]
  C --> D["post_await same ReqId T2"]
  D --> E["monitor gone, reply flushed"]
  E --> R2["returns timeout again -- ok/full lost"]
  classDef bad fill:#ffebe9,stroke:#cf222e,color:#1A1A1A;
  classDef good fill:#dafbe1,stroke:#1a7f37,color:#1A1A1A;
  class OK good;
  class AB,E,R2 bad;

Loading

Evidence & trace

OTP 27 stdlib-6.0/src/gen.erl receive_response/2, the after TMO branch:

after TMO ->
    erlang:demonitor(ReqId, [flush]),
    receive {[alias|ReqId], Reply} -> {reply, Reply} after 0 -> timeout end

The demonitor(..., [flush]) drops any pending reply/DOWN and removes the monitor, so a second post_await(ReqId, _) blocks to its own timeout and returns timeout again — never the buffered ok/full. gen_statem.erl:2138-2140 docs it outright: "receive_response/2 abandons the request at timeout."

Reproduced independently by three lanes and by a deterministic escript probe (suspend the box → post_await(ReqId,100) = timeout; resume → post_await(ReqId,2000) = timeout, never ok). The Opus general-review lane caught the README:241 example-comment as a third site the specialist lanes missed.

README.md:243 doc "can be awaited again" -> user catches `timeout` from post_await/2
 -> re-calls post_await(SameReqId,_) -> gen:receive_response after-branch already
    demonitor+flush'd on the 1st timeout -> 2nd call blocks then returns `timeout`
 -> ok/full reply lost silently

---

M2 — post_await/1 spec -> ok | full is unsound

src/pobox.erl:235

post_await/1 delegates to post_await(ReqId, infinity). The timeout branch is unreachable under infinity, but the 'DOWN' clause is timeout-independent: if the box dies mid-await, receive_response returns {error,{noproc,_}} → post_await/1 returns {error,noproc}, a value outside its declared ok | full.

Blast radius: contained — a caller trusting the spec and writing ok = pobox:post_await(ReqId) gets a badmatch (caller crash) on a legitimately-reachable value.
Scope of cause: this spec line. Note Dialyzer's green run does not vindicate it — it tolerates an under-declared (narrower-than-reality) return.
Fix: -spec post_await(gen_statem:request_id()) -> ok | full | {error, term()}.

flowchart TD
  A["post_await/1 spec: ok or full"] --> B["post_await ReqId infinity"]
  B -->|box replies| OK["ok or full  -- in spec"]
  B -->|box exits mid-await| DN["gen.erl DOWN clause"]
  DN --> ER["returns error noproc -- OUTSIDE the ok|full spec"]
  ER --> CR["caller: ok = post_await R  -> badmatch crash"]
  classDef bad fill:#ffebe9,stroke:#cf222e,color:#1A1A1A;
  classDef good fill:#dafbe1,stroke:#1a7f37,color:#1A1A1A;
  class OK good;
  class ER,CR bad;

Loading

Evidence & trace

gen.erl receive_response/2 'DOWN' clause: {'DOWN',ReqId,_,Object,Reason} -> {error,{Reason,Object}} — fires regardless of Timeout; send_request installs the monitor. So under infinity the DOWN path is live. post_await/2 maps it to {error,Reason}.

Dialyzer passes because success typing does not flag an under-declared return (spec narrower than actual) as a contradiction — only impossible/oversized returns. So the green Dialyzer run is not evidence the arity-1 spec is sound. Verified by the correctness lane against the OTP source; the reachability of {error,noproc} is the same path the suite's async_await_noproc case exercises (via arity-2).

post_await/1 spec claims ok|full -> body calls post_await(ReqId,infinity)
 -> box exits mid-await -> gen.erl DOWN clause -> {error,{noproc,_}}
 -> post_await/2 maps to {error,noproc} -> arity-1 returns {error,noproc}
 -> caller `ok = post_await(R)` badmatch crash

---

M3 — post_await/1 + the timeout/re-await path have zero test coverage

test/pobox_async_SUITE.erl

All 3 CT cases call post_await/2; none calls post_await/1, none forces a timeout return, none re-awaits. The suite is green precisely because it never tests the one claim (M1) that does not hold, nor the arity-1 spec (M2).

Fix: add one line exercising arity-1 (ok = pobox:post_await(pobox:post_async(Box, a))), and a characterization test for the timeout→re-await path (suspend box → await 100ms asserts timeout, resume → await asserts the real result). The latter fails as written — which is what forces the M1 reconciliation.

Evidence & trace

all() -> [async_post_and_await, pipelined_posts, async_await_noproc]; every await in the suite is post_await(R, <int>). The arity-1/infinity path and the timeout-then-reawait path have no assertion.

The existing cases were separately audited and are sound: Msgs = lists:seq(1,N) is a genuine match (fails the case on order violation, not a no-op), [ok] = lists:usort(Results) asserts all-100-ok, and async_await_noproc is race-free (it blocks on a 'DOWN' barrier before post_async). The gap is coverage, not correctness of what's there.

all/0 lists 3 cases -> each binds post_await(R,Int) -> arity-1 never invoked &
timeout branch never forced -> M1's false claim & M2's spec both ship untested
 -> green suite on an unsound contract

---

LOW

L1 — post_await/2 return type mixes bare atom timeout with {error, Reason} tuple

src/pobox.erl:236,240-247

Ergonomics wart: ok | full | timeout | {error, term()} mixes a bare atom and an error tuple, so a caller matching {error, _} for all failures silently skips the timeout case. Consider {error, timeout} for a uniform failure channel, or document the distinction. Not a correctness bug.

Evidence & trace

Flagged by the Opus general-review lane as a wart the specialist lanes filtered out. Judgment call for the maintainer — a bare-atom-plus-tuple union is a common OTP idiom, but it does invite the {error,_}-only failure match.

post_await/2 -spec ok|full|timeout|{error,term()} -> caller writes `{error,_} = ...`
to catch failures -> a `timeout` return does not match -> badmatch or silently
unhandled depending on the caller's shape

L2 — Guard asymmetry — verified correct-by-design

src/pobox.erl:234-240

post_async/2 carries ?PROCESS_NAME_GUARD(Box) (needed — Box is a name/pid); post_await/1,2 has none. This is correct: request_id() is opaque with no public shape to guard. Recorded so it isn't re-flagged. No change needed.

Evidence & trace

Verified live: post_await(not_a_reqid, 100) raises error(badarg) from gen_statem:receive_response (clean crash on programmer error); post_await(make_ref(), 100) returns timeout with no monitor created by post_await (no leak). The cross-model codex lane rated this HIGH, but it names no worst-case beyond a clean crash on misuse — reconciled down to LOW (lower tier absent a concrete exposure).

post_await(BadReqId,T) -> gen_statem:receive_response(BadReqId,T)
 -> gen:receive_response catch error:badarg -> error(badarg)   [atom arg]
    OR blocks then returns `timeout`                            [bare ref, no monitor to leak]
 -> clean failure, no resource leak

L3 — vsn 1.2.0 → 1.6.0 skips 1.3–1.5 (sibling PRs) — merge-train note

src/pobox.app.src:3 · README.md:379

vsn jumps three minors with only a 1.6.0 changelog line; 1.3–1.5 belong to the sibling PRs (16 / 17 / 18). Per this review's scope, cross-PR sequencing is out of scope — reconcile at merge-train time. Not a blocker in isolation.

Evidence & trace

The changelog wording is accurate for this PR's code and (verified) does not repeat the false re-await claim; app.src modules: [pobox] is correct (the new file is a test/ CT suite, not a src module).

app.src vsn 1.2.0->1.6.0 + README changelog 1.6.0 -> 1.3/1.4/1.5 owned by siblings
 -> if merged out of order the published vsn is ahead of shipped content
 -> merge-train concern, out of scope here

---

Runtime-observable behavior (event fan-out)

N/A — no runtime-observable side-effects in the diff. These are pure client-side wrappers over gen_statem:send_request/2 + receive_response/2; there is no server-side change (the existing {post,Msg} call handler, shared with post_sync, is reused unchanged). No DB write, broadcast, channel push, GenServer-side message, or async job is introduced.

Test execution (run locally in an isolated worktree at 01f88e9)

The repo has no CI workflow configured; all checks were run locally via the bundled ./.bin/rebar3, in a dedicated --detach worktree (this clone is shared with the sibling-PR reviews, so an in-place git checkout would race them).

Check	Command	Result
Common Test	rebar3 ct	70/70 passed (67 original + 3 async)
PropEr	rebar3 proper	3/3 properties passed
Dialyzer	rebar3 dialyzer	0 warnings (exit 0)
Compile	warnings-as-errors	clean

Tests added by the PR (test/pobox_async_SUITE.erl, 3 cases): async_post_and_await (ok/ok/full into a size-2 keep_old box), pipelined_posts (100 async posts → collect-all-ok → in-order delivery), async_await_noproc (dead box → {error,noproc}). Coverage gap named in M3.

Live probes run during review (throwaway escripts, not committed): (1) suspend-box re-await → proved M1 (timeout, then timeout again — reply lost); (2) post_await(atom,_) → badarg, post_await(make_ref(),_) → timeout no-leak → settled L2; (3) noproc reproduction confirming the arity-2 error path M2 rides.

Recommended action

None of the three MEDs is a runtime crash on the happy path, so this is a COMMENT, not request-changes — but M1 and M2 are contract lies a downstream user will hit by following the docs, and both are one-liner fixes. I'd land all three (M1 doc correction across the 3 sites, M2 spec widening, M3 two-line test) before merge; L1/L3 are the maintainer's call and L2 needs no action.

Approval gate — exactly these 3, nothing else

• M1 — correct the re-await claim at all three sites — README.md:241, README.md:243, src/pobox.erl:238
• M2 — widen the spec to ok | full | {error, term()} — src/pobox.erl:235
• M3 — add arity-1 + timeout/re-await coverage — test/pobox_async_SUITE.erl

Does NOT gate: L1 (ergonomics), L2 (verified correct), L3 (merge-train/out-of-scope).

[eric-descourtis-thenvoi]

Review addressed ✅

All findings from the review above are fixed (TDD RED→GREEN for the behavioral one), pushed as 946d8f4 + 86f83d6. Suite: 73 CT / 3 PropEr / dialyzer clean.

MED:

• M1 — post_await switched from receive_response/2 (which abandons on timeout, losing the result) to wait_response/2, so a timed-out promise is genuinely re-awaitable — the documented contract. Verified with a deterministic sys:suspend/resume timeout test.
• M2 — post_await/1 spec widened to ok | full | {error, term()} (a box dying during an infinity await returns {error, noproc}).
• M3 — added coverage for post_await/1 (happy path + noproc) and the timeout-then-re-await path.

LOW:

• L1 — documented that the bare atom timeout (non-terminal, re-awaitable) is deliberately a different shape than {error, Reason} (terminal); timeout mirrors gen_statem:wait_response/2.
• L3 — changelog notes 1.3.0–1.5.0 are the sibling changes (the version gap is a merge-order artifact).
• L2 — guard asymmetry was confirmed correct-by-design in the review; no change (recorded).