JOURNAL

A timestamped log of work done against PLAN.md: what was done, what make reg showed, and any surprises. README.txt remains the historical, hand-written journal and is left untouched; this file tracks the plan's execution.


2026-08-21

20:5x — Phase 0 start

Beginning execution of PLAN.md on the Sonnet model, per the approved plan saved earlier today. Phase 0 (hygiene/baseline, no behavior change).

21:02 — Cleanup

21:05 — Baseline build

make clean && make -j12 sim (native/simulator target, clang++ -DSIMULATOR): clean build, 9 warnings total, all pre-existing and none new:

(Note: the previous session's plan draft anticipated only the p_3waycall.cc:34 warning; the 7 main.cc unused-variable warnings and the p_ppap.cc unused-function warning are additional pre-existing warnings not previously called out. None block Phase 0; the ones in scope are tracked against their respective plan items above.)

Pico cross-build: arm-none-eabi-gcc is present (/usr/bin/arm-none-eabi-gcc). cd build && make -j12 — nothing to rebuild (already up to date from a prior session), confirms the CMake/pico-sdk build tree is healthy.

21:08 — Baseline make reg

Regenerated datafill from scratch (./callp -t 999) after an initial make wipedb && make reg surfaced a known, expected failure: with a freshly wiped DB and no -t 999 prefix in the reg target yet, test 1040 (basic hotline) times out waiting for a DTMF digit collection that never happens, and the run ends with FAIL: THERE ARE STILL JUNCTORS CONNECTED. This is exactly the gap Phase 1 step 1 exists to close (reg: target doesn't yet self-datafill). Re-ran with -t 999 applied manually first, matching today's actual reg: target (no -t 999 prefix) plus a pre-existing datafilled DB, which is the baseline the plan describes.

Result: 13 batches, 36 tests, all green. Exactly 3 soft *** FAIL *** There is no DTMF receiver associated with line 0 lines (sim_testrunner.h:182), matching the plan's documented expectation for tests 1014/1025 (soft-fail by design, not yet hardened — Phase 1 step 4 addresses this). Zero TEST ABORTED, zero STILL JUNCTORS CONNECTED / leak failures. regression.txt is 16559 lines. Wall time ~1.9s for the full native make reg.

This confirms the wipedb-from-scratch gap is real and tracked (Phase 1.1) rather than a new regression — baseline is otherwise clean.

21:15 — Phase 1 step 1: self-contained make reg

Prepended ./callp -t 999 >regression.txt to the reg: target (Makefile:103) and changed the first -V line to >>. Confirmed 999 is test_999_database_reset (sim_tests.cc:778) — it resets lines, LNLU, speedcalls, sysdb, and trunks, then waits ~11 simulated seconds for the dual-page EEPROM write to complete before exiting.

Checked LNR tests 3020-3024 (sim_tests.cc:2095-2273): each dials and hangs up its own setup calls before exercising #/LNR-speedcall, so none of them depend on residual DB state left by a previous test or run — no fix needed. (They're separately marked @@@ INCOMPLETE TEST @@@ because require_lnr only checks storage, not outpulsed digits — that's Phase 1 step 6, SMDR introspection, not this step.)

Verified make wipedb && make reg: green from a fully wiped DB — same 3 expected soft DTMF fails, zero aborts, zero leak failures. Re-ran make reg again without wiping (self-datafill on every run is idempotent): identical result. make reg is now fully self-contained.

21:25 — Phase 1 step 2: require_stable_for(ms)

Added require_stable_for(ms) to sim_testrunner.h. Design: it must directly follow a require_*() call (reads events.back(), requires is_conditional); it captures that event's unwrapped predicate (new raw_condition field on TestEvent, set alongside the existing SUCCESS-logging-wrapped condition in add_conditional_event) and appends a new is_stability_check event. run() re-evaluates that raw predicate every tick for ms of simulated time; any false is an immediate hard abort (mirrors the existing timeout-abort path — prints, simulator_exit(), exit(EXIT_FAILURE)); holding true for the whole window logs "CONFIRMED" and advances.

Proved it can fail, then restored, using test 1000 ("go offhook, wait 10ms, hangup; verifies... no call took place") as the vehicle: temporarily chained .require_dialtone_absent(0).require_stable_for(500) after the existing wait(10). Result: TEST ABORTED, with our new "STABLE FOR 500ms - FLIPPED (became unstable)" message — dialtone actually turns on ~170ms after offhook (CCB/DTMF-rx/treatment setup happens immediately; there's no offhook-glitch debounce in ep_lines.cc today), so the flip-detection path fired exactly as designed. Reverted the temporary chain; sim_tests.cc is back to its committed state and make reg is green again.

Real finding, logged for later, not fixed now (out of scope for this step): test 1000's require_dialtone_absent(0) only passes today because it's checked before dialtone has had time to turn on (the "hasn't happened yet" weak semantics the plan called out) — a genuine 10ms offhook currently does start a real call (CCB, junctor, DTMF rx, dialtone all get allocated), it just hasn't reached dialtone yet at the 10ms mark the test happens to sample. This is precisely the gap Phase 3 step 3 is scoped to close (line_glitch(id, ms) DSL primitive + new test 1003 for sub-35ms glitch rejection, plus whatever debounce that requires in ep_lines.cc). Left test 1000 as-is for now; Phase 3.3 should revisit it alongside 1003.

21:45 — Phase 1 step 3: cadence-aware require_treatment()

The facility-only detect_busy()/detect_dialtone()/etc. can't tell Busy from Error from Fault (all ToneFacility_Busy), or Dialtone from InitialStutterDial from StutteringDial (all ToneFacility_Dial) — only the cadence differs. Rather than duplicating cadence constants into the test file (drift risk), exposed the real cadence table as the source of truth: added treatment_tone_lookup(TreatmentType) (treatment.h/.cc), returning a pointer into the existing file-static tones[] array.

Added TreatmentTracker (sim_testrunner.h) and .require_treatment (line_id, TreatmentType, timeout_ms=5000). It decodes the real Tone (pattern/timebase/length/repeat) into a run-length sequence of (facility, duration_ms), tracks the line's junctor tone as an edge-triggered run list via fetch_tone(), and matches:

Relocated the T_NONE/T_QUIET/T_BUSY/T_RING/T_DIAL facility-code macros and fetch_tone()'s prototype from sim_tests.cc (where they were defined too late in the translation unit for sim_testrunner.h — which gets included near the top via main.hsimulator.h — to see them) to sim_main.h, alongside the other detect_* prototypes that make the same journey. No behavior change, just makes the macros usable from the new tracker.

Proved both directions on test 3000 before trusting it further: temporarily changed its expected type to Treatment_Busy (wrong) — correctly TIMEOUT/TEST ABORTED; reverted to Treatment_Fault (correct) — SUCCESS.

Real finding — TESTS.txt and the code disagree about which "reorder-family" treatment applies, in both directions:

Also retrofitted 4030/4031 (call transfer) per the plan: after the flash, P_Collect::stutter_dialtone(true) applies Treatment_InitialStutterDial while the flashing party is parked and mid-dial. Added .require_treatment(0/1, Treatment_InitialStutterDial) right after .line_flash() and before the follow-up dial_string() (it naturally gates the subsequent dial until the ~1.2s one-shot stutter pattern is classified or times out) — both SUCCESS.

make wipedb && make reg: green (3 expected soft DTMF fails, zero aborts, zero leaks). Pico cross-build (cd build && make -j12): still builds clean (only pre-existing warnings — a translate.cc:125 sign-compare and the usual newlib-stub linker notes, both unrelated to this change).

Not done from the original Phase 1.3 scope, deferred: the 1000-1042 retrofit of require_treatment/stutter assertions onto other existing line tests, and 3000-3012 test are the only ones actually needing Busy/Error/Fault disambiguation today — no other currently-passing test relies on the weak facility-only checks in a way that hides a real cadence bug, based on this session's read of TESTS.txt vs sim_tests.cc. require_treatment is now available for Phase 2/3/4 work (e.g. the 4A park/retrieve and 4D EBO warning-tone tests will want it).

22:05 — Phase 1 step 4: DTMF-RX soft-fail hardening

dtmf_dial_digit()'s "no DTMF receiver associated with line N" case only ever wrote a Debug line; it never failed the test, so a genuinely unexpected receiver-allocation bug in any other test could sit in regression.txt forever as an ignored *** FAIL *** line. Added allow_no_dtmf_rx (default false) and .expect_no_dtmf_rx(bool); when false (the default), hitting "no DTMF receiver" now hard-aborts the test (same TEST ABORTED / simulator_exit() / exit(EXIT_FAILURE) path as a timed-out require_*).

Only two tests trigger this today, both intentionally: 1014 (pulse-dial "1" releases the DTMF RX; the follow-up DTMF "23" should find none — split its single dial_string("P1T23") into dial_string("P1") + .expect_no_dtmf_rx(true) + dial_string("T23") so the opt-in applies to exactly the right digits) and 1025 (DTMF RX is deallocated after "100"; dialing "9" should find none). Both now opt in explicitly.

Proved the hard-fail path: temporarily dropped 1014's opt-in → ./callp -t 1014 correctly hit *** FAIL *** then TEST ABORTED. Restored the opt-in. make wipedb && make reg: still exactly 3 soft *** FAIL *** lines (the same 1014/1025 occurrences, now deliberately allowed rather than silently ignored), zero aborts, zero leaks. regression.txt has zero unexplained *** FAIL *** lines, matching the plan's stated goal for this step. Pico cross-build still clean.

22:20 — Phase 1 step 5: per-batch -k 1 for cadence-heavy batches

No-op for now: no reg batch uses trunk_ring yet (that only arrives with Phase 2.5's incoming-trunk tests), and the cadence checks added in step 3 (3000-3012, 4030/4031) have run reliably at the default 5ms tick across every rebuild+rerun this session — no flakiness observed, so no -k 1 override added. Revisit when Phase 2.5 adds a trunk_ring-using batch, or if any cadence batch turns out flaky later. make reg wall time is holding around 2-6s (varies with -j12 object-cache state, -t 999's fixed ~11s simulated datafill-write wait dominates each run's simulated time but not real wall time since the simulator runs faster than real time).

22:25 — Phase 1 step 6: SMDR introspection

Added a #ifdef SIMULATOR ring buffer (8 entries) of completed calls' outpulsed digits in p_smdr.cc: sim_smdr_record(digits) (called from P_SMDR::handler's existing CCBEvent_Onhook branch, right after the JSON emit() it already does — same ccb -> digits() value, just captured as real state instead of only a Debug stream side effect, respecting the "Debug<< compiles away on Pico" constraint) and sim_smdr_contains(digits). New DSL verb .require_smdr_digits(content) in sim_testrunner.h, same conditional-event shape as require_lnr.

Retrofitted the four LNR tests that were marked @@@ INCOMPLETE TEST @@@ because "we really need an SMDR introspection tool" (3021-3024). Used ./callp -t <N> with the JSON SMDR trace to find each test's actual final-call outpulsed digits before asserting them (rather than guessing): 3021 → "6135551212", 3022 → "6135551213" (not "101", the intervening extension call), 3023 → "6135551214" (not "6135551200", the SC 200 target), 3024 → "6135551212" (not the empty "9"-only call). All four now assert both require_lnr (storage) and require_smdr_digits (that the recalled number was actually redialled) — the missing half TESTS.txt asked for. Removed the @@@ INCOMPLETE TEST @@@ banner markers and the matching usage-string lines.

Proved the negative: temporarily changed 3021's expected digits to a bogus value → TIMEOUT/TEST ABORTED; reverted.

make wipedb && make reg: green (same 3 expected soft DTMF fails, zero aborts, zero leaks). Pico cross-build clean.

22:35 — Phase 1 step 7: pbx.html path tolerance

writeHTMLToFile/writeJunctorHTMLToFile (html.cc) already degrade gracefully on an open failure (check is_open(), print to stderr, don't crash) — this environment even has /data/internet/internal/ writable, so nothing was actually broken here. But the path is hardcoded to one developer's machine layout, which means every test run on any other checkout prints a spurious Error: Could not open file twice. Gated both calls (sim_main.cc's snapshot: exit path) behind getenv("CALLP_HTML"): unset (the new default) skips the dump entirely and prints nothing; set to a directory, writes $CALLP_HTML/pbx.html and $CALLP_HTML/pbxj.html there, same as before. Verified both paths directly with -t 1000: no CALLP_HTML → no HTML/error output at all; CALLP_HTML=<scratch dir> → both files written and reported, same as the old hardcoded behavior.

make wipedb && make reg: green. Pico cross-build clean.

Phase 1 (test infrastructure) is now complete — steps 1-7 all done (step 5's -k 1 batching was a documented no-op, nothing currently needs it). make reg is self-contained, has three composable strong- assertion primitives (require_stable_for, require_treatment, require_smdr_digits) beyond the original facility-only checks, zero unexplained soft fails, and no environment-specific noise. Moving to Phase 2 (bug fixes) next.

Phase 2 — Bug fixes

23:00 — Phase 2 step 1: NULL-processor path → fast busy

P_LineState_Route (p_line.cc:109-114) called translator -> insert_processor() and threw away the return value. CCB::insert_processor(ProcessorName) (cc.cc:210-219) already returns NULL and logs *** UNABLE TO INSERT NEW PROCESSOR *** when processor_factory() has no case for the requested name (e.g. PName_Pickup/PName_ProgramSpeedcall, both commented out at processor.h:196-197) — nobody downstream checked, so the CCB was left with no terminator processor at all: dead air, confirmed matches the plan's description. Fixed: capture the return value; on NULL, apply Treatment_Error (fast busy, per README.txt:1599 "We should go to fast busy if we can't insert a processor, e.g., '*201'") and go straight to WaitingForHangup instead of PostRouting.

Also removed { "*2XX", PName_ProgramSpeedcall } from ttable (translate.cc:48, commented out not deleted) — feature-programming access codes are out of scope per this plan's binding decisions, and a ttable entry with no possible processor was exactly this trap (it used to swallow "*2" through "*299" waiting for more digits, then hit the same NULL-processor dead end at 3 digits; now it fails immediately as a non-match, same as any other unassigned code).

Added tests 3003 (dial 3 → confirmed Treatment_Error, i.e. fast busy, not dead air) and 3004 (dial *29 → confirmed Treatment_Fault — worth noting: removing the ttable entry means translate() now resolves as a complete non-match after just 2 digits ("*2"), not 3; split the dial into "*2" then .expect_no_dtmf_rx(true) then "9" to match that reality once I saw the first version of 3004 abort a genuine make reg run on this — the DTMF RX is already released by the time the "9" arrives since translate() gave up at "*2"). Both use require_treatment from Phase 1.3. Added TESTS.txt prose, usage-string entries, scan_tests cases, and a -V3000,3001,3002,3003,3004 Makefile batch (the existing 3000-3002 batch, extended) — the standard 5-edit mechanics.

Left as a @@@ note in 3003 for Phase 4B: once call pickup is implemented, dialing 3 with nothing ringing will route through P_Pickup's own "nothing to pick up" reorder path instead of this NULL-processor fast-busy path — 3003 should be revisited then (the plan already flags this at Phase 4B's test list).

make wipedb && make reg: green (4 soft *** FAIL *** lines now — the original 3 plus 3004's new deliberately-allowed one — zero aborts, zero leaks). Pico cross-build clean.

23:20 — Phase 2 step 2: feature_target_target() ring-group support

feature_target_target() (utils.cc:462-490) had branches for extension/speedcall/LNLU targets but fell through to "***" for FEATURE_TARGET_RINGGROUP/_RINGGROUP_END (230-237) — meaning CFWD/ CFNA/CFB pointed at a ring group was silently unroutable ("***" isn't a valid dial string in ttable, so translate() would just fail). Added the missing branch: "74" + to_string(t - FEATURE_TARGET_RINGGROUP), matching ttable's "74X"PName_RingGroupP entry — same pattern as feature_target_string()'s existing display-side branch (utils.cc:451-453), which already had this case right.

Datafilled a spare line (71, unused previously) with p_cfwd = FEATURE_TARGET_RINGGROUP + 0 (ep_lines.cc) — unconditional call forward from extension 171 to ring group 0. Added test 4003: dial 171, confirm ring group 0's six members (extensions 120-125, lines 20-25) all ring, mirroring test 4100's existing ring-group assertions. TESTS.txt prose, usage string, scan_tests case, and the 4000,4001,4002 Makefile batch extended to include 4003 — the standard 5-edit mechanics. Required a -t 999 re-datafill (new LineDB value); make wipedb && make reg picks that up automatically since Phase 1.1.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks) — all 6 ring-group members confirmed ringing in the trace. Pico cross-build clean.

23:45-00:40 — Phase 2 step 3: nested ring groups (grew well past the plan's scope)

Started from the plan's literal ask: p_ringgroup.cc:82 used feature_target_string() (the display formatter, #ifdef DUMP gated but DUMP is unconditionally defined in main.h so that wasn't the issue) instead of feature_target_target() (the dialable-digits formatter) for a ring-group member's digits. They happen to produce identical strings for extension/speedcall targets (0-199) — pure coincidence of the formulas — which is exactly why this stayed hidden: every existing ring-group test only uses extension/speedcall members. It diverges for LNLU and nested-ring-group members. Fixed the one-line call site. This alone should have been the whole step. It wasn't.

Depth guard, per the plan's own ask to "confirm... limited gracefully": confirmed it was not. P_RingGroupP::handler's Idle case had no bound on recursion, and ccb_alloc() (cc.cc:20-33) returns NULL on pool exhaustion (NCCBS=24) with no NULL check at the P_RingGroupP call site (c -> digits (...) on a NULL c) — a self-referential or circular ring-group chain was a straight-up NULL-deref crash waiting to happen, not a graceful failure. Added ringgroup_depth to CCB (cc.h), incremented per nesting level, checked against a new MAX_RINGGROUP_DEPTH (4, p_ringgroup.h) in the Idle case; added the missing ccb_alloc() NULL check (skip that member, don't crash).

Ringback never reached the caller in a nested group — a second, deeper bug: built test 4104 (dial 745 = RG5, whose only member is RG0; verify 120-125 ring, answer 120, verify 121-125 stop, hang up clean) to exercise the fix, per the plan. It failed at the very first assertion: require_ring_present(0) timed out. Root cause: P_RingGroupP never emits CCBEvent_CallProgress — only P_CallExtension does. A nested P_RingGroupP shares its ccb with its parent's P_RingGroupS (the one that routed into it), so that parent needs a CallProgress event to react to, exactly like it reacts to a plain terminator. Nothing sent one, so the whole chain above the inner group sat in EigenstateBusyRing forever — no ringback to the caller, and (see below) no way to ever resolve the call. Added P_RingGroupP::emit_call_progress() and called it: CallProgress_Ringing when a member first rings, CallProgress_Terminated when one connects, CallProgress_Busy when all fail / the group is empty / the group ID is invalid / the depth guard trips. Also broadened P_RingGroupS's destructor: it only called ccb->teardown() when ccb->terminator was set, which is never true for a still-ringing nested group (only the inner primary's ccb gets a terminator, once something answers) — so abandoning an unanswered nested group leaked every one of its own secondaries forever. Now also tears down when processor (whatever we routed to — a nested P_RingGroupP here, but also covers a not-yet-committed plain terminator) is still active.

Third, even deeper bug, found while fixing the second: answering a ring group call has never worked, nested or not. secondary_connected() — the method that hands the real terminator up to the primary and shuts down every other secondary — turned out to be completely dead code. It's only reachable via CallProgress_Terminated, and NOTHING emitted that when a ringing P_CallExtension got answered (Offhook in P_CallExtensionState_Ringing just adds SMDR and goes straight to WaitForHangup — confirmed by grepping every CallProgress emission site in the codebase; p_callext.cc only ever sent Ringing/Busy/ Terminated-on-intercept, never Terminated-on-answer). Verified this against the existing, currently-passing non-nested ring-group suite first, to make sure I wasn't inventing a problem: -t 5,740,2,120,2,2 (answer 120, wait, then originator hangs up) never once logs "has a terminator that has connected" — the clean teardown I originally saw there happens because the originator hanging up cascades active=false through every secondary regardless of who "won", not because answering ever actually connected anything. A member answering today just picks up to silence and waits for the caller to give up. Fixed by adding the missing CallProgress_Terminated emission in P_CallExtensionState_Ringing's offhook handler (mirroring the existing Ringing/Busy emissions right above it) — confirmed safe for plain (non-ring-group) calls too: P_CallForwardNoAnswer already cancels itself directly off the raw CCBEvent_Offhook, not CallProgress, so it's unaffected, and P_Line doesn't have a CallProgress case at all.

Fourth bug, self-inflicted, caught before it shipped: infinite recursion / stack-overflow crash. emit_call_progress() calls ccb->handle_event(), which delivers to every processor on that ccb — including, when called from inside P_RingGroupP::handler's own Idle case (the depth-exceeded / invalid-group / empty-group failure paths), back to itself, reentrantly, while its own state variable hadn't moved off Idle yet. The Idle case doesn't gate on event type, so the reentrant CallProgress delivery re-ran the entire Idle setup logic again, which emitted another CallProgress, which reentered Idle again — unbounded recursion in a single C++ call stack. Proved this concretely (not hypothetically) by temporarily datafilling ring group 6 to point at itself (sysdb.cc, reverted after) and dialing it: ./callp -t 1,746,3 segfaulted (SIGSEGV, confirmed via $?=139), with "P_RingGroupP ... Idle ... CallProgress_Busy" / "nesting depth 4 exceeded" repeating 1829 times in the log before the crash. Fix: transition state to WaitForHangup before calling emit_call_progress() in all three Idle-case failure branches, so the reentrant self-delivery lands on the inert WaitForHangup case instead of looping. Re-ran the same self-referential scenario: exits cleanly ($?=0), "nesting depth" logged exactly once, no leaks, no FAIL/ STILL lines. (secondary_ringing/secondary_connected/ secondary_terminator_failed don't have this problem — they're called from a different processor's handler frame, not reentrantly on their own, so self-delivery there just hits an already-transitioned, inert case.)

Verification: test 4104 (nested ring group, answer, cleanup) full green. Full non-nested ring-group suite (4100-4103) still green — the answer-connects fix doesn't regress anything, because nothing previously exercised the answer path. Self-referential ring group (temporary datafill, reverted): no crash, single graceful Fault, clean teardown, confirmed by hand since it's not something to leave as a permanent regression test (would require permanently datafilling a self-loop, which is more a "does the guard exist" smoke test than a real feature test — the depth-guard code path is already exercised structurally by 4104's real RG5->RG0 chain). make wipedb && make reg: green throughout every stage of this (same 4 soft fails, zero aborts, zero leaks). Pico cross-build clean throughout.

Scope note for future-me: this step's real size was "fix four compounding bugs in ring-group call completion and nesting," not "swap one function call." Flagging because Phase 4's PPAP/pickup/EBO work reuses this same junctor-custody and multi-ccb-processor pattern (memory: "Phase 4 order rationale... establishes the junctor-custody pattern Pickup and EBO reuse") — expect similar-sized surprises there, and budget accordingly rather than assuming the plan's one-line descriptions are the true size of the work.

00:45 — Phase 2 step 4: ring-group early ringback — already fixed, test was stale

The plan's next item was the "early ringback" bug (sim_tests.cc:2721 in the original line numbering): callers to an all-busy ring group supposedly heard a ringback blip before busy, because treatment was applied before secondaries reported. Checked the current code first (P_RingGroupP::handler's Idle case doesn't apply any treatment at all; Treatment_DoubleRing only gets applied in secondary_ringing(), and Treatment_Busy in secondary_terminator_failed()'s all-failed branch) — this already matches the plan's desired fix exactly. Traced ./callp -t 4102 (all 6 RG0 members busy, dial 740) end to end: CCB[0] goes Dialtone → (no treatment) → straight to Busy, zero ringback/DoubleRing in between. The bug is gone; test 4102's // @@@ fail here comment and its weak require_busy_present() (which can't distinguish Busy from a ringback blip anyway) were just never updated after whatever earlier commit fixed the underlying code — most likely 81ddaaf "Bugfix for ringback; more tests for CFNA", from before this plan's execution started.

Did what the plan actually asked for regardless: replaced the apologetic assertion with require_treatment(0, Treatment_Busy) + .require_stable_for(2 * MS) (2 real seconds) — strict cadence classification, held stable, so a reintroduced early-ringback blip would now genuinely fail this test instead of the old facility-only check silently accepting it. Removed the stale comment.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks). Pico cross-build clean.

01:00 — Phase 2 step 5: trunk origination routing

P_TrunkOriginating (p_trunkorig.cc:38) hardcoded "107" for every incoming trunk call, with a @@@ note saying it should ask the translator. Fixed: read Trunk::cosdb->routing (TCOSRouting_Extension/_RingGroup/_None) and Trunk::db->route_target (a feature-target byte, same encoding as CFWD/CFNA/CFB), decode it with feature_target_target(), and route through a local Translator — mirrors exactly how P_Line's hotline/warmline paths already do this (p_line.cc:55-57). TCOSRouting_None or a route_target that doesn't resolve to a processor both apply Treatment_Fault, using the same NULL-check-on-insert_processor() pattern from Phase 2 step 1.

Datafilled trunk 0 -> COS 1 -> TCOSRouting_Extension + extension 107, to preserve the exact prior hardcoded behavior (ep_trunks.cc). Note for later: Trunk::cosdb is latched once at construction (db->cos read in Trunk::Trunk()) — this is safe here because every -t/-V invocation in make reg is a fresh process reading the already--t 999-persisted EEPROM files, so the constructor always sees current data; it would matter if anything ever changed db->cos at runtime within one process (nothing currently does).

Verified via ./callp -t 12,0,1,1,1 (trunk 0 rings once, wait, line 7 answers, hangs up): trace shows translation recommends P_CallExtension with digits [107], line 7 is now ringing, then answers and hangs up cleanly — confirms the routing fix works and (as a bonus) that the CallProgress_Terminated fix from step 3 also fires correctly here (CallProgress message on answer).

Did not promote tests 11/12 into make reg, contrary to this step's plan text — investigated and found a real, separate, deeper gap. ./callp -t 12,... exits with FAIL: THERE ARE STILL ACTIVE CCBS. Confirmed via git stash that this is pre-existing, not a regression from the routing fix (identical failure against the original code). Root cause: test 12 never simulates the trunk itself hanging up (no .trunk_hangup()), because tests 11/12 are "canned interactive" scenarios meant for manual -t inspection, not complete automated lifecycles. But adding a reciprocal trunk_hangup() wouldn't actually fix it either — traced the trunk hardware FSM (ep_trunks.cc:268-386) and found incoming calls have no path to TrunkState_Active at all. That state is only reached via the outbound seize sequence (Seize -> SeizeTimeout -> WaitForDialtone -> Seized -> Active); an incoming call goes Ringing -> InterRing -> (Idle, if abandoned, or back to RingingDebounce if it rings again) and never transitions to Active just because we answered it. Reversal detection (TrunkEvent_Reversal, the far end hanging up) is gated to case TrunkState_Active only — so even a correctly-scripted test answering an incoming call has no way to ever detect the far end hanging up, and the CCB is stuck forever (matches the observed leak: Trunk @ ... InterRing, never reaches Active).

This is a real, separate defect in the trunk FSM (answering an inbound call doesn't transition trunk hardware state), not something to bolt onto this step. The routing fix above is complete and verified on its own; leaving 11/12 out of make reg and flagging this for a dedicated follow-up (needs an InterRing/Ringing + "answered" -> Active transition, then reversal-triggered hangup can work for real).

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks — nothing new added to reg this step). Pico cross-build clean.

01:30 — Phase 2 step 6: resource exhaustion (two real crashes, both proven and fixed)

Junctor exhaustion (Line::emit, ep_lines.cc): allocate_junctor()'s return was dereferenced unconditionally (ccb -> junctor -> facility right after the assignment, for the Debug line) with no NULL check — a straight NULL-deref crash when the 12 junctors are all in use. ccb_alloc() itself can also return NULL (CCB pool exhausted) with the same problem one line earlier. Fixed both: on ccb_alloc() failure, log and return (nothing to undo yet); on allocate_junctor() failure, log, undo ccb_alloc()'s effect (ccb->originator = NULL; ccb->active = false; — cheaper and just as correct as calling the private, unexported ccb_free(), since no processor was inserted yet at that point) and reset the line's ccb back to NULL so it can try again on a later event. Net effect matches the plan's "congestion = silence": the 13th caller just never gets dialtone.

Added test 1043: 13 lines (0-12) off-hook at once against 12 junctors. First proved the crash directly: temporarily reverted just this fix (git stash push -- ep_lines.cc ringer.cc) and ran the new test against the original code — segfault, exit 139, at exactly the predicted ccb -> junctor -> facility dereference on line 12's failed allocation. Restored the fix (git stash pop) and reran: line 12 correctly gets congestion (require_dialtone_absent), lines 0 and 11 both get real CCBs/junctors, clean exit, no leaks. (Assertion note: initially also asserted require_dialtone_present(11), which timed out — turned out to be a different, already-known, unrelated constraint: only 2 DTMF receivers exist system-wide, so lines 2-11 are all still waiting for one and never reach dialtone even though their junctors are fine. Swapped to require_ccb_digits(11, ""), which confirms line 11 got a real CCB/junctor without depending on DTMF-RX timing.)

Ringer-phase exhaustion (ringer.cc) — a second, independent crash, same class, worse blast radius: deallocate_ringer() was the only resource deallocator in the whole codebase without the if (r) { ... } NULL guard that deallocate_junctor()/ deallocate_dtmf_rx()/deallocate_pager() all already have (checked all three for the pattern before concluding this was the outlier). Line::make_ring() already logged-and-continued on allocate_ringer() failure (setting ringer = NULL) rather than aborting the call, which is exactly right per the plan's "keep state consistent so answer still works" — the problem is downstream: LineState_RingingOffHookDebounce unconditionally calls deallocate_ringer(ringer) (no guard, unlike Line::teardown()'s already-safe if (ringer) {...}) when the line being rung is answered. So: any line that rang without getting a real ringer FSM (all 24 slots in use) would crash the moment someone answered it — a wider blast radius than the junctor bug, since ringer exhaustion is far more reachable in practice (e.g. concurrent ring groups: each can ring up to 6 members from one call, and 24 slots is not a lot of headroom against many concurrent ring-group calls).

Proved this one with a targeted, temporary change rather than constructing a full 24-ringer-exhausting scenario: set #define SLOTS 1 (from 6*4) in ringer.cc, confirmed test 4100 (ring group 0, 6 members) still completed cleanly with 5 of 6 members getting "no audible ring" congestion instead of crashing, then specifically answered one of the congested members (./callp -t 5,740,2,121,2,2 — line 121 got no ringer per the trace) to hit the exact RingingOffHookDebounce path: clean exit, "deallocate ringer 0" (a NULL pointer) logged safely. Reverted just the if (r) guard (kept SLOTS 1) and reran the identical scenario: segfault, exit 139, confirming the guard is what prevents the crash, not something else about the reduced slot count. Restored both the guard and SLOTS to their real values.

Didn't add a permanent regression test for ringer exhaustion — reaching it deterministically at the real SLOTS=24 needs a carefully constructed multi-ring-group scenario (multiple originating lines each ringing a full 6-member group concurrently) that's a good chunk of test-authoring effort for a code path already covered by the same fix class as the junctor test, and already proven both ways above. Revisit if a future feature (e.g. Phase 4C's callback-busy, which rings on a timer completely outside normal call setup) makes exhaustion more reachable in practice.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks, ringer.cc/ep_lines.cc diffs contain only the real fixes -- confirmed via git diff after all the temporary proof edits were reverted). Pico cross-build clean.

01:45 — Phase 2 step 7: PostRouting blind cast

P_LineState_PostRouting's CCBEvent_Offhook handler (m -> from == ccb -> terminator) unconditionally static_cast <Line *> (ccb -> terminator) -> connect_junctor (...), with a @@@ note admitting the Line assumption. Guarded it with ccb -> terminator -> type (the Endpoint discriminator, endpoint.h:12-23): Endpoint_Line keeps the existing connect_junctor() call; Endpoint_Trunk calls xbar_connect_trunk() instead — already the established pattern for connecting a trunk to a junctor (used the same way for outbound trunk-group calls, p_calltrunkgroup.cc:94); anything else logs and does nothing rather than casting blind.

Checked whether a trunk terminator can actually reach this branch today: CCBEvent_Offhook is only ever emitted by Line::emit() — grepped ep_trunks.cc/p_calltrunkgroup.cc for CCBEvent_Offhook and found no matches. So a trunk terminator can't currently trigger this path at all (matches the trunk-FSM gap found in step 5 — an incoming call never reaches TrunkState_Active, and outbound trunk-group calls (p_calltrunkgroup.cc) already connect the crossbar themselves without routing through this Offhook branch). This fix is correctness/defense for real today: no test added for the trunk branch specifically, since it's not reachable yet, but the guard means a future trunk-answer path (e.g. whatever fixes the step 5 FSM gap) won't silently miscast when it starts flowing through here.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks — confirms the Line branch, exercised by every line-to-line regression test, still works correctly). Pico cross-build clean.

02:00 — Phase 2 step 8: hotline/warmline datafill

p_line.cc hardcoded two numbers directly in code: "96135920442" (hotline, LCOSType_Hot, dials immediately on off-hook) and "96137451576" (warmline, dialed after the P_Collect first-digit timeout if nobody dialed anything) — both already marked @@@ in the code, and ep_lines.cc's datafill for lines 60/61 had a matching @@@ placeholder ("somehow, we need to associate this with a feature target").

Added uint8_t p_hotline to LineDB (ep_lines.h) — one field serves both hotline and warmline, since which branch fires is already determined by LCOSType; it's a speedcall-range FEATURE_TARGET_* byte (same encoding as p_cfwd/p_cfna/p_cfb), because an 11-digit number can't fit in one byte directly, only a pointer to a speedcall entry holding it can. sizeof(LineDB) stays well under DATABASE_PAYLOAD_SIZE (56) — Digits (16) + 5 uint8_t = 21 bytes.

Datafilled line 60's p_hotline -> speedcall 210, line 61's -> speedcall 220 (ep_lines.cc), and repurposed speedcall slots 10/20 — previously just cleared/empty by convention — to hold the exact same "96135920442"/"96137451576" strings (speedcall.cc), so tests 1040-1042 dial identical digits, just routed through the speedcall table instead of a string literal. p_line.cc's two hardcoded-string sites now call feature_target_target (orig -> db -> p_hotline), and both explicitly check FEATURE_TARGET_NONE first and apply Treatment_Fault (rather than relying on feature_target_target()'s "***" fallback happening to also produce Fault via a translate() non-match — same explicit-check idiom as the NULL-processor fix in step 2.1, not an accident of string matching).

Verified zero churn with the new SMDR introspection tool (Phase 1.6): ./callp -t 1040/-t 1041 SMDR traces show "digits":"6135920442" / "6137451576" — byte-identical outpulsed digits to before, now routed through speedcall 210/220 in the debug digit-stack instead of a hardcoded literal. make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks — EEPROM layout change picked up automatically by Phase 1.1's -t 999 prefix, no manual re-datafill step needed). Pico cross-build clean.

02:10 — Phase 2 steps 9-10: 3WC stub cleanup, cosmetic banners

Step 9: deleted the unreachable wait_for_death: label in p_3waycall.cc (nothing gotos to it — 3WC stays inert, deferred with the two-leg redesign per the plan's binding scope). Confirmed the -Wunused-label warning is gone: clean make clean && make -j12 warning count dropped from 9 to 8 (the 7 main.cc unused-variable warnings and the p_ppap.cc unused-function warning from the Phase 0 baseline remain, both tracked against their own plan items — 2.9 didn't touch either).

Step 10: two banner copy-paste bugs. Test 1002 printed test 1001's banner text (sim_tests.cc, the 1002 off hook, wait 45 seconds, hangup test said "TEST 1001, go offhook, wait 100ms, hangup") — fixed to describe what 1002 actually does. Test 2003 (basic trunk group hunt round robin, confirmed by its own function name and the usage-string entry) printed "TEST 2002, basic trunk group hunt max" — fixed to match.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks). Pico cross-build clean.

02:20 — Phase 2 step 11: BadDigit dead path

LineEvent_BadDigit was fully wired downstream (Line::emit maps it to CCBEvent_Error, ep_lines.cc:518-521) but never emitted anywhere. LineState_DialCompute's pulse-count-exceeded branch (pulsecount > 10) just silently forced state = LineState_OffHook with no event at all. Added the missing emit (LineEvent_BadDigit) right before that.

Traced downstream handling before strengthening the test, since "wire it up" without checking what receives it could go either way: neither P_Collect::handler nor P_LineState_Collect has a case for CCBEvent_Error — both just return ProcessorReturn_Next on it today. Confirmed via -t 1012 trace this is a safe no-op, not a silently swallowed bug: the bad digit is rejected (never reaches ccb->digits_store()), dialtone and the DTMF receiver stay up, and collection keeps waiting normally for a retry — matches real phone behavior (garbage pulse train -> try again) better than aborting the whole call would have. P_LineState_PostRouting already has a CCBEvent_Error handler (applies Treatment_Error) for the post-routing case, e.g. ring-group routing failures (step 2.1) — that one's for a different call phase and was intentionally left alone.

Strengthened test 1012 (already existed, asserting an empty digit buffer after 11 pulses) with .require_dialtone_present(0) right after the bad pulse train and before hangup — proves the collector is still alive and listening, not just that the buffer happened to stay empty. Per the plan, no new test added.

make wipedb && make reg: green (same 4 soft fails, zero aborts, zero leaks). Pico cross-build clean.

02:30 — Phase 2 step 12: CFNA timeout -> sysdb

Turned out trivial (plan flagged it as skip-if-not, but the sysdb write path was straightforward). Added uint8_t cfna_timeout to SystemDB (sysdb.h) — well within DATABASE_PAYLOAD_SIZE (56 bytes; struct is now bool + uint8_t[8][6] + uint8_t = 50). Defaulted to 30 (seconds) in both create_new_system_db (real EEPROM first-init) and sim_database_reset_sysdb (-t 999). Removed the TIMEOUT_NO_ANSWER #define from p_callforward.cc and read `sys_db -> cfna_timeout * KILO

make wipedb && make reg: green, including all of 4010-4014 (the CFNA test family, which depends on the exact 30s timing) — confirms this is truly a no-op behaviorally, just moved from a compile-time constant to a datafillable value. Pico cross-build clean.

02:45 — Phase 2 step 13 (final): line-card write-register audit

Line-card crossbar registers (mute()/relay()/connect_junctor()/ disconnect_junctor(), ep_lines.cc:122-190): code already correct, verified by tracing the simulator's own crossbar model, not just re-reading the corrected comment.

The corrected doc (earlier "Document line card write register semantics" commit, done before this plan's execution started) says bit D4 ("W") means "if 0, use line selected by d1..d0, else use 0000 (no line connected)" — a specific per-line disable. mute()/relay() don't set that bit at all; instead they OR in LINE_HW_DRIVE_DISABLE (0x0f = D3..D0 = "1111"), which sets the junctor-set select bits (D3-D2) to "11" — documented elsewhere in the same file as "not used" (only 3 of 4 possible 2-bit junctor-set codes correspond to real 8804 chips; 12 junctors = 3 chips x 4). That looked like it might be relying on undocumented/coincidental hardware behavior instead of the "official" W-bit disable disconnect_junctor() uses (LINE_HW_DRIVE_CLEAR, 0x10) — worth actually checking, not assuming either way.

Traced the full simulated write path instead of guessing: Line::write() -> sx20_write() -> sim_rw.cc -> sim_write_relay_reg()/etc (mute and relay go through the line-address write, landing in mt8804_write5(), sim_junctors.cc:26-37) -> MT8804::write5() (sim_junctors.cc:273-280). mt8804_write5() computes chip = (value >> 2) & 3 (exactly the D3-D2 junctor-set bits) and returns immediately, before ever calling MT8804::write5(), when chip > 2 ("4th is invalid" — matches "junctor set 11 -> not used" exactly). So LINE_HW_DRIVE_DISABLE (0x0f, junctor-set="11") hits this early return and never reaches the W-bit check at all — a second, independent, equally-safe no-op path through the address-decode hierarchy, one level higher than disconnect_junctor()'s W-bit path. Confirmed separately that MT8804::write5() does correctly implement the W-bit semantics the corrected doc describes (if (val & 0x10) write(addr, 0); else write(addr, 1 << (val & 3));) — so disconnect_junctor()'s use of LINE_HW_DRIVE_CLEAR is also confirmed correct, via the other path. Both mute()/relay() and disconnect_junctor() are genuinely correct, just via two different (both real, both intentional per the corrected doc) demux layers — no code change, no test change. Sim register model already matches; nothing needed "in lockstep."

sim_misc.cc:208's LED item: real gap found, deferred — not a quick fix, not on the line-card critical path. The @@@ note points at sim_write_led_data_reg() and says the "sim20" reference project (a separate, older C++ SX-20 simulator at /mnt/data/source/telephony/mitel/sx20/sim20/, sibling to this pico/ tree) has "a much better understanding." Compared them directly (sim20/misc.cc:399-433 vs this file): sim20 models the front-panel LED shift register as two separate hardware registers — a data-shift write (sim_write_led_data_reg, masks to value & 1, one bit per call) and a separate latch/strobe write (sim_write_local_led_latch, a different address, 0x2112, that triggers the actual display flush once LOCAL_LED_SIZE bits have shifted in; a third address, 0x2111, resets the rotor). callp's current version conflates all of this into one function on one address (ADDR_DIS, confirmed the only LED-related entry in sim_rw.cc's write dispatch) and doesn't mask to 1 bit. This is a real structural gap, not a one-line fix — porting it correctly needs a second real address mapped in sim_rw.cc (and I don't have a confirmed value for what that address should be from the schematics consulted so far), a split into two functions, and some way to verify the result since zero tests touch the LED display at all (it's pure front-panel diagnostics, unrelated to call processing). Time-boxed per the plan's own allowance: journaling instead of guessing at a fix I can't verify. Not a blocker for Phase 4D (EBO only needs the line-card crossbar understanding, confirmed correct above, not this LED path).

No code changes this step (audit-only, per the plan's "code already correct — journal why" branch) beyond what's documented here. make reg unaffected (already green from step 12, nothing touched).

Phase 2 (bug fixes) is now complete — all 13 steps done. Along the way: two proven crash bugs beyond the plan's original list (ring-group self-reference recursion, junctor/ringer resource exhaustion), one previously-unknown fundamental gap (ring groups never actually connected calls on answer), and one deferred structural gap (LED latch register) journaled for later. make reg and the Pico cross-build have stayed green after every commit in this phase. Moving to Phase 3 (coverage backfill) next.

2026-08-22

09:12 — Phase 3 step 1: CFB coverage backfill (402X) uncovers and fixes a real crash + a real leak

Started Phase 3 (test coverage backfill) per the plan's own "moving to Phase 3 next" note at the end of the last entry. Step 1: 402X call-forward-busy coverage, which the plan flagged as implemented-but-zero-tested (p_callext.cc:176-193, datafilled line 13->25, TESTS.txt:343 had an empty header).

Self-guard audit (as instructed by the plan): P_CallExtension::intercept_busy (CFB) had no self-forward guard, unlike intercept_terminated (CFNA), which declines fwd == line -> unit. Added the same guard to intercept_busy (p_callext.cc) -- a CFB target datafilled to forward busy calls to itself would otherwise recurse through the digit-forward path relying entirely on P_Line's generic MAX_LOOPCOUNT (10 hops) instead of failing fast. Journaled per the plan's explicit "if missing, that's a real bug: add guard + journal."

Datafill: added four new CFB targets to sim_database_reset_lines (ep_lines.cc) using previously-unused extensions (units 14-59/63-64 were free -- confirmed by grepping every lines [N] datafill site): 140 (self-loop, guard test), 141<->142 (two-party loop, mirrors 4001/4002's CFWD loop tests), 143 (CFB to ring group 0).

Tests added (sim_tests.cc, TESTS.txt, Makefile -V batch):

Crash found and fixed: reentrant CCB::handle_event use-after-free. Test 4020 (the very first exercise of the dormant line-13-busy-forwards-to-125 datafill) segfaulted immediately. Root-caused with a one-off ASAN/UBSAN build (clang++ -fsanitize=address,undefined, not part of the normal Makefile -- built directly against the same source list into the scratchpad, since no sanitizer target exists in the tree) rather than guessing from gdb backtraces alone; ASAN pinpointed a precise heap-use-after-free on std::_List_iterator<Processor*>::operator++ at cc.cc:166.

Mechanism: P_CallExtensionState_Connect's busy branch does active = false; on itself and then, still inside its own handler() call (itself being invoked from the CCB's outermost for (auto p : processors) loop in CCB::handle_event), emits CCBEvent_CallProgress via a nested ccb -> handle_event(...) call -- an already-established, normally-safe reentrancy pattern (same one used for the Ringing/Busy emissions and last phase's ring-group work). But if a topology change happened earlier in the same synchronous chain (here: P_Line inserting the forwarded-to processor during the recursive digit-routing that intercept_busy() itself triggers), the nested call's own do { ... } while (topology_change) loop calls cull_dead_processors(), which physically erases and deletes the very list node the outer, still-paused loop is sitting on -- because the processor that just called active = false; on itself is exactly the node the outer loop is mid-iteration on. The outer loop's next ++iterator then dereferences a freed node. This is a real, general reentrancy hazard (any processor that does active = false; then recursively calls handle_event() after an earlier topology change in the same chain is exposed to it), not specific to CFB -- CFB just happened to be the first path that exercised it, because it was the first never-tested busy-forward.

Fix: added int handle_event_depth to CCB (cc.h), incremented/decremented around CCB::handle_event (cc.cc), and gated the cull_dead_processors() call on handle_event_depth == 1. Nested (reentrant) calls still notice topology_change and still skip inactive processors (if (!p -> active) continue, already present), they just defer the physical erase/delete to whichever frame is the outermost -- which is guaranteed to run its own cull check after its own for loop has fully unwound, i.e. with no other live iterator anywhere on the stack. scan_cc()'s existing post-handle_event cull_dead_processors() call remains as an outer safety net regardless of entry point. Verified against the original code first (temporarily reverting just the depth-guard hunks and re-running the ASAN binary) to confirm this specific change is what stops the UAF, then re-applied.

Leak found and fixed while re-testing under ASAN: CCB::teardown() never freed Treatment objects. With the UAF fixed, LeakSanitizer flagged 288 bytes / 6 allocations, all Treatment objects allocated from P_CallExtension's ringback (Treatment_DoubleRing) call. Confirmed this predates today's work and isn't CFB-specific -- test 4003 (existing, already green in make reg, unrelated to CFB) leaks the identical 288 bytes / 6 allocations under the same ASAN binary, one per ring-group member (120..125) that starts ringing and is never individually answered/hung-up before the CCB tears down. Root cause: CCB::teardown() (cc.cc) called treatment -> teardown(); treatment = NULL; instead of deleting the object -- and Treatment::teardown() was a literal no-op already carrying an @@@ marker ("feels like we should do more"). CCB::remove_treatment() (the normal in-call removal path) does it correctly (delete treatment;); the teardown path just never matched it. Fixed by making CCB::teardown() delete treatment; directly, and removed the now-dead, now-pointless Treatment::teardown() (zero remaining callers) from treatment.cc/.h. This is a real, general leak (any call that tears down while still carrying an active treatment -- ringback, busy, dial, etc. -- leaked it), not new, not CFB-specific, and not scoped to Phase 3 step 1's plan text, but cheap and safe to fix immediately since the ASAN tooling was already in hand and the fix is a one-line change with an obvious, narrow blast radius.

Verification: re-ran the ASAN/UBSAN binary against 4020-4023 -- exit 0, zero ASAN errors, zero leaks (the one remaining sanitizer line, ep_trunks.cc:268 reading an uninitialized TrunkState at startup under UBSan, predates this session, fires on plain -t 999 before any test code runs, and is unrelated to CFB/CCB work -- left alone, out of scope). Then make wipedb && make reg on the normal (non-ASAN) build: exactly the same baseline as every prior phase (4 soft "no DTMF receiver" fails, zero TEST ABORTED, zero STILL JUNCTORS/CCBS failures) -- confirms the fix didn't regress any existing test and the new 4020-4023 all pass as part of the batch. Pico cross-build (cd build && make -j12) clean.

09:34 — Phase 3 step 2: ring group coverage (4105-4108) uncovers four more real bugs

Continuing Phase 3. Step 2: ring groups 2-6 (sysdb.cc:58-97 datafills these, but only RG0/RG1 had ever been exercised by a test before today). Reused the existing RG2/RG3/RG6 datafill (never required new sysdb.cc entries) and RG4's pre-existing speedcall member.

Tests added:

Bug 1 (hang, not a crash): P_RingGroupS::Connecting's CCBEvent_NewDigits handler never re-translated. Test 4108 infinite-looped (timeout, not a segfault) the first time it ran. A ring-group secondary that routes to P_Speedcall gets a CCBEvent_NewDigits back once the number resolves ("211" -> "96135551211"), which is supposed to be re-translated and re-routed -- P_Line's equivalent PostRouting handler does exactly that (state = translator->translate()). But P_RingGroupSState_Connecting's NewDigits branch just did state = P_RingGroupSState_Route;, and Route only calls translator->insert_processor() -- reusing the stale recommendation from the translator's one-and-only earlier translate() call. So it kept reinserting P_Speedcall on the same un-re-translated "211" digits, forever. Fixed (p_ringgroup.cc) by calling translator->translate() fresh on NewDigits, exactly like P_Line. Also added a loopcount member to P_RingGroupS (p_ringgroup.h) with the same MAX_LOOPCOUNT (10) guard P_Line uses, in case a future datafill ever creates a genuine translate-time loop through a secondary -- collapses to P_RingGroupSState_Treatment (reusing the existing busy/fail path) instead of looping.

Bug 2 (crash): another reentrant-handle_event use-after-free, this time in P_RingGroupS::~P_RingGroupS(). With the hang fixed, the combined 4xxx batch (not the isolated 4105-4108 run -- order/CCB-numbering dependent) crashed: processor->active in the secondary's destructor dereferenced a P_CallExtension that cull_dead_processors() had already deleted earlier in the same pass (both were marked inactive together; list order put P_CallExtension first). Root cause is structurally the same class as the CFB UAF from the last step (a raw pointer to a sibling processor going stale when that sibling is culled independently), just via a different member (processor, not reentrant handle_event() itself). Fixed by not dereferencing processor directly: re-derive "is there still a live processor on this ccb besides me" by scanning ccb->processors fresh (anything already erased this pass is simply absent from that list, so this can't observe a stale pointer).

Bug 3 (crash): the identical hazard, cross-CCB, in P_RingGroupP. Same combined-batch run then crashed in P_RingGroupP::~P_RingGroupP(), dereferencing secondaries[i]->active where secondaries[i] pointed at a P_RingGroupS that had already been culled on its own (separate) ccb. secondary_connected() has the identical pattern (secondaries[i]->active = false; in a loop) and is equally exposed, just not yet proven to crash. Since secondaries live on different ccbs than the primary, the "scan ccb->processors" trick doesn't directly apply -- but each CCB the primary points at via ccbs[i] is a slot in the static, never-freed CCB ccbs[NCCBS] array (only ever reset, never deallocated), so comparing a raw P_RingGroupS* pointer value against that ccb's live processors list is always safe (no dereference of the possibly-freed object). Added a small secondary_still_alive() helper (p_ringgroup.cc) and used it in both secondary_connected() and the destructor before touching secondaries[i].

Bug 4 (crash, and its precondition -- both real, both pre-existing, both outside ring groups): trunk origination was completely untested until 4108 was the first thing in the whole suite to ever dial 8/9. Fixing bug 1 let the resulting "9..." digits correctly route to P_CallTrunkGroup, which actually tries to seize a real trunk -- and that's where things got interesting. Trunk::state (ep_trunks.h) had no default member initializer (every other state machine in this codebase does -- P_LineState state = P_LineState_Idle; etc. -- this one didn't), so Trunk::is_idle() was reading uninitialized heap memory. Confirmed with the same one-off ASAN/UBSAN build from the last step: runtime error: load of value ..., which is not a valid value for type 'TrunkState', present from the very first line of any -t 999 reset output, i.e. entirely unrelated to ring groups -- just never noticed because nothing had exercised is_idle() before. Whether trunk 0 "looked" idle was pure garbage-dependent luck: under ASAN it read as not-idle (fast Treatment_Error, which is what the test first appeared to correctly exercise); under the plain non-sanitized build it read as idle, a real trunk got seized, the call ran for several seconds, and on hangup/teardown crashed in P_LNR::~P_LNR() (ccb->originator->type) -- because a ring-group secondary's ccb is allocated with ccb_alloc(NULL) (deliberately no originator; see last step's ep_lines.cc/p_ringgroup.cc notes), and P_LNR's destructor assumed ccb->originator is always a real line. Fixed both: Trunk::state now defaults to TrunkState_Idle (ep_trunks.h), and P_LNR::~P_LNR() (p_lnr.cc) guards the dereference (ccb->originator && ...) -- LNR simply has nothing to attribute the call to for a non-line originator, which is correct: skip the last-number-store, don't crash. Neither fix is ring-group-specific; both were silently waiting for anything in the suite to dial a trunk, ever. This makes the plan's earlier deferred "incoming trunk calls have no FSM path to Active" note look related but distinct -- that one's still open and still deferred (no test exercises it, and it's about answer/incoming, not origination).

4108's final, accurate behavior (test updated to match, not the originally guessed "no trunks provisioned -> fast busy"): with all four bugs fixed, dialing RG4 correctly seizes trunk 0 and runs the outgoing call to completion entirely on the ring-group secondary's own private ccb -- but the caller (line 0) never hears anything, not even ringback, because P_CallTrunkGroup has a pre-existing, already self-documented gap (p_calltrunkgroup.cc:24, literally @@@ We should add CCBEvent_CallProgress to this) and never reports status up through the ring-group secondary/ primary chain the way P_CallExtension does. Left alone and journaled -- this is real, but it's the original author's own flagged future work, cross-cutting (affects every trunk-originated call, not just ring groups), and out of scope for a ring-group coverage step. Test 4108 asserts the honest current behavior: no ring, no dialtone, stable (silent) for 2s, then clean hangup with no leak.

Verification: one-off ASAN/UBSAN build (same non-Makefile approach as the 402X step) run against the entire make reg test list (all 69 tests, 1000-4108) start to finish: exit 0, zero ASAN errors, zero UBSan warnings (the two ep_trunks.cc ones from earlier are gone now that Trunk::state is initialized), zero leaks. make wipedb && make reg on the plain build: same baseline as every prior step (4 soft "no DTMF receiver" fails, zero TEST ABORTED, zero STILL JUNCTORS/CCBS) -- including a real segfault caught and fixed along the way (the plain, non-ASAN build hit the Trunk::state/P_LNR crash chain that the ASAN build's differently-poisoned memory had masked into "just" a UBSan warning; confirmed via gdb backtrace, not guessed). Pico cross-build clean.

Phase 3 progress: step 1 (CFB, 402X) and step 2 (ring groups, 410X) done. Next up: step 3 (line FSM edges), step 4 (speakerphone COS honesty check), step 5 (stress tests, optional).

09:52 — Phase 3 step 3: line FSM edge coverage (1003/1016/1026/1027/1037/1044/1045)

Continuing Phase 3. Step 3: line FSM edges (ep_lines.cc's hardware debounce/pulse/flash state machine), which had no dedicated boundary/edge-case tests despite being the most timing-sensitive code in the simulator.

New DSL primitive: line_glitch(id, ms, delay_ms=100) (sim_testrunner.h) -- mechanically identical to the existing line_flash (onhook, wait, offhook), just named for probing durations below line_flash's usual 300ms default, for glitch/boundary tests.

Tests added:

Self-inflicted bug while writing these tests, caught before it reached make reg: several of the new tests used .wait(N * MS) intending "N milliseconds," not realizing MS (main.h, = KILO = 1000) is used throughout this test file as a seconds multiplier for .wait() (which itself takes raw milliseconds) -- e.g. existing test 4010's .wait(35 * MS) means 35 seconds, matching its 30s CFNA-timeout intent, not 35ms. .wait(200 * MS) in a fresh test therefore meant 200 seconds, not 200ms. This first surfaced as test 1044 failing (require_ccb_digits timeout) -- traced via the ASAN build's verbose log to find the real cause: the inflated 200-second initial wait blew straight past P_Collect's own 10s interdigit timeout, killing the digit collector before the test's actual glitch-probing even began, so later digits went nowhere. Fixed by using bare millisecond values (200, 1200, 150, 60) where real milliseconds were meant, and * KILO (not * MS, to avoid the same confusion resurfacing) where seconds were genuinely intended. Re-verified every new test individually and in the full combined batch under ASAN before trusting any of them.

Design constraint discovered while writing 1026: NDIGITS (30) is unreachable through ordinary dialing. Every ttable (translate.cc) entry resolves (routes) or faults within at most 5 characters -- there's no pattern that stays "insufficient, need more digits" past a handful of digits, so P_Collect's own bounds check (P_CollectEvent_DigitBufferFull) can't be reached by dialing an extension, speedcall, or ring group access code. The only reachable overflow path is P_LNR's post-trunk-seizure digit recording (p_lnr.cc:54, ccb->digits_store() called without checking the return value, unlike P_Collect) during an outgoing trunk call's dialed number. Test 1026 dials "9" (seizes a real trunk) then DTMF-dials 35 digits (NDIGITS+5), confirming CCB::digits_store()'s bound (cc.cc:283-292, if (ndigits < NDIGITS)) is genuinely overflow-safe regardless of which caller relies on it, not just the one that checks.

Test 1027 documents, but doesn't fix, a real (pre-existing) gap: P_Collect's 30s wait-for-DTMF-RX timeout (TIMEOUT_DTMF_RX, p_collect.cc:27) correctly fires and P_Collect cleans itself up -- but P_LineState_Collect's CCBEvent_Timeout handler (p_line.cc:90) only reacts to P_CollectEvent_TimeoutWaitingForFirstDigit; the DTMF-RX-wait and interdigit-timeout subtypes fall through with no treatment applied. A caller stuck waiting 30+ seconds for a DTMF receiver (or who stops dialing mid-number) is left in silence until they hang up themselves -- confirmed by testing, not just reading the code (existing test 1021 already hinted at this: it only asserts DTMF RX gets released after its own timeout, never that any treatment is applied). No crash, no leak, no hang -- just an honest UX gap, journaled per the same "honest minimal behavior beats silent empty states" principle the plan applies to the speakerphone COS step. Out of scope to fix here (would mean redesigning P_Line's Collect-state timeout handling generically, well beyond "line FSM edge coverage").

Note: Trunk::state's initialization fix from the previous ring-group step is exactly why test 1026 (which seizes a real trunk) and the entire pre-existing 2000-series trunk suite continue to pass reliably -- confirms that fix was correct and general, not ring-group-specific, as journaled last step.

Verification: every new test run individually and in combination under the one-off ASAN/UBSAN build; the grand-total run (all 82 tests across the whole make reg list, 1000-4108) came back clean: exit 0, zero ASAN/UBSan errors, zero leaks, the same 4 known soft "no DTMF receiver" fails and 5 expected "LOOP COUNT EXCEEDED" lines (4001/4002/4022's loop tests, appearing across the full combined run). make wipedb && make reg on the plain build: identical baseline to every prior step. Pico cross-build clean (no source files relevant to the embedded target changed this step -- sim_tests.cc/sim_testrunner.h are simulator-only).

Phase 3 progress: steps 1 (CFB), 2 (ring groups), and 3 (line FSM edges) done. Remaining: step 4 (speakerphone COS honest-fault test, small) and step 5 (stress tests 9000/9001, optional -- add to reg only if runtime is acceptable).

09:56 — Phase 3 step 4: speakerphone COS honest-fault test (1046) uncovers a treatment-churn bug

Small step, as the plan expected: P_LineState_SpeakerPhoneIdle (p_line.cc:134-137) was a genuinely empty state (comment only, no code) -- speakerphone origination was never implemented, and going off-hook on a speakerphone-COS line (LEN 62) left the line completely silent forever. Per the plan's "honest minimal behavior beats silent empty states" principle, changed it to apply Treatment_Fault and added test 1046 to assert it.

Bug found while writing the test (not in the empty state itself, but in the fix): Processor::treatment() (processor.h:108-115) unconditionally does remove_treatment(curtreat_); add_treatment(t); on every call -- correct and harmless for the idiom used everywhere else in this codebase, where every other treatment() call site transitions away from its state immediately afterward (call once, then leave). But P_LineState_SpeakerPhoneIdle is the first state that calls treatment() from inside a state it stays parked in indefinitely across every subsequent CCBEvent_Poll -- so an unconditional call there churns (destroys and rebuilds) the Treatment object every single 5ms simulator tick, forever. First caught as test 1046 timing out on require_treatment; traced via the debug log to the remove_treatment/add_treatment Fault pair repeating on every tick, meaning the cadence tracker never saw a stable, continuous pattern to match. Fixed with a curtreat_ != Treatment_Fault guard so it's applied exactly once. Left the general Processor::treatment() helper itself alone -- churn is only a problem for a persistent-state caller like this one, and singularizing every call site would be a much bigger, unrelated change than this step calls for.

Verification: grand-total ASAN/UBSAN run across all 83 tests (1000-4108, the entire make reg list): exit 0, zero errors, zero leaks, same known baseline (4 soft fails, 5 expected loop-count-exceeded lines). make wipedb && make reg on the plain build: same baseline as every prior step. Pico cross-build clean.

Phase 3 progress: steps 1-4 done (CFB, ring groups, line FSM edges, speakerphone COS). Remaining: step 5 (stress tests 9000/9001, explicitly optional -- "add to reg only if runtime acceptable; else leave manual"). Phase 3's core coverage-backfill goals are met; stress tests are a bounded, lower-priority nice-to-have. Next: either step 5, or move on to Phase 4 (features) per the plan, pending direction.

10:20 — Phase 4A: PPAP completion (park/retrieve), skipping straight past step 5

User said "continue with phase 4"; Phase 3 step 5 (stress tests) is explicitly optional in the plan and left for later/never. Starting Phase 4A: PPAP (dial 0) park-then-retrieve, the plan's chosen first feature because it "establishes the junctor-custody pattern Pickup and EBO reuse."

Pre-existing state was non-functional, not just incomplete. Before touching anything, I traced the whole flash-park flow by hand against p_line.cc's flash handling (which moves the pre-flash terminator to ccb->thirdparty and clears ccb->terminator) and found the original p_ppap.cc had three latent bugs that meant park never actually worked:

  1. P_PPAPState_Idle checked ccb->terminator to detect "there's a party on the call from a flash" -- but p_line.cc's flash handling clears terminator and sets thirdparty, so that check was always false on the only code path that reaches it. park_terminator() was consequently never called from the real flash-and-dial-0 flow.
  2. Even fixing (1), calling park_terminator(ccb) alone doesn't survive CCB::teardown() -- once the parker hangs up and every other processor goes inactive, CCB::teardown() unconditionally tears down originator/terminator/thirdparty and frees the junctor; a raw file-static Endpoint *car gives no way to stop that, so the "parked" party would get disconnected the instant the parker hung up.
  3. P_PPAPState_WaitForHangup (the state reached after successfully connecting to a parked call) never sets active = false on any event -- the processor, and therefore its ccb, would never be culled, leaking a ccb + a P_PPAP forever on every successful retrieve (and on every busy/error path that jumps to wait_for_hangup, which has the same gap).

Design (deviates from the plan's literal sketch, journaled per the plan's own "journal the deviation" convention used elsewhere): the plan sketches static struct { Endpoint *party; Junctor *junctor; } park_slots[NPARK]. Taken literally, that requires detaching the parked party from any ccb (ccb->thirdparty = NULL with no replacement), which leaves the party's own Line::ccb pointer either null (and then their own physical onhook, per Line::emit(), gets misread as a fresh origination instead of a hangup) or stale (pointing at a ccb that gets freed and reused, silently corrupting an unrelated future call). Instead, park_slots[NPARK] holds CCB*: parking works by keeping the original ccb alive (its P_PPAP stays active, holding the party as ccb->thirdparty, in a new P_PPAPState_Parked state) rather than detaching raw Endpoint/Junctor pointers. The parker's own line is explicitly torn down (parker->teardown(), clearing their Line::ccb so they can originate again) while the ccb structure itself and the parked party stay put. Retrieval then transplants ccb->thirdparty and the real junctor onto the retriever's own (fresh) ccb -- the same "adopt into a live ccb" idiom the plan describes for Pickup (4B) -- and lets the now-empty original ccb wind down normally via the existing, already-correct CCB::teardown(). This also fixes bug (3) above along the way (WaitForHangup now sets active=false on Onhook, matching the existing Paging state's pattern), and adds a defensive sweep in P_PPAP's destructor that purges any stale entry pointing at this->ccb from park_slots[] in case a future bug takes a path that skips the explicit reservation cleanup.

Two more real bugs found only by actually running the new tests (not by inspection):

Tests (404X, new TESTS.txt header): 4040 park then retrieve from a different line (asserted with a new require_line_line_connection()/detect_line_line_connection() DSL primitive that reads the actual crossbar wiring, mirroring the existing line-trunk version -- this is a real, strong assertion, not a proxy); 4041 same, but the same line retrieves its own park; 4042 park with the single slot full -> busy, and the original park is unaffected and still retrievable afterward; 4043 dial 0 with nothing parked -> plain paging, asserted honestly (no dialtone, stable) since no treatment is applied to the paging leg; 4044 the parked party abandons (hangs up) before anyone retrieves -- proven by successfully parking and retrieving a second, unrelated call through the same single slot afterward (if the slot or junctor had leaked, this would either busy-out or fail the leak check). Deliberately left unfixed and journaled here rather than in-code (not exercised by any of the 5 tests, and the plan's scope for this step doesn't call for it): flashing a second time during ParkedPaging to "reconnect" instead of parking is a real gap in the original code (P_Line's own PostRouting state has no Flash handling either), not something introduced this step; the reservation is still released correctly on that path so it can't leak a slot, but the actual reconnect-to-the-flashed-party behavior doesn't happen. Also out of scope this round: parking a trunk (flash mid trunk-access call) -- declined gracefully with busy rather than mishandled, guarded by an Endpoint_Line type check.

Verification: new tests pass individually and as part of the full -V4040,4041,4042, 4043,4044 batch; grand-total ASAN/UBSAN run across the entire make reg list (1000-4108, now 88 tests) came back clean -- exit 0, zero ASAN/UBSan errors, same known "LOOP COUNT EXCEEDED" lines from the pre-existing loop tests. make wipedb && make reg on the plain build: identical to the established baseline (4 soft DTMF-RX fails, zero aborts, zero leak failures). Pico cross-build clean (p_ppap.cc/.h are already in both build files; no new files added, so no CMakeLists.txt change needed).

Next: Phase 4B (Call Pickup), which the plan explicitly designed to reuse this same cross-ccb custody idiom -- and per the bug found above, will need the same line->ccb = target_ccb re-pointing when adopting the picked-up line.

11:12 -- Phase 4B: Call Pickup

New p_pickup.cc/.h from p_TEMPLATE, wired into processor.h (factory case and dump-switch case uncommented; ProcessorNameString was already active) and both build files (Makefile's OBJECTS/COMMON, CMakeLists.txt). ttable's {"3", PName_Pickup} entry was already in place from an earlier phase.

Design. On Idle+Poll, scan lines[] (for (Line *l = lines; l < lines + MAX_LINES; l++)) for the lowest-unit line with is_ringing(). None found -> Treatment_Error (reorder/fast busy) -> WaitForHangup, same observable cadence test 3003 already expected before this phase existed (updated its comment/name below). Found: this is the same cross-ccb custody idiom 4A built (PPAP retrieve), reused here per the plan's own framing -- the picker's freshly-dialed CCB dissolves without tearing the picking line down, and Line::ccb gets re-pointed at the CCB that survives.

The one real design question 4A didn't have to answer: which CCB survives, when the ringing line found might be an ordinary call's terminator, or one member of a ring group. For an ordinary call, the ringing line's own ->ccb is the real CCB (has the real junctor, P_Line, everything). For a ring-group member, ->ccb is the secondary's own member CCB (allocated with no originator, so no junctor of its own -- confirmed by reading P_RingGroupP::handler's Idle case, ccb_alloc (NULL)) -- the real junctor lives on the primary's CCB, the one hosting the original caller.

Rather than reimplement "what does an answer do" (clear the originator's ringback treatment, insert SMDR, and -- critically -- run P_RingGroupP::secondary_connected()'s cascade that stops every other member from ringing), Pickup synthesizes the exact same CCBEvent_Offhook a real answer would generate -- from = the ringing line, delivered via member_ccb -> handle_event (...) -- before touching any custody pointers, so it matches what P_CallExtension captured as its own line when the call started ringing. This is a new pattern for this codebase (grepped every existing ccb -> handle_event (CCBEvent_...) call site first -- all of them fire into their own ccb field, never a different one), but it's exactly what the plan asked for ("synthesize CCBEvent_Offhook ... into the target CCB so the existing answer machinery ... completes normally"), and cross-ccb reentrancy is safe here: each CCB's handle_event_depth and processors iterator are per-object, so a call into a sibling CCB's handle_event() from inside this CCB's own processor loop doesn't touch any live iterator.

One side effect of firing the event before the custody swap: for an ordinary (non-ring-group) call, P_Line's own PostRouting handler (which also lives on that same ccb) matches m->from == ccb->terminator and calls connect_junctor() on the old, about-to-be-abandoned ringing line -- a redundant, harmless connect, since Line::hwshadow is a per-line hardware register (each line has its own hwaddr), so this can't collide with the picking line's own register. It gets immediately undone a few lines later by found -> teardown ().

After the synthetic offhook: resolve real_ccb (== member_ccb for an ordinary call, or rgs -> primary -> ccb for a ring-group member, walking up through any further nesting the cascade above already collapsed -- ring groups can nest, capped at MAX_RINGGROUP_DEPTH). For the ring-group path, secondary_connected() already deactivates every P_RingGroupS in the chain (it's in each primary's own secondaries[]), but it never touches the P_CallExtension (or nested P_RingGroupP) each one was fronting -- rgs -> processor (and each outer -> processor while walking) gets explicitly deactivated, or it would sit forever in WaitForHangup on a now-orphaned member ccb that no endpoint's ->ccb points at any more: a genuine, silent CCB+processor leak that the leak checker would eventually catch once enough pickups exhausted NCCBS.

Then the actual junctor transplant, same disconnect-then-connect ordering 4A's Bug A taught (same line, same hwshadow register -- old connection must be cleared before the new one is written, or the write ordering can clobber it): picker -> disconnect_junctor (ccb -> junctor), xbar_connect_line (real_ccb -> junctor, picker), deallocate_junctor (ccb -> junctor), ccb -> junctor = NULL, ccb -> originator = NULL, real_ccb -> terminator = picker, picker -> ccb = real_ccb (the Bug B re-pointing lesson, applied from the start this time instead of found the hard way). Finally found -> teardown () resets the abandoned ringing line to Idle, and -- since this ccb (ccb, Pickup's own) now has no originator and will never receive another real event -- both P_Pickup (active = false) and the P_Line still sitting in PostRouting on the same ccb (find_processor (PName_Line), active = false) are explicitly wound down so the now-empty ccb can be reclaimed by scan_cc()'s ordinary processors.size()==0 cull.

Test 3003 (Phase 2-era, "dial 3, no pickup processor yet") renamed/retargeted to test_3003_dial_pickup_nothing_ringing -- same DSL assertions (Treatment_Error), same observable behavior, now exercising P_Pickup's own reorder path instead of P_Line Route's NULL-processor fallback. Confirms the plan's own prediction in that test's old comment.

Tests (405X, new TESTS.txt header): 4050 basic pickup (A calls B, C dials 3; B stops ringing, A and C bridged, asserted with require_line_line_connection -- real crossbar ground truth, not a proxy); 4051 dial 3 with nothing ringing -> reorder; 4052 pickup of a ring-group call (dial 740 rings 20-25, a different line dials 3; asserts 20, 21, and 25 all stop ringing -- not just the one picked -- and picker/caller end up bridged) -- this is the one that actually exercises the secondary_connected() cascade path; 4053 two independent calls ringing at once, a fifth line dials 3 -> the lower-unit call is the one picked (require_line_not_ringing on it), the other call is untouched and asserted stable (require_stable_for), then cleaned up by having its own originator abandon it.

Deliberately left unexercised, not unfixed: nested ring-group pickup (a ring group whose member is itself a ring group). The walk-up loop resolving real_ccb handles it (each level's P_RingGroupS -> processor gets deactivated the same way, and the loop keeps walking until no further P_RingGroupS is found), but no test drives it -- consistent with the existing codebase, where even a real (non-pickup) answer to a nested ring-group member has never been exercised either (test 4104 only checks ringing/busy routing, never an answer). Worth noting for whoever eventually writes that test: it would exercise the exact same P_Line PostRouting-never-sees-the-member's-own-offhook gap this session's design work turned up while tracing how a real ring-group member answer completes its audio path -- the connect only happens because P_CallExtension's own committed line matches, not because P_Line (which lives only on the primary's ccb) ever sees that specific event.

Verification: -V4050,4051,4052,4053 isolated run clean (exit 0, no FAIL/ABORT/STILL). make wipedb && make reg (93 tests now) matches the established baseline exactly (4 soft DTMF-RX fails, zero aborts, zero leak failures). Grand-total ASAN/UBSAN rebuild across the full make reg list clean (zero sanitizer errors; same pre-existing "LOOP COUNT EXCEEDED" debug lines from 4001/4002/4022's own loop-detection tests). Pico cross-build (make -j24) clean -- only pre-existing, unrelated pico-sdk warnings.

Next: Phase 4C (Automatic Callback-Busy, code 6) -- a genuinely different shape of work (a queue plus a superloop tick, not another CCB-adoption feature), so none of this phase's custody-pattern lessons carry over directly the way 4A's carried into 4B.

13:52 -- Phase 4C: Automatic Callback-Busy

New p_callbackbusy.cc/.h (two processors, P_CallbackBusy for dialing 6 and P_CallbackRing for the ring-back, plus the superloop monitor -- all one file, per the plan), wired into processor.h (two new ProcessorName values, factory, dump-switch), both build files, ttable's {"6", PName_CallbackBusy}, and one line in main.cc's superloop (callback_busy_tick(), right after scan_cc()).

Arming. Went with the plan's primary suggestion, not the fallback: Line gained two runtime (not persisted -- same category as ringer/hwshadow, not datafill) fields, last_busy_target/last_busy_time, set by a new hook in P_CallExtension's real-busy branch (station-to-station only; CFB-intercepted busy doesn't count -- the caller never actually heard it). Dialing 6 within CALLBACK_ARM_WINDOW (10s) of that arms a CallbackRequest in a static callbacks[16] array; dialing 6 with the window expired instead clears any of this line's already-pending requests (the plan's "pragmatic substitute for the manual's 444 reset"); dialing 6 with neither an armable busy nor anything pending reaches reorder (Treatment_Error) -- this is what makes test 4065 work: a ring-group-all-busy never touches last_busy_time at all (P_RingGroupP's busy path is entirely separate code from P_CallExtension's), so it falls straight through to reorder, matching the plan's own expected result for that test without any special-casing.

The plan's documented fallback ("arm against LineDB::last_number if the collector doesn't cooperate") was never needed -- the primary mechanism worked on the first design, confirmed by test 4060 passing outright. Also explicitly out of scope, journaled rather than attempted: trunk-group targets. The plan says "stations + trunk groups", but there's no FEATURE_TARGET_* encoding for a trunk group anywhere in this codebase (the range stops at ring groups, FEATURE_TARGET_RINGGROUP_END), and P_CallTrunkGroup's own busy path is entirely separate from P_CallExtension's -- representing an armable trunk-group target would mean inventing an encoding from scratch. None of the plan's own 406X tests need it.

The monitor (callback_busy_tick(), not a processor -- called unconditionally from the superloop, needs no CCB while merely queued) fires the oldest still-queued request per target once both ends are actually idle (LineState_Idle for the originator, is_available() for the target -- deliberately different checks: the originator gate is about physically being reachable to ring, the target gate needs to match exactly what P_CallExtension itself would consider free). FIFO-per-target is enforced by never letting a request fire while an older request for the same target is still active (queued or mid-ring) -- test 4064 exercises this directly with two originators against one target. Ringing uses RingerType_ShortDouble for a distinctive cadence, per the plan's suggestion (supported directly by ringer.cc, no new cadence needed).

No P_Line on this ccb. The ring-back ccb is synthesized by the tick function (ccb_alloc (NULL) -- no originator, matching how ring-group member ccbs are built), so there's no P_Line to do the two jobs it normally does: connecting the answering originator's own crosspoint (P_Line's Originate state), and connecting the target's once it's dialed and answers (P_Line's PostRouting state). P_CallbackRing duplicates both by hand. The second one took two tries: the natural place to mirror PostRouting's check (m->from == ccb->terminator on a raw CCBEvent_Offhook) never actually fired, because P_CallExtension sits earlier in the processor list and, from inside its own Offhook handling, synchronously and reentrantly emits CCBEvent_CallProgress -- which reaches P_CallbackRing and (correctly) deactivates it before the outer dispatch loop ever gets back around to deliver the original Offhook to it. Diagnosed by reading the debug log line-by-line against cc.cc's handle_event() reentrancy comments (the same handle_event_depth/topology-change machinery documented from earlier phases) and confirmed by moving the connect into the CallProgress_Terminated case instead, where ccb->terminator is still the target Line at that exact point -- test 4060 then passed outright. Once the target answers or goes busy, P_CallbackRing self-deactivates and steps out of the way entirely, letting P_CallExtension own the rest of the call's lifecycle exactly as it would for a plain call (including its own Onhook handling and destructor teardown of ccb->terminator) -- no special-casing needed there.

Cancels. 4 unanswered rings is an elapsed-time proxy (CALLBACK_RING_TIMEOUT, 24s against a ring_started snapshot), the same style CFNA's own timeout already uses -- matching "~4 rings," not literally counting ring cadence edges. 8-hour expiry (CALLBACK_EXPIRY) checked in the tick loop before anything else. Parties-talked cancellation is a small hook in P_CallExtension's own answer path (callback_busy_cancel_if_talking(), exposed from p_callbackbusy.h) -- cancels any pending request between the two parties in either direction, the instant they end up talking through an unrelated, ordinary call. P_CallbackRing itself also checks callbacks[request_index].active on every Ring-state Poll, so an externally-cancelled request aborts an in-flight ring cleanly rather than ringing someone whose callback was just cancelled out from under it. P_CallbackRing's destructor unconditionally clears its own slot's ringing flag as a defensive safety net (mirrors PPAP's destructor sweep from 4A) so a request can never get stuck permanently blocking its target's FIFO queue.

Tests (406X, new TESTS.txt header): 4060 full cycle; 4061 unanswered cancels and never returns (require_stable_for); 4062 duplicate arm ignored; 4063 target busy again right at answer -> busy tone, auto re-arm, fires again later; 4064 FIFO between two originators on one target; 4065 ring-group-busy is never armable -> reorder. All six needed their own, never-reused-elsewhere line units within this subseries -- last_busy_time is real, persistent (for the life of the process) runtime state on the Line itself, not reset between tests the way sim_initialized is, so two tests sharing a line risked stale arm-window bleed from one into the next.

Two real bugs found only by running the tests, not by inspection, beyond the reentrancy-ordering one above:

  1. Every hangup-then-immediately-redial-the-same-line step needed an explicit .wait (1 * MS) first, to clear HANGUP_TIME's 1-second onhook debounce -- without it, the DTMF dial landed before the line had actually re-originated, and hit sim_testrunner.h's "no DTMF receiver associated" abort. (Already-established convention elsewhere in the suite, e.g. the PPAP tests from 4A -- just missed on the first pass here.)
  2. Test 4065's ring-group-busy setup (7 lines all going offhook to either dial or just sit busy) exhausted the system's only 2 DTMF receivers, aborting the second dial on line 14 -- fixed by having line 14 grab its receiver first (before the group members compete for the other one) for the first dial, and explicitly freeing all the busy group members before the second dial needs a receiver again, rather than holding them offhook (and their receivers, if they'd grabbed one) for the entire rest of the test.

Also found and fixed in passing: test 3000 ("unassigned code") dialed literal "6" as its example of a code with no ttable match -- broken by this phase's own ttable addition. Moved to "4" (still genuinely unassigned), since the test was written to exercise the no-match-at-all path, not that specific digit; comment and TESTS.txt updated to explain why. Caught by the full make reg run, not the isolated 406X batch (3000 runs in an earlier batch than 406X in the Makefile's reg target).

Verification: -V4060,4061,4062,4063,4064,4065 isolated run clean. make wipedb && make reg (99 tests now) matches the established baseline exactly (4 soft DTMF-RX fails, zero aborts, zero leak failures) -- after the test-3000 fix above; the first full run correctly caught that regression. Grand-total ASAN/UBSAN rebuild clean (zero sanitizer errors, same pre-existing "LOOP COUNT EXCEEDED" lines). Pico cross-build (make -j24) clean -- only pre-existing, unrelated pico-sdk warnings.

Next: Phase 4D (Executive Busy Override, code 5) -- 3-party junctor bridging with COS gating, a different shape of work again (no queue, no monitor, but real-time bridging of a third party into an existing 2-party call).

14:22 -- Phase 4D: Executive Busy Override (code 5)

Implemented p_busyoverride.cc/.h (P_BusyOverride, one processor type with four states: Idle/WaitForHangup run on the overrider's own freshly-dialed ccb to validate and, on success, transplant; Warning/Talking run on a fresh instance inserted onto the target's ccb, driving the tone cadence). ttable gains { "5", PName_BusyOverride } (the last of the two "available" slots 4C left behind).

Arming reuses 4C's exact mechanism (Line::last_busy_target/last_busy_time, BUSYOVERRIDE_ARM_WINDOW = the same 10s as CALLBACK_ARM_WINDOW) -- dial 5 within the window after hearing real busy, no separate arm digit needed, since dialing 5 itself both checks and acts in one step.

Validation (Idle+Poll, before transplant): target resolved via lookup_line_by_extension; refuse (reorder/Treatment_Error) unless the target's ccb is active, has no current treatment (catches ringing, hearing tone, AND mid-dial-after- first-digit -- P_Collect clears its own treatment once the first digit lands, so treatment==NULL alone doesn't catch "still dialing," hence the extra explicit find_processor(PName_Collect) check), and has no thirdparty already (3-way in progress). Then COS: overrider's own COS needs ebo_enable, target's COS must NOT have ebo_security -- both new LineCOSDB bits (COS 4 = ebo_enable, COS 5 = ebo_security, datafilled onto dedicated sim-only lines: 44/46/48/50/52 as one-per-test overriders, 45 as the sole protected target, mirroring 406X's "no shared lines" isolation policy since last_busy_time is the same kind of persistent runtime state here).

Transplant is the same disconnect-then-connect custody idiom as PPAP/Pickup/Callback- Busy: o->disconnect_junctor(ccb->junctor) then xbar_connect_line(target_ccb->junctor, o), free the origin ccb's own junctor, ccb->originator=NULL, target_ccb->thirdparty = o, o->ccb = target_ccb. Unlike Pickup (done once adopted), this feature needs an ongoing presence on the target ccb afterward, so a new P_BusyOverride instance (state Warning) is inserted there; the original instance just deactivates and lets its ccb wind down.

Warning tone is driven directly against the crossbar (xbar_connect_tone/ xbar_disconnect_tone on ccb->junctor with ToneFacility_Dial, which the hardware comment already documents as 350/440 Hz) rather than through the generic Treatment class: it has to sit on top of live voice on an otherwise-idle-treatment ccb, on a cadence (1s solid, then 200ms every 6s) the fixed-timebase tones[] table can't express. Driven by hand with a Poll-timer state machine, mirroring P_CallForwardNoAnswer's own style, per the plan's own suggestion.

The interesting bug, and the one real design lesson from this phase: my first cut made P_Line/P_CallExtension/P_SMDR "thirdparty-aware" -- skip self-deactivation on CCBEvent_Onhook if m->from == ccb->thirdparty, so the overrider hanging up wouldn't drop the real call. That's necessary (the original unconditional "die on any onhook" would wrongly end A-B's call when the overrider alone hangs up) but the specific form of the fix was wrong twice over:

  1. First attempt checked m->from == ccb->thirdparty: make reg (not the isolated 407X batch) caught a STILL ACTIVE CCBS leak in test 4040 (PPAP). Root cause: P_BusyOverride sits at the front of the processor list (push_front) and clears ccb->thirdparty as part of its own cleanup before P_SMDR/P_CallExtension/P_Line -- further back in the same list, same dispatch pass -- ever get to check it. By their turn, thirdparty is already NULL, so the "skip if from thirdparty" check never engaged and they died correctly for this specific case, but the fix as shaped was fragile.

  2. So I flipped it to check the fields that don't get mutated by EBO -- m->from != ccb->originator && m->from != ccb->terminator -- and immediately broke test 4040 a different way: PPAP's own park bookkeeping (P_PPAPState_ParkedPaging, also at the front of the list) legitimately sets ccb->originator = NULL when the parker hangs up, while a leftover P_CallExtension/P_SMDR/P_Line from the original pre-flash 2-party call are still on the same ccb, correctly expected to die right then. With the field already nulled by the time their turn came, my "must still match originator/terminator" check backfired the other way: they never matched, so they never died, and the ccb leaked. PPAP repurposing ccb->thirdparty/ccb->originator for its own custody bookkeeping (see p_ppap.cc's own big comment) meant there's no ccb-field snapshot immune to both features' mutations.

    The actual fix: revert P_Line/P_CallExtension/P_SMDR entirely back to unconditional "die on any onhook" (their original, simpler, and -- for every case except this one -- correct behavior). Instead, P_BusyOverride itself, on the overrider's own onhook, returns ProcessorReturn_Consumed after its cleanup, so that event never reaches the other processors on this ccb at all. For the other onhook case (an original party hangs up, "all drop"), P_BusyOverride does nothing special -- doesn't even touch ccb->thirdparty -- and just returns Next like everyone else; once every processor (including it) has deactivated, CCB::teardown() tears down originator/terminator/ thirdparty unconditionally on its own, which already is the desired "all drop" behavior, for free. Net effect: far less code, and no reliance on any ccb field staying stable across a single dispatch pass -- the general lesson from 4C's reentrancy bug (a processor's handler can affect what other processors on the same ccb see within the same event) applies just as much to plain mutation-then-read as it does to nested handle_event() calls.

Fidelity shortcut, journaled per the plan's own text: when the overridden station hangs up, or either original party flashes, the overrider is simply dropped to silence (their Line is torn down/idled) rather than given a proper reorder cadence. Keeping them on a live treatment-bearing ccb slice through the disconnection, just to play them a tone before the drop, would add real complexity for a simulator-only feature the plan itself flagged as optional ("or silence ... @@@ note if no junctor available"). If their handset is still physically off-hook afterward, the normal off-hook debounce picks them up again as a fresh origination (dial tone) or, if no edge is detected, just sits silent -- either is an acceptable approximation.

Tests 407X (new TESTS.txt header): 4070 full cycle (bridge in, 1s warning tone, then periodic 200ms/6s bursts, verified via require_dialtone_present/absent against the shared junctor rather than a fixed Treatment cadence, since this tone isn't one); 4071 refused on a ringing target; 4072 refused on EBO-security COS; 4073 overrider hangs up, original call continues, bursts stop; 4074 either original party flashes, overrider dropped, original call continues. One early test-authoring bug caught by the isolated batch itself (not make reg): I put B off-hook before A dialed in several of these, which makes B busy instead of ringing -- fixed by only taking B off-hook to answer, after require_line_ringing.

Verification: -V4070,4071,4072,4073,4074 isolated run clean. make wipedb && make reg (109 tests now) matches the established baseline exactly (4 soft DTMF-RX fails, zero aborts, zero leak failures). Grand-total ASAN/UBSAN rebuild clean. Pico cross-build clean -- only pre-existing, unrelated pico-sdk/newlib warnings.

Next: Phase 4E (call-forward family completeness audit) -- expected to be journal-only, no code, per the plan's own text.

15:05 -- Phase 4E: call-forward family completeness audit

Journal-only, as the plan anticipated -- no source changes, one README.txt note.

Self-guard audit (the plan's specific ask: does intercept_termination's loop protection match intercept_terminated's -- CFNA's -- self-check?). It doesn't, and that turns out to be intentional-by-omission rather than a bug:

Loop protection breadth. Confirmed by tracing the code (not by adding a test) that CFNA shares the exact same breaker as CFWD/CFB: P_CallForwardNoAnswer's eventual forward also goes through ccb->handle_event(CCBEvent_NewDigits, ...), landing in the same P_Line::loopcount-guarded PostRouting path on the same long-lived P_Line instance. A genuine mutual-CFNA bounce (A forwards-no-answer to B, B forwards-no-answer to A, neither ever answers) would hit MAX_LOOPCOUNT after 10 unanswered 30s timeouts (300 simulated seconds -- "free" per the established sim-time convention) and fast-busy, same as 4001/4002/4022. No dedicated test drives a CFNA bounce all the way to that limit the way those three do for CFWD/CFB, unlike test 4014 which demonstrates a correct 2-hop CFNA bounce (165->66->65) without exhausting the counter. Judged a minor, optional test-coverage gap rather than a functional one (the mechanism is proven, just not by a CFNA-specific test at the boundary) -- no test added, matching this phase's "expect no code" framing.

Ring-group targets. Confirmed present for all three families, not just CFWD: 4003 (CFWD to a ring group), 4105 (a ring-group member's own CFNA still fires from inside a P_RingGroupS), 4106 (same for CFB). Matches the plan's claim.

CFFM. Added a note in README.txt (after the two-leg redesign's state-table Notes, next to where "call forward follow me" is named in note [2]) making the existing PLAN.md Phase-0 deferral locally discoverable at the point CFFM is actually described, rather than only in PLAN.md. No functional change -- CFWD/CFNA/CFB stay on fixed datafill targets (p_cfwd/p_cfna/p_cfb), never live-redirected mid-call.

No make reg/build changes needed (no source touched); git diff --stat confirms only README.txt plus the usual auto-generated build.h/version.h churn.

Next: Phase 4 is now complete (4A PPAP, 4B Pickup, 4C Callback-Busy, 4D Executive Busy Override, 4E audit). Remaining PLAN.md items are Phase 3.5 (9000/9001 stress tests, noted as optional/bounded) if not already covered, and final overall verification -- worth re-reading PLAN.md in full before declaring the plan done.

15:14 -- Phase 3 step 5: stress tests 9000/9001

Neither actually existed before this. TESTS.txt had prose specs for both, written back when the test catalog was first sketched out, but no code -- 9000's spec exactly matched an already-implemented, undocumented ad-hoc test (test_101_line_hook_stress, dispatched via -t 101, no TESTS.txt entry, no case-number relationship to "9000" at all), and 9001 didn't exist anywhere.

9000: renamed test_101_line_hook_stress -> test_9000_line_hook_stress, case 101 -> case 9000 in the dispatch switch, so -t 9000[,p1[,p2[,p3]]] actually matches its own documentation (previously -t 9000 just errored "no such test"). Confirmed nothing else referenced the bare number 101 (grepped TESTS.txt; "101X" there is the unrelated 1010-1016 regression subseries). Left the implementation itself untouched -- it's a continuous, never-self-terminating random offhook/onhook generator by design, meant to be run by hand with -M to bound wall-clock time. Not added to reg: -M's simulator_exit() path runs through the exact same leak check (STILL JUNCTORS CONNECTED / STILL ACTIVE CCBS) that makes the rest of the suite trustworthy, and cutting a continuous stress test off mid-flight always trips it (lines mid-call, junctors connected) -- that would be a false failure, not a real one, so reg and this test are structurally incompatible. Documented the reasoning in both TESTS.txt and the code comment so nobody "fixes" this later by trying to force it in.

9001: new, from scratch. Built as a CompactTestRunner chain assembled in a loop (Fisher-Yates over sim_optL lines for the offhook order, a second independent shuffle for the hangup order, 2s between each event either way) rather than test 9000's hand-driven Poll style, because unlike 9000 this one is genuinely bounded -- every line goes off-hook once and back on-hook once, then .finish() -- so the standard leak check at the end is a real, meaningful assertion, not a structural mismatch. rand() is never seeded anywhere in this program, so the "random" order is actually the same fixed sequence on every run -- confirmed by diffing two back-to-back runs: identical except for ASLR'd pointer values in the debug log.

Ran it standalone first (-t 9001) before touching the Makefile, per the plan's own "add to reg only if runtime acceptable" test. Result: clean exit, zero unexplained FAIL/ STILL lines, 0.22s real time for ~4m50s of simulated coverage (72 lines * 2 phases * 2s apart) -- trivially cheap to add.

Confirmed TESTS.txt's own "@@@ we'll run out of junctors too!" note, and it's benign. With only MAX_JUNCTORS (12) shared across up to 72 lines all going off-hook with no intervening hangups, junctor allocation starts failing partway through the offhook phase (Line::emit logs "could not allocate a junctor, ignoring event (congestion)" and leaves that line's ccb NULL, though Line::state still correctly reaches OffHook -- the state transition happens in Line::scan() before emit() is even called, unconditionally). The interesting part: later hanging up one of these congestion-denied lines calls Line::emit(LineEvent_Onhook) with ccb == NULL, and emit()'s ccb-bootstrap prologue doesn't distinguish why ccb is NULL from what event triggered it -- it unconditionally treats any event on a CCB-less line as a fresh origination attempt, so a bare Onhook can momentarily allocate a brand new ccb + junctor + P_Line, exactly as if it were an Offhook. This is harmless only because every processor (P_Line included) unconditionally deactivates on CCBEvent_Onhook regardless of what state it's in, so the phantom ccb gets torn down again in the very same event dispatch, before scan_cc()'s next pass -- confirmed in the debug log: allocate, dispatch, immediately deactivate, teardown, free, all in one Line[N]::emit Onhook block. Self-healing, not a leak, and the 12+-lines-congested scenario this requires never arises outside a stress test like this one -- noted here rather than "fixed," since there's no observable bug to fix (a guard limiting the ccb-bootstrap prologue to LineEvent_Offhook specifically would be more obviously correct, but this phase is about finishing the stress tests, not auditing Line::emit, and the current behavior is already provably safe).

Verification: -t 9001 standalone clean (both counted above). make wipedb && make reg (110 tests now, -V9001 appended as the final reg line) matches the established baseline exactly. Grand-total ASAN/UBSAN rebuild clean. Pico cross-build unaffected (sim_tests.cc is simulator-only, not in CMakeLists.txt's source list) and confirmed to still build clean.

With this, PLAN.md's Phase 3 step 5 (previously explicitly deferred as optional) is done, and every phase in the plan is now complete.