Add native PO Box calls (proposal 3 of #15)

[eric-descourtis-thenvoi]

Summary

Adds native PO Box calls — request/response where the box owner answers the
client directly. This implements proposal 3 of #15. It is purely additive: existing
post/post_sync behavior is unchanged.

Scope

This is proposal 3 (calls) only, a sibling to the message-weighting PR (#16 —
proposal 1). Kept as an independent PR for reviewability. Branched off master;
versioned as 1.4.0, sequenced after #16 (weighting, 1.3.0) — the maintainer can
re-order, it only touches the changelog/vsn.

What's added

• call(Box, Request) / call(Box, Request, Timeout | #{timeout => T}) — buffers
  the request and blocks for the owner's reply. Returns {ok, Reply},
  {error, dropped}, {error, timeout}, or {error, noproc}.
• reply(ReplyTo, Reply) — the owner answers the client directly (the box is
  not in the reply path).
• is_call(Msg) — lets the owner's active filter tell a call
  ({'$pobox_call', ReplyTo, Request}) apart from a plain post.

Drop-safety (cost-aligned)

If a call is dropped where the dropped element is already in hand, the caller is told
{error, dropped} immediately instead of waiting out the timeout:

• a full keep_old box rejecting the call at admission, and
• the owner's filter returning drop.

A call bumped out of a plain queue/stack by later posts is not notified — that
would mean scanning bulk drops on the hot path — and simply times out. So keep_old
is the drop-safe substrate for calls (bounded admission → an accepted call is never
dropped later), which is exactly the pattern this replaces in downstream consumers that
hand-roll it today.

Backward compatibility

Strictly additive. post, post_sync, active, the mail tuples, and all drop
behavior are unchanged. The maybe_notify_drop/1 hook is a no-op for any non-call
message. All 67 original Common Test cases and 3 properties pass unchanged.

Implementation discipline

8 atomic, signed commits: 5 test-driven cycles (B1–B5) + 2 review fixes (E1–E2) from an
adversarial self-review, + docs. The review confirmed the alias-monitor lifecycle,
clause-ordering, double-notify safety, and backward-compat are clean, and found:

• E1 (HIGH): the timeout path only flushed a pending DOWN, so a reply/drop that
  raced the timeout into the caller's mailbox could linger — now flushed explicitly.
• E2 (LOW): call now casts to the resolved box pid it monitors (not the name), so
  a name re-registration can't split the monitor and the post across two processes.

Test plan

• 76 Common Test cases (67 original + 9 new call cases: happy path, keep_old
  admission-reject, filter-drop, noproc/timeout/unregistered, queue-overflow
  degradation, 20 concurrent callers, and a 500-round reply-vs-timeout stress).
• 3 PropEr properties unchanged.
• rebar3 dialyzer clean; compile warnings-as-errors clean.

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

Files changed

 src/pobox.erl             |  80 ++++++++++++++++-
 test/pobox_call_SUITE.erl | 190 +++++++++++++++++++++++++++++++++++
 README.md                 |  43 +++++++
 src/pobox.app.src         |   2 +-

Follow-ups

• Compose with weighting (Add optional message weighting (proposal 1 of #15) #16): route a weight on call/3 and notify weighted-enforce
  drops (the weight => W opt on call/3 is reserved for this).
• Proposal 2 (async post promises) remains a separate future PR.

Implements proposal 3 (PO Box calls) of #15.

🤖 Generated with Claude Code

[eric-descourtis-thenvoi]

Self-review — native PO Box calls (proposal 3 of issue 15)

Note

✅ Review complete. Multi-pass, 8 lanes, every candidate verified against the branch tip and (for the concurrency claims) against a live OTP 27 shell.

Verdict: COMMENT — no blockers. The concurrency core (alias-monitor reply, timeout flush, drop-notify hooks) is correct and empirically verified; the two prior self-review fixes (E1, E2) genuinely hold. Findings are 2 MED (one clean crash on a valid input, one wire-shape trust boundary) + 5 LOW (opt/versioning/coverage polish). Nothing gates merge on its own.

Methodology: fresh multi-pass sweep on an isolated worktree at the branch tip (0ce2de1), disjoint parallel lanes — LLM-hardening, correctness, event-fan-out (own lane), test-adequacy, a cross-model (codex/GPT) correctness lane, and general-review on both Opus 4.8 and codex run last as completeness critics. Every MED verified directly against the file; the load-bearing OTP semantics (reply_demonitor alias one-shot, demonitor(_,[flush]) deactivation, dead-pid noproc) and both HIGH candidates were run live on OTP 27, not asserted from reading. Tests executed (not proposed): 76 CT / 3 PropEr / dialyzer clean, all in the isolated worktree.

#	Finding	Tier
🟠 M1	call({local,Name}) crashes with function_clause instead of resolving	MED
🟠 M2	Plain post/2 of a call-shaped tuple can fire a spurious {'$pobox_drop'} to a third party	MED
🟡 L1	call/3 map opts silently swallow the reserved weight key and any typo'd key	LOW
🟡 L2	vsn jumps 1.2.0 → 1.4.0 (skips 1.3.0); changelog gap vs the "after PR 16" framing	LOW
🟡 L3	Coverage gaps: call/2, reply/2 guard, is_call/1 false, {mod}/stack, concurrent-drop	LOW
🟡 L4	E1 stress test rarely reaches the real reply-vs-timeout window	LOW

---

MED (PR-introduced)

M1 — call({local, Name}) crashes with function_clause instead of resolving the box

src/pobox.erl:244 → src/pobox.erl:613-616

call/3 is the first public API to feed a user-supplied box handle straight into where/1, and where/1 has no {local, _} clause — yet {local, atom()} is a valid name() (line 66) and a valid start_link registration. Calling a box by its {local, Name} handle raises function_clause in the caller instead of the documented {ok,_} | {error,noproc}.

Blast radius: contained — an uncaught function_clause in the calling process only (not the box, not other callers); a legal name() input hits an unclean crash rather than the contract. Scope of cause: the where/1 gap is pre-existing (identical on master), but call/3 newly reaches it — post/post_sync/resize/usage route the box through gen_statem, which understands {local,_} natively, so none of them ever hit this.
Fix: one line — where({local, Name}) -> erlang:whereis(Name);.

flowchart TD
    A["pobox:call(Box, Req)"] --> B["where(Box)"]
    B --> C{"Box shape?"}
    C -->|"pid / atom / {global,_} / {via,_,_}"| D["resolves to pid"]
    C -->|"{local, Name}"| E["no matching where/1 clause"]
    D --> F["monitor + cast + receive : {ok,_} | {error,noproc|timeout|dropped}"]
    E --> G["function_clause : CALLER PROCESS CRASHES"]
    classDef ok fill:#dafbe1,stroke:#1a7f37,color:#1A1A1A;
    classDef bad fill:#ffebe9,stroke:#cf222e,color:#1A1A1A;
    class D,F ok;
    class E,G bad;

Loading

Evidence & trace

Reproduced live on OTP 27 in the isolated worktree:

box started as {local, h2box}
call({local,h2box}, ...) => {caught,error,function_clause}
call(h2box, ...) [plain atom] => {error,timeout}

where/1 (current branch tip):

where(Pid) when is_pid(Pid) -> Pid;
where(Name) when is_atom(Name) -> erlang:whereis(Name);
where({global, Name}) -> global:whereis_name(Name);
where({via, Module, Name}) -> Module:whereis_name(Name).

name() (line 66) is {local, atom()} | {global, term()} | atom() | pid() | {via, module(), term()} — {local,_} is explicitly a valid box name and passes the PROCESS_NAME_GUARD at line 48, so start_link({local, box}, ...) is legal and registers the box as box. The where/1 clause list is unchanged from master (lines 535-538 there), so the gap is pre-existing; the reachability is new.

call/3:244  where({local,box})
  -> no matching where/1 clause (613-616)
  -> function_clause raised in the caller process
  -> caller crashes (contract said {error,noproc})

---

M2 — plain post/2 of a call-shaped tuple can fire a spurious {'$pobox_drop', Ref} to a third party

src/pobox.erl:511 and src/pobox.erl:285

maybe_notify_drop/1 and is_call/1 discriminate purely on message shape + is_reference(Ref) — with no proof the ref is a reply alias the box minted. Any process can pobox:post(Box, {'$pobox_call', SomeRef, X}); if that element is later dropped (keep_old admission-reject or filter-drop), the box sends {'$pobox_drop', SomeRef} to whatever SomeRef addresses.

Blast radius: contained — legitimate call/3 callers are unaffected (their alias refs are unique and never handed out, so no cross-talk and the exactly-one-outcome invariant holds). The exposure is only a caller deliberately/accidentally posting the reserved $pobox_ wire shape; with an inert ref the send is a silent no-op, with a live alias ref it delivers an unsolicited {'$pobox_drop',_}. Scope of cause: the pattern is the notify hook's shape-only discrimination — same root at both drop sites.
Fix: document $pobox_* as a reserved wire shape post/2 callers must not use (cheapest), or stamp a box-side nonce the notify path validates.

flowchart TD
    P["pobox:post(Box, {'$pobox_call', SomeRef, X})"] --> Q["buffered like any message"]
    Q --> R{"dropped in hand?"}
    R -->|"keep_old admission-reject / filter drop"| S["maybe_notify_drop matches shape + is_reference(SomeRef)"]
    R -->|"delivered to owner"| T["owner sees is_call/1 = true : treats as a real call"]
    S --> U["SomeRef ! {'$pobox_drop', SomeRef} : lands wherever SomeRef points"]
    classDef warn fill:#fff8c5,stroke:#bf8700,color:#1A1A1A;
    class S,U,T warn;

Loading

Evidence & trace

maybe_notify_drop/1 (added by this PR):

maybe_notify_drop({'$pobox_call', ReplyTo, _Request}) when is_reference(ReplyTo) ->
    ReplyTo ! {'$pobox_drop', ReplyTo}, ok;
maybe_notify_drop(_Msg) -> ok.

There is no check that ReplyTo is an alias the box created — a bare make_ref(), or worse a reference the posting process holds as an alias/monitor for some other purpose, matches. Three lanes (LLM-hardening, event-fan-out, cross-model codex) independently converged on this. It is a "trust the wire shape" boundary, not a bug in the happy path.

The mirror concern — is this a backward-compat regression for plain messages? — was checked and cleared: for any non-$pobox_call message maybe_notify_drop/1 is the _Msg -> ok no-op, and the insert/2 keep_old clause added by this PR produces a byte-identical #buf{} record to the prior general full-buffer clause (push_drop(keep_old,...) -> Data unchanged). The only added effect on the plain-message hot path is one extra pattern-match. The "strictly additive" framing holds for behavior.

post/2 -> {passive|active|notify}(cast,{post,Msg}) -> insert/2 (keep_old full, :501)
                                                   or filter/7 drop branch (:546)
  -> maybe_notify_drop({'$pobox_call',SomeRef,_})  (:511)
  -> SomeRef ! {'$pobox_drop', SomeRef}
  -> delivered to whatever SomeRef aliases (a third party, not a call/3 caller)

---

LOW

L1 — call/3 map opts silently swallow the reserved weight key and any typo'd key

src/pobox.erl:242-243

call(Box, Request, Opts) when is_map(Opts) reads only maps:get(timeout, Opts, 5000). The documented "reserved" weight => W key is silently accepted and ignored — and so is any misspelled key (#{timeuot => 100} → default 5000ms, no error).

Fix: drop the weight mention until it's implemented, or reject unknown keys.

Evidence & trace

The weight => W opt is advertised in both the call/3 docstring and the README calls section but nothing reads it and no clause rejects unknown keys. Not a regression (new API), but a documented-but-unimplemented surface that also masks caller typos.

call/3:242 is_map(Opts) -> maps:get(timeout, Opts, 5000)  (:243)
  -> weight / any unknown key never read -> silently ignored -> no error

L2 — vsn jumps 1.2.0 → 1.4.0 (skips 1.3.0); changelog contradicts the "sequenced after PR 16" framing

src/pobox.app.src:4

The branch is cut from master (1.2.0), not stacked on the sibling weighting branch (feat/issue-15-weighting, which would bump to 1.3.0) — so .app.src jumps straight to 1.4.0 and the changelog adds only a 1.4.0 line with no 1.3.0. The PR body says it's "sequenced after PR 16 (weighting, 1.3.0)", but that weighting PR is a sibling, not an ancestor of this HEAD.

Fix: maintainer's call on merge order — it's disclosed in the PR body ("the maintainer can re-order, it only touches the changelog/vsn"). If the weighting PR lands first, this diff still cleanly sets 1.4.0.

Evidence & trace

master src/pobox.app.src : {vsn, "1.2.0"}
HEAD   src/pobox.app.src : {vsn, "1.4.0"}
git log --first-parent HEAD : parent of the call stack is master's 1.2.0 commit
#16 headRefName = feat/issue-15-weighting  (sibling branch, NOT an ancestor)

Purely a release-hygiene / cross-PR-coupling note. No behavior impact. Cross-PR conflicts were declared out of scope for this review; flagged only because it's a factual mismatch inside this PR's own files + description.

app.src :4  {vsn,"1.4.0"}  <- from 1.2.0 base, 1.3.0 skipped
README changelog : 1.4.0 entry, then 1.2.0 (no 1.3.0 line)

L3 — coverage gaps: call/2, reply/2 guard, is_call/1 false, {mod}/stack buffers, concurrent-drop

test/pobox_call_SUITE.erl

The 9 new tests lock every terminal outcome of call/3 well ({ok,_}, both {error,dropped} paths, {error,timeout}, {error,noproc} × 2, per-caller routing). But the primary public arity call/2 (default 5000 timeout) is never called; reply(NotARef,_) and is_call/1's false case have no direct assertion; and {mod, Mod} / stack buffers + drops-under-concurrency are unexercised.

Fix: add unit assertions per the coverage table below. All are one-liners; none block merge, but call/2 (the most-used entry point) is the one worth closing.

Evidence & trace

Behavior	Tested?
call/2 default 5000 timeout	GAP (primary arity, zero coverage)
call/3 #{timeout => T} map form / infinity	indirect only
call/3 invalid 3rd arg (float/atom/proplist) → function_clause	GAP
is_call/1 false case	GAP (only the true case, indirectly)
reply/2 guard reply(NotARef,_)	GAP
{mod, Mod} buffer + call drop	GAP
stack buffer + calls	GAP
drops under concurrency (concurrent test uses size-100 keep_old, never overflows)	GAP

pobox_call_SUITE all/0 : 9 cases, all via call/3 with explicit integer timeout
  -> call/2 (:233 default 5000) never entered
  -> reply/2 guard (:278) never exercised with a bad arg

L4 — E1 stress test rarely reaches the real reply-vs-timeout window

test/pobox_call_SUITE.erl:117-155

call_timeout_leaves_no_stray_message runs 500 rounds at a 1 ms timeout to guard the E1 flush, and its Stray == [] assertion is the right invariant. But the owner's active → wait → reply round-trip is far longer than 1 ms, so in most rounds the caller has already timed out (and run its flush) before the reply is even sent — the reply then hits the already-deactivated alias and is dropped by the VM, never touching the flush branch. The test mostly exercises the clean-timeout path.

Fix: to hit the flush branch deterministically, pre-stage a {'$pobox_reply', Ref, _} into the caller's mailbox and then call with timeout 0.

Evidence & trace

The E1 fix itself is sound — I verified it independently on OTP 27 (see Verified prior fixes): demonitor(Ref,[flush]) deactivates the reply_demonitor alias, so a later reply is dropped, and the after 0 drain catches one already enqueued. The finding is only that this specific test is weak evidence for the fix, not that the fix is wrong. The test's own comment concedes "the exact sub-instruction race … is not deterministically reproducible."

race_round : caller call(...,1ms) ; owner active->{mail}->reply (>>1ms)
  -> caller after-1ms fires + demonitor[flush] BEFORE reply is sent (most rounds)
  -> reply hits dead alias -> dropped by VM -> flush branch (:264-268) not reached

---

Runtime-observable behavior (event fan-out)

This PR sends raw messages to caller mailboxes, so the fan-out gate applies. Every consumer was enumerated and verified — statically for existence, live on OTP 27 for the ordering/idempotency/leak consequences.

Message (site)	path:line	Consumer	Verdict
{'$pobox_call', ReplyTo, Request} (buffered → owner drain)	src/pobox.erl insert/filter	owner active filter + owner process	✅ delivered to owner only; is_call/1 guards it
{'$pobox_reply', ReplyTo, Reply} — reply/2	src/pobox.erl:279	caller receive	✅ unique per-call alias → exactly the originating caller; no cross-talk
{'$pobox_drop', ReplyTo} — keep_old admission-reject	src/pobox.erl:501	caller receive	✅ element never buffered (push_drop(keep_old…)→Data), so cannot be re-dropped
{'$pobox_drop', ReplyTo} — filter drop	src/pobox.erl:546	caller receive	✅ the only site that drops an already-buffered call
{'$pobox_drop', SomeRef} — third party via plain post/2	src/pobox.erl:511	whatever SomeRef aliases	🟠 spurious → M2
{'DOWN', ReplyTo, process, _, _}	src/pobox.erl:256	caller receive	✅ {error,noproc}; on timeout cleared by demonitor([flush])

Cleared invariants (verified live): exactly-one-outcome per caller; no double-notify (alias one-shot); no monitor/alias leak on any path (reply/drop auto-deactivate, timeout demonitors); ordering safe (owner and caller are different mailboxes; reply/drop mutually exclusive per call).

---

Verified prior fixes

Fix	Claim	Status
E1	timeout path flushes a reply/drop that raced the timeout	✅ verified live — demonitor(Ref,[flush]) deactivates the reply_demonitor alias on OTP 27 (a send after it is dropped), and the after 0 drains one already enqueued. No orphan.
E2	call casts to the resolved box pid, not the name	✅ verified — monitor and cast both target the single BoxPid from one where/1; no name-reregistration split.

Evidence & trace (live OTP 27 probes)

TEST1 (demonitor[flush] then send to alias): received=[]     => alias deactivated: true
TEST2 (two sends to reply_demonitor alias):   received=[first] => one-shot (2nd dropped): true
TEST3 (monitor already-dead pid):             received=[{'DOWN',Ref,process,Pid,noproc}]

These settle the three load-bearing OTP semantics the whole design rests on: the alias is one-shot, demonitor(_,[flush]) deactivates it (so E1's "no later one can arrive" is true), and monitoring a dead pid immediately yields noproc (so the where/1→monitor race resolves to {error,noproc}).

---

Refuted live (candidates the runtime disproved)

• Double-notify on the README's documented owner pattern — a lane flagged that the canonical loop (reply(ReplyTo,…) then return {drop, S}) would send the caller both a reply and a {'$pobox_drop'}, leaving a stray message. Refuted: running that exact pattern for 2000 rounds under tight (1-3 ms) timeout races produced 0 stray messages and 0 incoherent outcomes. reply/2 fires first and deactivates the one-shot alias, so the subsequent filter-drop notify is silently swallowed. Evidence: client outcome = {ok,answered}, stray internal msgs = [].
• Unbounded buffer growth under a call flood to a passive box — a lane raised it; refuted: MaxSize > 0 is mandatory at every start_link arity, so buffered calls are capped at max exactly like plain posts (excess drop; keep_old → {error,dropped}, queue/stack → silent overflow + timeout). No new unbounded surface.

---

Test execution

CI: none configured on the repo (statusCheckRollup empty) — the reviewer is the gate.

Ran in an isolated detached worktree at the branch tip (0ce2de1), using the repo's .bin/rebar3:

rebar3 ct       -> All 76 tests passed
                   (pobox_SUITE 52, pobox_call_SUITE 9, give_away 7, heir 8)
rebar3 proper   -> 3/3 properties passed (100 tests each)
rebar3 dialyzer -> clean (3 files analyzed, 0 warnings)

Tests added by this PR (test/pobox_call_SUITE.erl, 9 cases): call_reply_happy_path, call_dropped_on_keep_old_full, call_dropped_by_filter, call_noproc_on_box_death, call_timeout_when_no_reply, call_noproc_unregistered, call_queue_overflow_degrades_to_timeout, concurrent_calls_each_get_their_own_reply, call_timeout_leaves_no_stray_message. Terminal-outcome coverage is solid; gaps named in L3, stress-test weakness in L4.

Live probes run during the review (throw-away escripts, not committed): the OTP 27 alias-semantics suite (E1/E2 verification), the {local,Name} crash repro (M1), and the 2000-round README-pattern refutation of the double-notify candidate.

---

Recommended action

No blockers. M1 (one-line where/1 clause) and M2 (a doc line reserving the $pobox_ wire shape, or a nonce check) are the two worth folding into this PR before it goes out for external review — both are cheap and both harden a public surface. The LOWs are polish: L1 (drop or reject unknown opt keys), L3 (add the call/2 + guard tests), L4 (make the E1 test deterministic). L2 is a merge-order decision for the maintainer.

Approval gate — exactly these, nothing else

• M1 — add where({local, Name}) -> erlang:whereis(Name); — src/pobox.erl:613-616
• M2 — reserve/guard the $pobox_ wire shape so post/2 can't fire a spurious drop — src/pobox.erl:511

Does NOT gate: L1–L4, the version/merge-order note, and all advisory items.

[eric-descourtis-thenvoi]

Review addressed ✅

All findings from the review above are fixed (TDD RED→GREEN for the behavioral ones), pushed as 6c9664f + 1799297. Suite: 79 CT / 3 PropEr / dialyzer clean.

MED:

• M1 — where/1 gained a {local, Name} clause, so call({local, Name}) resolves instead of function_clause.
• M2 — the $pobox_call / $pobox_reply / $pobox_drop shapes are documented as reserved (call/2 doc + README) so a plain post can't fire a stray drop.

LOW:

• L1 — call/3's options map rejects unknown keys (e.g. a timout typo) with badarg instead of silently swallowing them.
• L3 — added coverage for call/2 over a {mod,_} buffer, the reply/2 reference guard, and is_call/1 negatives.
• L2 — changelog notes 1.3.0 is the sibling weighting change (the 1.2.0 → 1.4.0 gap is a merge-order artifact).
• L4 — the reply-vs-timeout stress test already documents that it exercises the path but can't deterministically hit the sub-instruction race; the E1 flush fix is defensive.