fix(tests): eight version assertions the v22 bump broke, five of them defects

CI red on #221: all five Test jobs, one identical test, every platform
— deterministic, not a flake. The production code was never wrong.

WHY MY GATES MISSED IT. The standing gate is "the touched acceptance
suites", selected from the diff. A PROTOCOL_VERSION bump breaks
version-assertion tests that appear nowhere in it. Worse, CI showed
only ONE of the eight, because cargo stops at the first failing
target; the rest surfaced only under `--tests --no-fail-fast`, and one
at a time would have cost four more red rounds.

Three of the eight were invisible even to that, because they are
crdt-gated real-daemon tests asserting on a live socket. Found by
`--tests --features crdt --no-fail-fast`. That is the handoff's
existing "a local sweep is blind to whichever configuration it does
not build" lesson, hit again by a different lane.

THREE TRIPWIRES, WORKING AS DESIGNED. `assert_eq!(PROTOCOL_VERSION,
21)` in statusline_segments, bottom_panel_stage2b_gpu, and
vterm_stage3 are meant to fire and take a deliberate edit; each says
so in its own comment. Updated to 22 with the reason recorded. Worth
noting the pin that must NEVER be edited —
ADVERTISED_PROTOCOL_VERSION == 20 — did not fire, which is the
mechanism behaving exactly as designed.

FIVE DEFECTS, ONE SHAPE: an absolute contract expressed as arithmetic
on, or equality with, a MOVING constant. Each was true when written
and silently false afterwards.

  - `PROTOCOL_VERSION - 1` meaning "below the panel version". Held
    only while PROTOCOL_VERSION == PANEL_MIN_VERSION; at v22 it
    equalled PANEL_MIN_VERSION exactly, so the fixture's "old" peer
    became panel-capable and the daemon correctly sent it a frame.
    Now `PANEL_MIN_VERSION - 1`.
  - `assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION)` — a coincidence
    true only while panels were the newest feature. Replaced by the
    two durable bounds: above the advertised floor, at or below this
    binary's wire.
  - `assert_eq!(PROTOCOL_VERSION, 21)` in a test named
    `the_panel_stage_takes_protocol_v21` — the current wire as a proxy
    for the panel stage's own version, in a test whose name says which
    one it means. Now PANEL_MIN_VERSION.
  - `session_protocol_version == "21"` in two real-daemon probes. What
    the counter-offer activates is THIS BINARY's wire, so the literal
    was only ever right by accident. Now PROTOCOL_VERSION, plus an
    explicit `>= PANEL_MIN_VERSION` for the panel capability the
    literal had been carrying implicitly.

The codebase already had the right idiom: src/daemon.rs and
pmacs-gpu/src/main.rs spell it `PANEL_MIN_VERSION - 1` in five places.
Every outlier was in tests/.

ALSO LOGGED, NOT FIXED: U2 in ci-red-signatures.md.
`process::tests::m6_1_pty_raw_mode_disables_kernel_echo` failed once
during a full corpus run and did not reproduce (108 targets exit 0,
plus 3 isolated --lib runs at 1917/0). It is in no registry row, so it
is a new incident, and leaked `pmacs --daemon` processes remain an
unexcluded rival explanation. Recorded with a selector this time —
unlike U1, whose name I destroyed by piping through `tail`.

Gates: fmt; clippy --workspace --all-targets -D warnings, both
configurations; --lib 1917/0; --lib --features crdt 2102/0; --tests
--no-fail-fast 108 targets exit 0; --tests --features crdt
--no-fail-fast 108 targets exit 0; PMACS_REQUIRE_GPU=1 -p pmacs-gpu
228/0; git diff --check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
Levi Neuwirth 2026-08-07 20:15:33 +02:00
parent 8c64a4597b
commit 4d70ff6931
No known key found for this signature in database
8 changed files with 191 additions and 21 deletions

View File

