#604 item 1: fleet_list reports charterBytes alongside charterSha256 #605

2026-09-20T11:15:17+02:00

agent commented

2026-09-20 11:15:17 +02:00

Closes item 1 of #604 (items 2 and 3 are documentation notes, left untouched).

The gap: CharterReceipt carries both charterSha256 and charterBytes, but SessionManager.rosterView only copied the digest into the roster map, so fleet_list never reported the size.

Change: charterBytes is now written nested inside the existing if (charterSha256 != null) block, so it always travels with the digest. Rationale: CharterReceipt.compose only ever produces a non-null digest together with a real byte count (or both null/0 when no charter was composed at all) — the size is a companion to the digest, not an independent field. A member with no composed charter still reports charterSource only, exactly as before.

Tests (SessionManagerTest.java):

Updated rosterViewExposesTheCharterReceiptButNeverTheCharterText to assert charterBytes against the receipt's own value (not a hardcoded literal), plus the real UTF-8 length of the composed string.
Added rosterViewOmitsCharterBytesAndDigestWhenNoCharterWasComposed — no role charter and no reply charter composed reports charterSource="none" and neither charterSha256 nor charterBytes.
Added rosterViewOmitsCharterFieldsEntirelyWhenTheReceiptItselfIsAbsent — session.charterReceipt() == null still suppresses all three keys.

No existing test needed to be weakened.

Build: mvn -o clean install from the worktree — Tests run: 1821, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Closes item 1 of #604 (items 2 and 3 are documentation notes, left untouched). **The gap:** `CharterReceipt` carries both `charterSha256` and `charterBytes`, but `SessionManager.rosterView` only copied the digest into the roster map, so `fleet_list` never reported the size. **Change:** `charterBytes` is now written nested inside the existing `if (charterSha256 != null)` block, so it always travels with the digest. Rationale: `CharterReceipt.compose` only ever produces a non-null digest together with a real byte count (or both null/0 when no charter was composed at all) — the size is a companion to the digest, not an independent field. A member with no composed charter still reports `charterSource` only, exactly as before. **Tests (`SessionManagerTest.java`):** - Updated `rosterViewExposesTheCharterReceiptButNeverTheCharterText` to assert `charterBytes` against the receipt's own value (not a hardcoded literal), plus the real UTF-8 length of the composed string. - Added `rosterViewOmitsCharterBytesAndDigestWhenNoCharterWasComposed` — no role charter and no reply charter composed reports `charterSource="none"` and neither `charterSha256` nor `charterBytes`. - Added `rosterViewOmitsCharterFieldsEntirelyWhenTheReceiptItselfIsAbsent` — `session.charterReceipt() == null` still suppresses all three keys. No existing test needed to be weakened. **Build:** `mvn -o clean install` from the worktree — `Tests run: 1821, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`.

agent added 1 commit 2026-09-20 11:15:18 +02:00

#604 item 1: fleet_list reports charterBytes alongside charterSha256

CI / shell-tests (pull_request) Failing after 9s

Details

CI / contract (pull_request) Successful in 1m24s

Details

CI / build (pull_request) Successful in 2m26s

Details

8b986a52e0

CharterReceipt carries a byte count next to its digest, but the roster
projection in SessionManager.rosterView only ever copied the digest
across. A digest tells a lead whether two members' charters match; it
cannot say how far apart they are when they don't. Report charterBytes
too, nested in the same conditional as charterSha256 so the two travel
together: the receipt's own contract only ever pairs a non-null digest
with a real byte count, and a member with no composed charter reports
charterSource alone, unchanged from before.

Tests: the existing charter-receipt roster test now asserts charterBytes
against the receipt's own value (not a literal), plus two new cases —
no charter composed (source "none", no digest, no size) and the receipt
itself absent (no charter keys at all).

ltms added 1 commit 2026-09-20 11:18:03 +02:00

Correct the invariant claimed in the charterBytes comment

CI / shell-tests (pull_request) Failing after 7s

Details

CI / build (pull_request) Successful in 1m36s

Details

CI / contract (pull_request) Successful in 1m42s

Details

ad3d81941f

The comment said CharterReceipt never pairs a null digest with a non-zero
byte count. It can. compose() derives the digest with digestOf(), which
returns null for blank text, while the byte count is getBytes().length,
which does not. A whitespace-only role charter on a profile with no MCP
produces exactly that pair.

No behaviour change. The gate already omits both fields on that path, which
is the right answer — a size with no digest would describe an artifact we
cannot fingerprint. Only the stated reason was wrong, and a false invariant
in a comment is worse than no comment, because the next reader will widen
the gate on the strength of it.

ltms commented

2026-09-20 11:18:11 +02:00

Lead review. The change is right and the tests are good. I merged after one correction, pushed as ad3d819.

Verified by me, not taken from the report

mvn -o clean install (from fleetd/) exit: 0
reports=143 tests=1821 failures=0 errors=0 skipped=0
SessionManagerTest: tests="74" errors="0" skipped="0" failures="0"

What the tests do well

