fix(backlog): item #320 used a status glyph the invariant does not allow - #122
Closed
wshallwshall wants to merge 1 commit into
Closed
fix(backlog): item #320 used a status glyph the invariant does not allow#122wshallwshall wants to merge 1 commit into
wshallwshall wants to merge 1 commit into
Conversation
BACKLOG #320 was filed with `> 📋 **Filed 2026-08-01, not started.**`. The status gate accepts five glyphs — ✅ SHIPPED, ⛔ DECLINED, 🪦 RETIRED (closed), 🔢 prioritized, 🚧 in progress (open) — and 📋 is not among them, so the item read as declaring no status at all. That turned main red at 13:26 (run 30701716567, both the ubuntu and windows-2022 legs) and every PR opened since inherits it through the merge ref. The item is open and not started, so 🚧 is the correct glyph; the wording is unchanged and it still carries exactly one banner.
Collaborator
Author
|
Superseded by #123, which fixed the same defect with a fuller banner Verified before closing: the status invariant reports zero errors against Two sessions independently fixed this within the hour — worth noting that a drive-by fix to another |
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
… (BACKLOG #122) *** STILL A RESCUE COMMIT. STILL UNVERIFIED. DO NOT MERGE. *** The lane kept working after the first rescue commit and was cut off again by the session limit. Eight more files were uncommitted, including further work on logging_guard.py and test_log_write_guard.py plus two PHI-inventory tests (test_phi_at_rest_inventory.py, test_phi_logging_inventory.py) and the ADR index. --no-verify again, deliberately and recorded rather than hidden: the alternative was leaving eight modified files with no SHA a second time. The .mf-measure/ scratch directory is deliberately NOT staged. WHAT A RESUMER MUST DO, because none of it was done: - re-run the canonical quartet from scratch; there is no verified state here - establish a fresh baseline in THIS worktree before the first edit and diff node-id by node-id; there is no stable baseline on this box - confirm ADR 0162's number came from scripts/coord/alloc.ps1 and was NOT chosen by grepping -- a grepped number merges clean and corrupts the ledger - the two-stage design is the whole safety story and must not be collapsed: roll to a fresh file and RECOVER first; stop the connection ONLY if the replacement also cannot be written
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
…internal stages too (BACKLOG #122) The two rescue commits below this one landed the guard unverified and with the enforcement half-built. This finishes and verifies it. THE DEFECT, measured rather than reasoned about. The stage-2 halt stopped the inbound LISTENER and paused the outbound lanes, and that is not "refuse to process". The router and transform workers are registry-tied, not source-tied (_ensure_inbound_workers says so), and stop_inbound is documented as halting intake WHILE DELIVERY KEEPS DRAINING -- so a message already durably committed to the ingress stage kept flowing ingress -> routed -> outbound with no application log behind it. An end-to-end test over a genuinely unwritable sink caught the committed row sitting on the OUTBOUND stage after the halt, held back only by the outbound pause. Routing and transforming a message you cannot log is exactly what the owner's ruling forbids; the pause only made it quiet. THE FIX. _halt_inbound_processing shuts the internal stages (router / transform / loopback response) for every registry inbound, cooperatively and never by task.cancel -- a cancelled mid-item worker strands its claimed row INFLIGHT and reset_stale_inflight is startup/DR-only. Pooled (the default) pauses each stage dispatcher's lane, the same primitive _stop_outbound_unsafe uses; per_lane returns out of the worker at a loop-top gate. A lane mid-episode finishes at most its one in-flight head. _resume_inbound_processing re-arms per connection from _start_inbound_unsafe, BEFORE the worker respawn -- otherwise the respawned worker hits its own gate and the restart reports success having re-armed nothing. Also: the escalation seam holds one responder, and a second RegistryRunner taking the slot silently unguarded the first. Kept single-slot (one process runs one engine) but made audible on the last-resort channel. VERIFICATION, none of which existed before. - The test that decides the item runs the whole chain: a real unwritable file (closed handle + the log's parent directory replaced by a regular file, so the roll cannot succeed either), the real handler configure_logging installs, the real guard, a real running RegistryRunner. It asserts the committed ingress row is still RECEIVED with no outbound rows. Paired with a negative control on the identical rig proving the row IS processed when the log is healthy, and a recovery test proving a restart drains it. Both claim modes. - Each halting mechanism was disabled on purpose and the matching arm confirmed to fail: pause_lane removed -> [pooled] fails, [per_lane] passes; the loop-top gates removed -> [per_lane] fails, [pooled] passes. - test_the_stdlib_handler_..._does_write_record_content_to_stderr COULD NEVER PASS as committed: tests/conftest.py sets logging.raiseExceptions = False for the whole session, under which the stdlib handleError is a no-op. It now pins the value it is asserting under, and the guard's own ambient test is parametrized over both values so the default True is exercised at all. - The two other observability channels the ADR leans on are pinned: the log_write_failed alert is operator-rule-targetable, and GET /status's log_sinks reports the break from process memory carrying no record content. Docs reconciled to the built behaviour: ADR 0162 section 4 and its index row now say all three tiers and record the measurement; SERVICE.md's recovery line said "reload or restart" and a reload never rebuilds the dispatchers, so it now says restart the affected connections; CONFIGURATION.md likewise. Two alternatives are recorded as rejected rather than left as apparent gaps: detecting a corrupted-but- writable log (the item's scope line says "corrupted or unwritable"; its trigger is a log that stops recording, and reading the log back to check is a PHI read of the one artifact section 8 keeps record content out of), and cancelling the workers. ADR 0162's number is accounted for: claim file 0162.json in the shared registry, taken 2026-08-10T19:01:11 by branch w3-log-write-failure with the title matching the filename slug -- allocated by scripts/coord/alloc.ps1, not grepped. Its index row is on the branch (added by the second rescue commit, one commit after the ADR file; both are in the base..HEAD change set CI evaluates). Verified with hooks enabled from this worktree's venv.
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
…d correct three docs (BACKLOG #122) The previous commit's docs said a /config/reload "deliberately does not resume a lane", reasoning that a reload never rebuilds the stage dispatchers. That is true of the dispatchers and false of the outcome: reload() quiesces every source and then calls _start_inbound_unsafe for each inbound the new graph re-binds, and the re-arm rides that start -- so a reload DOES re-arm the inbounds it re-binds. Measured, in both claim modes, rather than re-read: after a halt a reload moves the committed row to the OUTBOUND stage and it stops there, because the outbound pause is operator-owned and a reload must never resume it (#115/#233). So "a reload fixes it" and "a reload fixes nothing" are each half right, and shipping either sentence alone sends an operator the wrong way during an incident. All three operator-facing statements (SERVICE.md, CONFIGURATION.md, ADR 0162 section 4 + its index row) now say both halves: a reload re-arms routing, delivery still needs start_outbound or a service restart. No production behaviour change -- this commit is the test that pins the reload path plus the prose it falsified.
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
…h (BACKLOG #122) Found by running the suite, which the rescue commits never did. Both are the branch's OWN gates catching the branch's OWN new artifacts, and both would have reached CI red: test_every_alert_event_type_is_named_in_the_inventory -- `log_write_failed` was added to settings._ALERT_EVENT_TYPES but named nowhere in PHI.md section 7. The gate derives its list from the settings registry precisely so a new event type cannot ship undocumented. Row 8 now names it with its payload shape: the sink LABEL, the stage, a safe_exc reason and a count of connections stopped, never the record whose write failed. test_log_sinks_are_exactly_the_documented_set -- messagefoundry/logging_guard.py matches the sink-token scan (it subclasses RotatingFileHandler) and was in no allow-list. This is ASVS 16.2.3 working: tray.log shipped undocumented under the frozen token list this gate replaced. Listed rather than excluded, because the class that opens and rolls the file genuinely lives there, so a future sink added beside it still trips the gate. It is not a new DESTINATION -- logging_setup constructs and filters these handlers and stream 1 already names both. The allow-list entry is one exact key against a set-equality assertion, so the gate is unchanged for every other module. Its ability to see this class was demonstrated rather than assumed: it is what reported both failures.
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
…umped (BACKLOG #122) The third gate this branch left red, and the same shape as the two PHI-inventory ones: the rescue commits added the additive `log_sinks` field to SystemStatus -- a DTO the web console renders -- without bumping the seam on either side or refreshing the golden. test_webconsole_seam_snapshot_matches_golden caught it, deterministically, and printed the exact remediation. Bumped ENGINE_UI_SEAM 18 -> 19 and messagefoundry_webconsole.SUPPORTED_ENGINE_SEAMS to match (the console accepts exactly the engine's seam, BACKLOG #279), and regenerated the golden -- a two-line diff, the version and the SystemStatus field list, nothing else moved. The bump is required even though the field is purely additive with a default: the golden introspects SystemStatus's field set, so the handshake trips on any added field. That is deliberate -- this gate is the sole backstop against a future engine's unbumped render-breaking DTO rename, so it must stay comprehensive. Verified: the 3 seam tests pass, and packaging/messagefoundry-webconsole/tests runs 350 passed / 9 skipped with the new seam pinned on both sides.
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
… (BACKLOG #122) Found by running the full suite, twice, and it is the most serious defect on this branch: a routine stdout stream swap took a running engine's connections down. MECHANISM. A logging handler holds the stream OBJECT it was constructed with. GuardedStreamHandler's stage-1 "roll" was a bare re-attempt on that same object, so once the object was closed or replaced -- a supervisor swapping the capture file, a closed pipe, or pytest tearing its capture down -- every write raised, INCLUDING stage 1's own notice write. Stage 1 therefore failed BY CONSTRUCTION and every stdout write failure escalated to stage 2. Measured in the full suite: LOG SINK stdout IS UNWRITABLE ... ValueError: I/O operation on closed file ALERT log_write_failed: sink 'stdout' unwritable; 7 connection(s) stopped The load engine's seven connections were halted and the run sent ZERO messages (tests/test_load_runner.py, `assert report.counters.sent > 0`). It reproduced in both full-suite runs and in neither of 12 isolated runs, which is what made it look like a flake and is exactly why it had to be diagnosed rather than re-run. TWO FIXES, each principled rather than symptomatic. 1. The stdout roll RE-RESOLVES sys.stdout instead of re-attempting a dead handle. That is the honest roll for a stream the engine did not open, and it is what "a re-attempt clears the transient" always claimed to do -- the replacement handle is the live one. A genuinely dead sys.stdout still raises and still reaches stage 2; a paired test pins both directions, and the pre-existing stdout test now PINS sys.stdout, because without the pin it passed for the wrong reason (nothing could ever heal, so "stage 2 fires" was true of every failure). 2. The stop is asked for only when EVERY guarded sink is unwritable. The ruling asks "can this process still log?", not "did a sink break?" -- with [logging].file configured, one sink dying while the other accepts every record means the processing IS logged, and halting there is a control resting on a false premise. One sink (the default) makes the two questions identical, so the halt is unchanged there. Detection, the alert and /status stay unconditional; only the ENFORCEMENT is conditioned on the thing it is about. The six end-to-end tests now kill BOTH sinks, which makes them a more faithful statement of the condition rather than a weaker one: they assert the engine refuses to process when it cannot log ANYWHERE. The stdout sink is re-pointed at a closed stream rather than having pytest's capture object closed underneath it -- killing the sink under test must not kill the harness that reports the result.
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
backlog: amend #1212 (built, refuted, reverted) and #122 (visibility is not enforcement)
wshallwshall
added a commit
that referenced
this pull request
Aug 11, 2026
…rm on the log (BACKLOG #122) The halt itself was right. Lifting it was not: recovery is an operator assertion ("I fixed the disk"), and the engine simply believed it. MEASURED, both claim modes, on the shipped rig: after a correct stage-2 halt, restart_inbound + start_outbound re-armed the whole pipeline while the guard's own state still read {'file': 'unwritable', 'stdout': 'unwritable'}, and the message ran to PROCESSED with no application log behind it. That is the owner's ruling inverted -- "we never want to process stuff if the processing cannot be logged" -- reached by the recovery door rather than the halt door. Worse, it was unrecoverable by construction: _log_write_stopped and the guard's per-sink already_down latch are both one-shot, so after that first restart nothing in the process could ever fail-closed again. The existing recovery tests passed because they never repaired the log before restarting, so they asserted the hole as if it were the feature. Fix, in two halves: * LogWriteGuard grows record_healthy / can_log / revalidate. revalidate re-tests each dead sink BY WRITING a real record to it, because a cached read cannot answer the question: unwritable is only ever set by a failed write and nothing clears it, so "repaired" and "still broken" are identical in memory. The probe rolls first when the handle is stale (a repaired directory does not un-close a file object), so "writable" means a record landed. Not the timer polling ADR 0162 rejected -- once, on an explicit operator action, never on the hot path. * RegistryRunner routes restart_inbound, start_outbound and a reload's re-bind through _log_recovery_ok. A process that still cannot log stays halted, and the inbound's listener is stopped again rather than left ACKing into a lane nothing drains. Deliberately an alert and not a raise: reload() rolls the whole graph back on an exception from that path, and one unwritable log must not turn a routine reload into a full intake rollback. A successful re-validation clears both latches so a LATER break halts afresh. Tests: the refusal is pinned end to end in both claim modes -- restart with the sinks still dead, assert the inbound stays halted, its listener stays down and the row stays RECEIVED, then repair the log, reissue the SAME two calls and watch it drain (so it cannot pass against an engine that merely never restarts anything). The gate was disabled and that arm confirmed to fail. The four pre-existing recovery tests now repair the log first, via a _revive_every_sink inverse of _kill_every_sink. Plus a guard-level unit covering both revalidate directions and the cleared latch. Docs corrected where they promised an unconditional re-arm: ADR 0162 (section 4 recovery, section 7, the consequences list), its index row, SERVICE.md and CONFIGURATION.md. Measured: full suite 11545 passed / 855 skipped / 0 failed, against my own baseline at the merge-base 751ca08 of 11499 / 855 / 0 -- +46 node ids, all in tests/test_log_write_guard.py, no node removed. ruff format + ruff check clean; mypy byte-identical to baseline (21 pre-existing errors, all missing optional extras, none in the touched files). Web console package collected, 356 nodes both sides.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
main is red and this one line fixes it. One-character change, no other file touched.
BACKLOG #320 was filed at 13:26 today with:
scripts/docs/backlog_status_check.pyaccepts five glyphs — ✅ SHIPPED, ⛔ DECLINED, 🪦 RETIRED(closed) and 🔢 prioritized, 🚧 in progress (open). 📋 is not among them, so the item parses as
declaring no status and the invariant fails.
Impact
tests/test_backlog_status_check.py::test_the_real_backlog_satisfies_the_invariantruns the checkeragainst the real
docs/BACKLOG.md, so this fails in the ordinary pytest job:The fix
📋 → 🚧. The item is open and not started, which is exactly what 🚧 means. Wording unchanged, and it
still carries exactly one banner — the other half of the invariant is that a CLOSED and an OPEN banner
must never coexist, so I checked the count rather than just the glyph.
Verification
pytest tests/test_backlog_status_check.py→ 15 passed. The ledger gate passes against this tree (nonew ADR/BACKLOG number is introduced — #320 already exists on main).
Split out of #121 at the owner's request so that PR stays single-topic; #121 will be red on this one
test until this merges.
🤖 Generated with Claude Code