@ -607,22 +607,66 @@ Edit tool, which errors on mismatch, for anything load-bearing.
### Verification (delivered)
Everything framing §7 sketched, plus three groups it did not anticipate
(framing §7.1). The exact gate run, all green on this machine:
(framing §7.1).
**The gate list below is the one that FAILED to catch this lane's CI
red, and it is kept only to show the hole.** "The touched acceptance
suites" is selected from the diff, and a `PROTOCOL_VERSION` bump breaks
version-assertion tests that appear nowhere in it. Five failed on CI's
first round. Use **`cargo test --tests --no-fail-fast`** on any
protocol bump — `--no-fail-fast` because cargo stops at the first
failing target, which is why CI showed one failure and the local
full-corpus run then found three more. Recorded in
`docs/agent-handoff.md`.
```
cargo fmt --check
cargo clippy --workspace --all-targets -- -D warnings
cargo clippy --workspace --all-targets --features crdt -- -D warnings
cargo test --lib # 1917 / 0
cargo test --lib --features crdt # 2102 / 0
cargo test --test line_wrap_acceptance # 6 / 0
cargo test --test long_line_readable_acceptance # 2 / 0
cargo test --test folding_acceptance # 21 / 0
cargo test --test full_grid_resync_acceptance # 1 / 0
cargo test --test m4_acceptance -- --skip basedpyright
cargo test --tests --no-fail-fast # 108 targets, exit 0
cargo test --tests --features crdt --no-fail-fast # 108 targets, exit 0
PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu # 228 / 0
git diff --check
```
**Both configurations, and that mattered.** The default sweep went
clean while `--features crdt` still had **three** failures, in
crdt-gated real-daemon tests (`vterm_stage3_acceptance` ×2,
`bottom_panel_stage2b_gpu_acceptance` ×1) that assert on a real
socket's negotiated version. This is the handoff's existing "a local
sweep is blind to whichever feature configuration it does not build"
lesson, hit again by a different lane — so **eight** version
assertions broke in total, not five.
The eight version-assertion failures, and what each one was:
| test | was | why it broke |
|---|---|---|
| `acc51_a_v20_peer_..._even_when_capable` (×5 jobs) | `PROTOCOL_VERSION - 1` | **bug** — an absolute contract ("below the panel version") as arithmetic on a moving constant. Now `PANEL_MIN_VERSION - 1`, the idiom `src/` already used in five places |
| `the_baseline_stays_and_the_counter_offer_activates` | `PANEL_MIN_VERSION == PROTOCOL_VERSION` | **bug** — asserted a coincidence true only while panels were the newest feature. Now the two durable bounds |
| `the_panel_stage_takes_protocol_v21` | `PROTOCOL_VERSION == 21` | **bug** — the current wire as a proxy for the panel stage's own version, in a test whose name says which it means |
| `a54_real_daemon_..._panel_hosted_terminal` *(crdt)* | `session_protocol_version == "21"` | **bug** — the negotiated session version is *this binary's* wire, so the literal held only while panels were newest. Now `PROTOCOL_VERSION`, plus an explicit `>= PANEL_MIN_VERSION` for the capability the literal carried implicitly |
| `a37_real_daemon_..._one_terminal_session` *(crdt)* | `session_protocol_version == "21"` | **bug** — same |
| `a13_17_26_..._version_cost` | `PROTOCOL_VERSION == 21`, `6..=21`, `!supported(22)` | **tripwire working as designed** — it says in its own comment that it tracks the current wire. Took the conscious edit |
| `the_baseline_stays_...` (same test) | `PROTOCOL_VERSION == 21` | **tripwire working as designed** |
| `terminal_mode_keeps_reporting_presence_...` *(crdt)* | `PROTOCOL_VERSION == 21` | **tripwire working as designed** |
**Five bugs and three tripwires.** The distinction is the useful part.
A tripwire that fires on a bump is doing its job, and "fixing" it means
editing it deliberately — the baseline pin `ADVERTISED_PROTOCOL_VERSION
== 20` is the one that must *never* be edited, and it never fired.
The five bugs share one shape: **an absolute contract expressed as
arithmetic on, or equality with, a moving constant.** `PROTOCOL_VERSION
- 1` for "below the panel version"; `PANEL_MIN_VERSION ==
PROTOCOL_VERSION` for a coincidence; `PROTOCOL_VERSION == 21` standing
in for the panel stage's own version; `"21"` for a negotiated session
version. Each was true when written and silently false afterwards.
`src/` already had the right idiom — `PANEL_MIN_VERSION - 1`, five
occurrences — and every outlier was in `tests/`.
`tests/long_line_readable_acceptance.rs` is the one that answers the
**report** rather than a mechanism: the shipped binary, a real PTY, a
line 200 columns wide in an 80-column terminal, and an assertion that
@ -650,6 +694,13 @@ establish that. Logged there as **U1**, explicitly *not* matched
against A1 (also GPU-headless-under-load) because matching requires a
selector and fragments this occurrence does not have.
**And one more, logged as U2.**
`process::tests::m6_1_pty_raw_mode_disables_kernel_echo` failed once
during a full `--tests --no-fail-fast` run and did not reproduce in a
later full sweep (108 targets, exit 0) or 3 isolated `--lib` runs. It
is in no registry row, so it is a new incident; leaked
`pmacs --daemon` processes remain an unexcluded rival explanation.
### Not in scope
Horizontal scroll in full (Stage 4). `M-q` / auto-fill / reflow. Word