Two things worth naming, because they are the reason this did not repeat the defect that started #604's sibling ticket.

assertEquals(receipt.charterBytes(), view.get("charterBytes")) asserts against the receipt's own value rather than a typed-in literal. A hardcoded expected size passes even when the wiring reads the wrong field. Pairing it with the real UTF-8 length of the composed string covers the other direction.

The two new tests use assertFalse(view.containsKey(...)). That is what makes the no-charter case real evidence — a containsKey assertion cannot be satisfied by code that always writes the field, which an assertNull could.

The correction: the comment stated an invariant that is false

The added comment said:

the receipt only ever pairs a non-null digest with a real byte count (CharterReceipt's own contract: no composed charter is an explicit null digest AND a zero byte count, never one without the other)

CharterReceipt.compose() can produce exactly the pair that sentence rules out:

byte[] bytes = composed.getBytes(StandardCharsets.UTF_8);
return new CharterReceipt(role, profile, source, digestOf(composed), bytes.length);

digestOf() returns null for blank text. getBytes().length does not. So a whitespace-only charter gives a null digest beside a non-zero size.

Reachable how: a blank fleet.charters.<role> on a profile with no MCP, so no reply charter is appended and composed is the blank role charter alone. That needs an odd config, so it is a defect on paper rather than one anybody has hit — which is exactly why it would have survived.

No behaviour change was needed. Gating on the digest already handles it, and handles it correctly: it omits both fields. Reporting a size with no digest would claim "they differ by N bytes" about an artifact we cannot fingerprint, which is the failure this whole ticket is about.

So only the stated reason was wrong. I replaced it with the real one and added the line that matters for the next reader: never widen this gate to the receipt-level null check without deciding what that case should report.

Why I did not let it stand

A comment claiming an invariant is a free test case — you can always go and check it. When it is false, it is worse than no comment, because the next person widens the gate on the strength of it and the compiler says nothing.

It also mattered here specifically. This PR exists because a digest cannot tell you how two things differ. Shipping it with a false claim about when digests and sizes travel together would have put the error inside the fix for it.

Items 2 and 3 of #604 stay open — they are the javadoc notes, and correctly out of scope for this unit.

Lead review. The change is right and the tests are good. I merged after one correction, pushed as `ad3d819`. ## Verified by me, not taken from the report ``` mvn -o clean install (from fleetd/) exit: 0 reports=143 tests=1821 failures=0 errors=0 skipped=0 SessionManagerTest: tests="74" errors="0" skipped="0" failures="0" ``` ## What the tests do well Two things worth naming, because they are the reason this did not repeat the defect that started #604's sibling ticket. `assertEquals(receipt.charterBytes(), view.get("charterBytes"))` asserts against the receipt's own value rather than a typed-in literal. A hardcoded expected size passes even when the wiring reads the wrong field. Pairing it with the real UTF-8 length of the composed string covers the other direction. The two new tests use `assertFalse(view.containsKey(...))`. That is what makes the no-charter case real evidence — a `containsKey` assertion cannot be satisfied by code that always writes the field, which an `assertNull` could. ## The correction: the comment stated an invariant that is false The added comment said: > the receipt only ever pairs a non-null digest with a real byte count (CharterReceipt's own contract: no composed charter is an explicit null digest AND a zero byte count, never one without the other) `CharterReceipt.compose()` can produce exactly the pair that sentence rules out: ```java byte[] bytes = composed.getBytes(StandardCharsets.UTF_8); return new CharterReceipt(role, profile, source, digestOf(composed), bytes.length); ``` `digestOf()` returns `null` for **blank** text. `getBytes().length` does not. So a whitespace-only charter gives a `null` digest beside a non-zero size. Reachable how: a blank `fleet.charters.<role>` on a profile with no MCP, so no reply charter is appended and `composed` is the blank role charter alone. That needs an odd config, so it is a defect on paper rather than one anybody has hit — which is exactly why it would have survived. **No behaviour change was needed.** Gating on the digest already handles it, and handles it correctly: it omits both fields. Reporting a size with no digest would claim "they differ by N bytes" about an artifact we cannot fingerprint, which is the failure this whole ticket is about. So only the stated reason was wrong. I replaced it with the real one and added the line that matters for the next reader: **never widen this gate to the receipt-level null check without deciding what that case should report.** ## Why I did not let it stand A comment claiming an invariant is a free test case — you can always go and check it. When it is false, it is worse than no comment, because the next person widens the gate on the strength of it and the compiler says nothing. It also mattered here specifically. This PR exists because a digest cannot tell you *how* two things differ. Shipping it with a false claim about when digests and sizes travel together would have put the error inside the fix for it. Items 2 and 3 of #604 stay open — they are the javadoc notes, and correctly out of scope for this unit.

ltms merged commit 9640deeffc into main

2026-09-20 11:18:17 +02:00

ltms referenced this pull request

2026-09-20 11:41:48 +02:00

Flaky: MessageServiceTest.anAlreadyCollectedTicketProducesNoNudge orders an async scheduler with Thread.sleep #608

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#605