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
- Deleted
p_line.cc.bak: a pre-fix backup ofp_line.cc. Diff against the current file shows it predates the "terminator joining a parked call" fix (theif (ccb->thirdparty) { ... join ... } else { ... park ... }branch) — a stale backup, not in-progress work. - Deleted
z: a stale manual debug capture of test 4030 showingFAIL: THERE ARE STILL JUNCTORS CONNECTED(junctor leak on line 1). Per the plan this was the leak that got fixed; current test 4030 passes cleanly (confirmed in this session's baselinemake regrun below). - Kept
states.svg(two-leg CCB redesign design doc) and added it to git — it documents work that is explicitly deferred, not abandoned, so it's worth tracking rather than leaving as a loose untracked file. .gitignorealready coveredbuild,pico_sdk_import.cmake,*.o,callp,database.0,database.1. Addedregression.txtandobj/(the object-file directory the Makefile actually uses;*.oalone doesn't matchobj/*.ocleanly in status output, andregression.txtis a generated artifact ofmake reg).
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:
main.cc:32-39— 7x-Wunused-but-set-variable(loopcount,time_scan_lines/trunks/dtmfs/cc/eeprom/ringer) — superloop bookkeeping vars that are set but never read back. Not touched by this plan's scope; noted here for completeness, not scheduled for cleanup.p_3waycall.cc:34— unreachablewait_for_death:label. Plan item 2.9 removes this when stubbing 3WC as inert.p_ppap.cc:43— unusedfind_parked_callfunction. Plan item 4A ("remove or use deadfind_parked_call()") addresses this.
(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:
- Continuous tones (
length == 1, e.g. Dialtone/Quiet): plain instantaneous facility check (chain.require_stable_for()for strictness — composing with Phase 1.2's primitive rather than duplicating it). - Cadenced + repeating (Busy/Error/Fault/Ring/DoubleRing/
StutteringDial): rotation-invariant match over >= 2 full observed
cycles (phase of the cycle we started watching in is unknown), with a
tolerance of
max(2 ticks, 10% of expected duration)per run. - Cadenced + one-shot (InitialStutterDial): fixed-prefix match from
the start, assuming
require_treatment()is armed at/near the moment the treatment is first applied (true for all current callers) — then confirms the final state holds.
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.h → simulator.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:
-
3000/3001/3002 (dial
6,1*,12*— invalid/insufficient digits): TESTS.txt says "fast busy" (Treatment_Error, 120 IPM). The code appliesTreatment_Fault(240 IPM) — confirmed via./callp -t 3000Debug trace,add_treatment Fault. This is theTranslator::translate()"no ttable entry matched at all" path (translate.cc:109), which unconditionally setsTreatment_Fault. -
3010/3011/3012 (dial
290/298/296— unassigned speedcall, 1- and 2-element speedcall loops): TESTS.txt says "fault" (Treatment_Fault). The code appliesTreatment_Error— confirmed via./callp -t 3010/-t 3011traces, both showP_Line PostRouting received error message/*** ERROR, LOOP COUNT EXCEEDED ***→add_treatment Error(p_line.cc:151,157). These all go throughCCBEvent_Erroror thePostRoutingNewDigitsloop-count-exceeded branch, never throughTranslator::translate()'s own Fault path, because"2XX"does match the ttable — the failure is discovered downstream (empty speedcall entry, or loop-count exceeded), not by the translator itself.So today's actual behavior is exactly the mirror image of what TESTS.txt documents: user-dialing-errors (3000-3002) get the system-fault tone, and datafill/loop problems (3010-3012) get the user-error tone. Not fixed — this reassigns real user-facing tone behavior and isn't in PLAN.md's Phase 2 bug list, so it needs a decision, not a silent change during test-infra work. Retrofitted all six tests with
.require_treatment(...)asserting the current actual treatment (Fault for 3000-3002, Error for 3010-3012), with inline@@@comments cross-referencing this journal entry, so the tests are meaningful and green rather than silently passing via the old facility-onlyrequire_busy_present()(which can't distinguish Busy/Error/Fault at all — all three areToneFacility_Busy). Flagging for the user: worth a real Phase 2-style bug-fix step later, deciding whether to swap the treatments or fix TESTS.txt's prose.
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
- MS` directly where it was used — matches the exact same 30s value, so zero behavior change.
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):
- 4020: line 13 busy (just off-hook, no dial needed since
is_available()only requiresstate != Idle), dial 113, expect 125 to ring. - 4021: line 40 busy, dial 140 (self-CFB), expect ordinary
Treatment_Busy(not forwarded, not a fault/error cadence) -- proves the new guard fires. - 4022: lines 41/42 connected to each other (busy, no DTMF RX held -- avoids
the known 2-DTMF-RX ceiling since neither is sitting at dial tone), dial
141, expect
Treatment_Error(loop count exceeded) via the cadence-awarerequire_treatmentprimitive. - 4023: line 43 busy, dial 143, expect ring group 0 (120..125) to ring, mirroring 4003's CFWD-to-ring-group test.
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:
- 4105: dial 742 (RG2: 120, 121, 112[CFNA->124]), wait past the 30s CFNA timeout -- confirms a ring group member's own CFNA fires correctly from inside a ring-group secondary, and unrelated members keep ringing throughout.
- 4106: bust 13, dial 743 (RG3: 120, 121, 113[CFB->125]) -- confirms a member's own CFB fires from inside a secondary too; also doubles as a regression test for the reentrant-handle_event UAF fixed in the 402X step, one level deeper (nested inside the ring group's own secondary/primary dispatch).
- 4107: dial 746 (RG6, no members datafilled) -- confirms the
!nactiveempty group path applies Treatment_Busy cleanly. - 4108: dial 744 (RG4, one member: speedcall 211, an outside number) -- see below; this one was the real find.
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:
- 1003: a 30ms hook glitch (one 5ms sim tick under
DIGIT_ONHOOK_TIME, 35ms) is fully ignored -- confirmed dialtone/DTMF RX survive untouched, then a normal dial still works. - 1016: hang up mid-pulse-train, staying on hook past
HANGUP_TIME(1000ms) while inLineState_DialCompute(between pulse breaks) instead of completing the digit -- confirms clean teardown (ep_lines.cc:284-289 already handles this correctly; this is a behavior-confirming test, not a bug find). - 1026 and 1027: see below.
- 1037: a ring-answer bounce (offhook 30ms, back onhook) well inside
DEBOUNCE_TIME(50ms) is not registered as an answer -- the ringer keeps going; a real (held) offhook past the debounce answers correctly. - 1044/1045: exact boundary tests at
DIGIT_ONHOOK_TIME(35ms) andFLASH_HOOK_TIME(250ms), using one 5ms sim tick under/over each (the simulator's default tick,sim_optk, is 5ms, not 1ms -- confirmed by reading sim_main.cc rather than assuming the plan's "±1 tick" meant 1ms). 1044: 30ms glitch ignored, 40ms registers as a pulse (digit '1'). 1045: during an established call, a 245ms break is classified as a digit pulse (not flash) and silently ignored byP_LineState_WaitingForHangup(which only handlesCCBEvent_Flash) -- no crash, no spurious transfer, and critically doesn't corrupt state for the real 255ms flash that immediately follows, which correctly starts the park/transfer flow (mirrors test 4030).
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:
P_PPAPState_Idlecheckedccb->terminatorto detect "there's a party on the call from a flash" -- butp_line.cc's flash handling clearsterminatorand setsthirdparty, 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.- Even fixing (1), calling
park_terminator(ccb)alone doesn't surviveCCB::teardown()-- once the parker hangs up and every other processor goes inactive,CCB::teardown()unconditionally tears downoriginator/terminator/thirdpartyand frees the junctor; a raw file-staticEndpoint *cargives no way to stop that, so the "parked" party would get disconnected the instant the parker hung up. P_PPAPState_WaitForHangup(the state reached after successfully connecting to a parked call) never setsactive = falseon 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 towait_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):
- Crosspoint quad-collision ordering bug. Retrieve needs to move the retriever's line
from its own (fresh, now-redundant) junctor onto the parked party's junctor. First attempt
connected to the new junctor, then called
disconnect_junctor()on the old one -- looked obviously safe. Test 4040 failed with "STILL JUNCTORS CONNECTED" at its own.finish(). Root cause, found by reading the raw debug log'sconnect junctor/disconnect junctorsequence against the line-card register doc at the top ofep_lines.cc: a line's crosspoint connection is a singlehwshadowregister:disconnect_junctor(j)sets the "clear this line's column" bit for whichever 8044 quad-of-4jfalls into. When the old and new junctors land in the same quad (a real possibility --allocate_junctor's rotor cycles through all 12), the disconnect call, issued after the connect, clobbers the connection that was just made, because both writes target the same physical chip. Fixed by swapping the order: disconnect from the old junctor first, then connect to the new one -- correct regardless of which quad either junctor is in. - Missing
Line::ccbre-pointing during the custody transfer. Even after fixing the ordering bug, test 4040 still failed the exact same leak check, but with the audio wiring now correct (junctor map showed the parked line still connected after the whole test's.finish(), not merely mis-wired mid-test). Traced by grepping the debug log fordisconnect junctoraround the parked party's hangup and finding none at all: retrieve correctly re-pointedccb->terminatorat the parked party's Endpoint, but never updated the parked party's ownLine::ccbfield to point at the new ccb. TheirLine::ccbwas still pointing at the original ccb, which gets freed (*c = CCB{}) moments later once itsP_PPAPdeactivates -- so when the parked party eventually hung up,Line::emit()routed their onhook into ahandle_event()call on a ccb with an empty processor list: silently swallowed,Line::teardown()(and thereforedisconnect_junctor()) never called, junctor wired forever. Fixed with one line:ccb->terminator->ccb = ccb;right after the transplant. This is exactly the hazard the custody-pattern design note up top exists to avoid, and it's reassuring that the design (keeping the original ccb's own custody self-consistent) held up fine -- the bug was in the one place a party's controlling ccb genuinely changes identity, which needs the same reassignment Pickup (4B) will need for the picked-up line.
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:
- Every hangup-then-immediately-redial-the-same-line step needed an explicit
.wait (1 * MS)first, to clearHANGUP_TIME's 1-second onhook debounce -- without it, the DTMF dial landed before the line had actually re-originated, and hitsim_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.) - 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:
-
First attempt checked
m->from == ccb->thirdparty:make reg(not the isolated 407X batch) caught aSTILL ACTIVE CCBSleak in test 4040 (PPAP). Root cause:P_BusyOverridesits at the front of the processor list (push_front) and clearsccb->thirdpartyas 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,thirdpartyis 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. -
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 setsccb->originator = NULLwhen 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 repurposingccb->thirdparty/ccb->originatorfor 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_BusyOverrideitself, on the overrider's own onhook, returnsProcessorReturn_Consumedafter 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_BusyOverridedoes nothing special -- doesn't even touchccb->thirdparty-- and just returnsNextlike 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 nestedhandle_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:
-
intercept_busy(CFB) andintercept_terminated(CFNA) both explicitly decline (fwd == line->unit) before ever touching the digit stack -- self-forward just falls through to ordinary busy / ordinary ringing, per tests 4021 and 4013. -
intercept_termination(CFWD, unconditional) has no such guard. Dialing extension 170 (datafilledp_cfwd = 70, i.e. self-forward) re-translates to the same digits every time throughP_LineState_PostRouting'sCCBEvent_NewDigitshandler, which incrementsP_Line::loopcount(a per-ccb member, shared across every CFWD/CFNA/CFB-triggered re-route on that ccb, not reset between intercepts) until it exceedsMAX_LOOPCOUNT(10), then appliesTreatment_Error. Test 4001 already asserts exactly this outcome ("dial 170 and expect fast busy").Adding a self-guard to
intercept_terminationto match CFB/CFNA would be a genuine behavior change, not just a hygiene fix: declining early means termination proceeds normally, i.e. line 170 would actually ring itself, rather than fast-busy after 10 loop iterations. That's arguably the more consistent choice (matches CFB/CFNA's "treat self-forward as if unset" philosophy) but it would silently invalidate test 4001's own documented intent ("single-hop infinite call forward loop detection") -- a design decision beyond this audit's scope, so left as-is. Noted here for whoever revisits CFWD next.
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.