View File

@ -404,6 +404,35 @@ someone forgot.
old. They are a rival explanation for any load-sensitive local
failure, so **check `pgrep -f "pmacs --daemon"` before trusting a
local red**. Lane recorded in `docs/active-work.md`.
- **A `PROTOCOL_VERSION` bump's blast radius is every version-sensitive
test, and NONE of them appear in the diff.** "The touched acceptance
suites" is the standing gate, and for a protocol bump it is the wrong
selector: long-lines Stage 3 bumped v21→v22, ran the suites it had
edited, and broke **eight** version assertions across six suites. CI
showed exactly **one**, because **cargo stops at the first failing
target**; the rest surfaced only afterwards, and one at a time would
have cost four more red rounds.
**On any protocol bump run `cargo test --tests --no-fail-fast` in
BOTH feature configurations.** `--no-fail-fast` because of the
stop-at-first-target behavior above, and `--features crdt` because
three of the eight were in crdt-gated real-daemon tests that assert
on a live socket's negotiated version — invisible to a default sweep,
which is the blindness the bullet below already names.
**Sort the failures before fixing them.** A *tripwire*
(`assert_eq!(PROTOCOL_VERSION, N)`) is meant to fire and takes a
deliberate edit; three of the eight were these, and the one pin that
must NEVER be edited — `ADVERTISED_PROTOCOL_VERSION == 20` — did not
fire. The other five were defects sharing one shape: **an absolute
contract expressed as arithmetic on, or equality with, a moving
constant.** `PROTOCOL_VERSION - 1` for "below the panel version";
`PANEL_MIN_VERSION == PROTOCOL_VERSION` for a coincidence true only
while panels were newest; `PROTOCOL_VERSION == 21` standing in for
the panel stage's own version; `"21"` for a negotiated session
version. Anchor on the constant the contract *names*`src/` already
spelled it `PANEL_MIN_VERSION - 1` in five places, and every outlier
was in `tests/`.
- **A local sweep is blind to whichever feature configuration it does
not build.** Stage 3's census and every verification sweep ran
`--features luajit` WITHOUT `crdt`, so no crdt-gated suite was

View File

@ -423,6 +423,21 @@ without a name there is nothing to call intermittent.
with `grep -E "FAILED|panicked|test result"`, which keeps failure
context, or capture the full log to a file and summarize from it.
### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, one local occurrence
Has a selector, which U1 lacks — but still no fragments, so it cannot
be matched either. Recorded so a recurrence is recognisable.
| field | value |
|---|---|
| **selector** | `--lib process::tests::m6_1_pty_raw_mode_disables_kernel_echo` |
| **job / flavor** | local (Linux), during `cargo test --tests --no-fail-fast` — the lib target alongside a full PTY-heavy corpus |
| **required fragments** | **none captured** — output was filtered to the `FAILED` line |
| **status** | **new incident, unreproduced** |
| **what IS established** | it failed once (`1916 passed; 1 failed`), in no registry row, under a full-corpus run |
| **what is NOT** | any mechanism. Not reproduced in a later full `--tests --no-fail-fast` sweep (108 targets, exit 0) nor in 3 isolated `--lib` runs (1917/0 each) |
| **rival explanation not excluded** | leaked `pmacs --daemon` processes, which the handoff names as a standing confound for any load-sensitive local red |
**The retirements are not occurrences and do not close the log.** R1 and
R3 stay live, and each retired row keeps its signature so a later red
matching one reopens it.

View File

@ -32,7 +32,7 @@ use pmacs::editor_core::GeometryUpdate;
use pmacs::protocol::{FrontendId, InstanceMessage, PROTOCOL_VERSION};
use pmacs::semantic_render::SemanticRenderState;
use pmacs::window::{FrontendView, Layout, Window, WindowId};
use pmacs_protocol::panel::{PanelFrame, PanelFramePayload};
use pmacs_protocol::panel::{PANEL_MIN_VERSION, PanelFrame, PanelFramePayload};
// ---------------------------------------------------------------------------
// Harness
@ -740,10 +740,25 @@ fn acc51_a_pre_panel_semantic_frontend_is_never_sent_a_panel_frame() {
);
}
/// A peer below [`PANEL_MIN_VERSION`] gets no `PanelFrame` even when the
/// daemon could build one for it.
///
/// **Anchored on `PANEL_MIN_VERSION - 1`, not `PROTOCOL_VERSION - 1`.**
/// The original spelling was the latter, which expressed an *absolute*
/// contract — "older than the version that introduced panel frames" —
/// as arithmetic on a *moving* constant. It held only while
/// `PROTOCOL_VERSION` happened to equal `PANEL_MIN_VERSION`, and the
/// long-lines lane's bump to v22 made `PROTOCOL_VERSION - 1` equal
/// `PANEL_MIN_VERSION` exactly: the fixture's "old" peer became
/// panel-capable, so the daemon correctly sent a frame and the test
/// correctly failed. The production code was never wrong.
///
/// `src/daemon.rs` and `pmacs-gpu/src/main.rs` already spell this
/// `PANEL_MIN_VERSION - 1` in five places; this was the one outlier.
#[test]
fn acc51_a_v20_peer_is_sent_no_panel_frame_even_when_capable() {
fn acc51_a_sub_panel_version_peer_is_sent_no_panel_frame_even_when_capable() {
let mut session = Session::new();
session.render = SemanticRenderState::for_peer(FID, PROTOCOL_VERSION - 1);
session.render = SemanticRenderState::for_peer(FID, PANEL_MIN_VERSION - 1);
session.declare(1, ROWS, COLS);
open_panel(&session, "*panel*", 4);

View File

@ -336,13 +336,31 @@ fn one_daemon_serves_a_v21_panel_session_and_a_shipped_v20_client() {
/// and the counter-offer is what reaches the current wire.
#[test]
fn the_baseline_stays_and_the_counter_offer_activates() {
assert_eq!(PROTOCOL_VERSION, 21);
// A deliberate tripwire: bumping the wire must be a conscious edit
// here, not a silent one. v22 is `LineWrapFacts` (long-lines Stage 3).
assert_eq!(PROTOCOL_VERSION, 22);
assert_eq!(
ADVERTISED_PROTOCOL_VERSION, 20,
"moving this is the incompatible act the mechanism exists to avoid"
);
const { assert!(PROTOCOL_VERSION > ADVERTISED_PROTOCOL_VERSION) };
assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION);
// Panel frames are gated ABOVE the advertised floor and at or below
// this binary's wire. Both halves are durable properties of the
// gate.
//
// This replaces `assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION)`,
// which asserted a **coincidence**: panel frames were the newest
// feature when it was written, so their minimum happened to equal
// the current wire. Any later feature falsifies that — v22 is the
// first, and the equality would have had to be edited on every
// subsequent bump while telling a reader something that was never
// the contract.
// `const` blocks, matching the line above: these are compile-time
// constants, so a runtime `assert!` is both a clippy error and a
// weaker check than the language already offers.
const { assert!(PANEL_MIN_VERSION > ADVERTISED_PROTOCOL_VERSION) };
const { assert!(PANEL_MIN_VERSION <= PROTOCOL_VERSION) };
// The current baseline is answered with this binary's own version.
assert_eq!(
@ -584,8 +602,24 @@ fn a54_real_daemon_real_pty_and_headless_gpu_render_one_panel_hosted_terminal()
"a deadline-driven pass must not read as success: {text}"
);
// The activation, end to end on a real socket.
assert_eq!(fact("session_protocol_version"), "21", "{text}");
//
// The session version is compared against `PROTOCOL_VERSION` rather
// than the literal "21" it used to pin: what the counter-offer
// activates is *this binary's* wire, so the literal was only ever
// correct while the panel stage was the newest one. The baseline
// stays a literal, because 20 not moving IS the claim.
assert_eq!(
fact("session_protocol_version"),
PROTOCOL_VERSION.to_string(),
"{text}"
);
assert_eq!(fact("baseline_protocol_version"), "20", "{text}");
// …and that negotiated version is panel-capable, which is the part
// "21" used to carry implicitly.
assert!(
number("session_protocol_version") >= PANEL_MIN_VERSION,
"the negotiated wire must reach the panel minimum: {text}"
);
// The band is real: declared, projected, focused, and carrying the child.
assert!(

View File

@ -14,7 +14,7 @@ use pmacs_protocol::message::{
AttachRequest, FrontendEvent, Hello, InstanceMessage, Modifiers, MouseButton, MouseKind,
};
use pmacs_protocol::panel::{
MAX_PANEL_VISIBLE_CELLS, PanelFrame, PanelFrameError, PanelFramePayload,
MAX_PANEL_VISIBLE_CELLS, PANEL_MIN_VERSION, PanelFrame, PanelFrameError, PanelFramePayload,
};
use pmacs_protocol::terminal::{
MAX_TERMINAL_COLS, TerminalFrame, TerminalFrameError, TerminalProcessState,
@ -107,8 +107,18 @@ fn terminal_frame(rows: u32, cols: u32) -> TerminalFrame {
#[test]
fn the_panel_stage_takes_protocol_v21() {
assert_eq!(PROTOCOL_VERSION, 21);
// The panel stage's own version, which does not move when a later
// feature appends to the wire.
//
// This was `assert_eq!(PROTOCOL_VERSION, 21)` — the CURRENT wire
// used as a proxy for the panel stage's version. The two were equal
// only until the next feature landed (v22, `LineWrapFacts`), and the
// proxy then failed in a test whose own name says what it means to
// pin. `PANEL_MIN_VERSION` is that constant.
assert_eq!(PANEL_MIN_VERSION, 21);
assert!(SUPPORTED_PROTOCOL_VERSIONS.contains(&21));
// This binary must be able to speak the stage it implements.
const { assert!(PROTOCOL_VERSION >= PANEL_MIN_VERSION) };
// The advertised version is a compatibility BASELINE, and Stage 2B-3
// made that permanent rather than temporary: the server-first Hello
// reaches an already-shipped frontend before that frontend can send

View File

@ -788,15 +788,21 @@ fn a12_builtin_lsp_provider_tracks_real_attachment_and_unknown_label() {
fn a13_17_26_protocol_semantic_init_late_join_and_version_cost() {
// Vterm Stage 3 appended the terminal family as v19; GPU initial targets
// appended the semantic bootstrap family as v20; bottom-panel Stage 2B-1
// appended the panel family as v21. This acceptance owns the STATUSLINE
// appended the panel family as v21; long-lines Stage 3 appended
// `LineWrapFacts` as v22. This acceptance owns the STATUSLINE
// variant's placement and gate, so it tracks the current wire version
// rather than pinning 18: the v18 floor it actually cares about is asserted
// below and in `peer_accepts_statusline_message`.
assert_eq!(PROTOCOL_VERSION, 21);
for version in 6..=21 {
//
// Tracking the current wire is deliberate, so every bump edits these
// three lines on purpose. The ceiling assertion is the load-bearing
// one — it says the supported set ENDS here, which is what makes an
// accidentally-widened set a failure rather than a silent pass.
assert_eq!(PROTOCOL_VERSION, 22);
for version in 6..=22 {
assert!(is_supported_protocol_version(version));
}
assert!(!is_supported_protocol_version(22));
assert!(!is_supported_protocol_version(23));
let sample = InstanceMessage::StatuslineSegments {
buffer_id: BufferId::from_raw(9),
left: vec![StatuslineSegment {

View File

@ -737,10 +737,18 @@ fn a37_real_daemon_real_pty_and_headless_gpu_render_one_terminal_session() {
// which is exactly the incompatible change this mechanism exists to
// avoid — and asserting only the baseline would pass with the whole
// activation missing.
// Compared against `PROTOCOL_VERSION`, not the literal "21": the
// counter-offer activates THIS BINARY's wire, so the literal held
// only while the panel stage was the newest one (long-lines Stage 3
// appended v22). The baseline assertion below stays literal,
// because 20 not moving is the actual claim.
// Fully qualified: this file imports `PROTOCOL_VERSION` inside a
// different test's scope, not at module level.
let session_version = pmacs_protocol::PROTOCOL_VERSION.to_string();
assert_eq!(
facts.get("session_protocol_version").copied(),
Some("21"),
"the real client must negotiate the v21 panel wire: {text}"
Some(session_version.as_str()),
"the real client must negotiate this binary's wire: {text}"
);
assert_eq!(
facts.get("baseline_protocol_version").copied(),
@ -880,7 +888,9 @@ fn terminal_mode_keeps_reporting_presence_so_peers_drop_the_stale_caret() {
panic!("timed out waiting for {what}");
}
assert_eq!(PROTOCOL_VERSION, 21);
// Tripwire: a wire bump must be a conscious edit here. v22 is
// `LineWrapFacts` (long-lines Stage 3).
assert_eq!(PROTOCOL_VERSION, 22);
let daemon = common::daemon::TestDaemon::spawn_with_env_and_init(
&[
("PMACS_INSTANCE_SEMANTIC_RENDER", "1"),