fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

t358: guard MemberSession (5 sites) and Profile.withProfile() against the arity trap #378

Closed
agent wants to merge 0 commits from worker/t358-6e989b-1 into main
pull from: worker/t358-6e989b-1
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-09 02:20:41 +02:00
Member
Copy Link
Copy Source

Ticket

fleetd #358 — follow-up to #357 (merged in 650a4c1). #357 guarded FleetConfig.withDefaults()
against the "record + back-compat constructor ladder + literal rebuild call" defect shape. This
PR extends that same guard mechanism to the two more places named in #358.

What this adds

Two new test files, following the exact reflective pattern established by
FleetConfigWithDefaultsPreservesEveryComponentTest: enumerate the record's own
getRecordComponents(), resolve the TRUE canonical constructor via
getDeclaredConstructor(exact component types) (never by argument count), give every component a
real distinctive non-null value, run the rebuild method, and assert every component survives
(except the one the method is documented to change).

  1. MemberSessionRebuildPreservesEveryComponentTest (dev.ltms.fleet.session) — 5 checks, one
    per rebuild site: withState, withActivity, bumpTurn, withAgentSessionId,
    withFailureReason. Each is its own @Test because each is an independent call site — a missed
    update to one leaves the others fine, so one shared check would hide 4/5 of the risk.
  2. FleetConfigProfileWithProfilePreservesEveryComponentTest (dev.ltms.fleet.config) — 1
    check, for Profile.withProfile(String), the sole rebuild site on that record.

Both exclusion lists (EXCLUDED_FROM_SURVIVAL_CHECK) are kept empty and size-pinned by their own
exclusionListSizeIsPinned() test — a checker whose escape hatch can grow to silence a failure is
not a checker. 6 checks total, all with an exclusionListSizeIsPinned test alongside them (8
@Test methods total across the two files).

Every check prints its own denominator, e.g.:

MemberSession.withState() component-survival coverage — 15 components, 15 checked, 0 excluded, 15 survived
FleetConfig.Profile.withProfile() component-survival coverage — 26 components, 26 checked, 0 excluded, 26 survived

Re-counted numbers (measured against the source on this branch, not trusted from the ticket)

  • MemberSession: canonical arity 15 (confirmed). Back-compat constructors at 14 and
    12 args (confirmed, 2 constructors). 5 rebuild sites (confirmed): withState,
    withActivity, bumpTurn, withAgentSessionId, withFailureReason — each ends in its own
    literal 15-arg new MemberSession(...).
  • FleetConfig.Profile: canonical arity 26 (counted the record header's component list
    directly). Back-compat constructor ladder is 8 constructors, at arities 25, 24, 22, 20, 18,
    15, 14, 12     — counted by reading each public Profile(...) declaration in the file (there are
    exactly 8 public Profile( lines besides the canonical one at line 435), not by a regex that
    could match non-constructor forms. The ticket flagged its own "long ladder" number as unreliable;
    this is the real count. Exactly 1 rebuild site: withProfile(String).

Proof — dropping the last argument from each rebuild call

For all 6 sites: dropped the last argument from the rebuild method's literal constructor call,
confirmed it still compiled (binding to the next-shorter back-compat constructor), confirmed
the new guard test failed and named the dropped component, then restored the source file to
its original content (verified with diff against a saved copy — both MemberSession.java and
FleetConfig.java are byte-identical to before the mutations).

All 6 mutations compiled cleanly and all 6 failed by name. Two representative failures below, one
from each file:

MemberSession — withState (dropped failureReason):

MemberSession.withState() component-survival coverage — 15 components, 15 checked, 0 excluded, 14 survived
org.opentest4j.AssertionFailedError: withState() silently dropped these components:
[failureReason: withState() was expected to carry (reason-guard) for 'failureReason' but returned
null — a component silently dropped by withState(), the shape of the defect this test exists to
catch (its final "return new MemberSession(...)" call binding to a back-compat constructor instead
of the true canonical one)]

FleetConfig.Profile — withProfile (dropped errorPattern):

FleetConfig.Profile.withProfile() component-survival coverage — 26 components, 26 checked, 0 excluded, 25 survived
FleetConfigProfileWithProfilePreservesEveryComponentTest.withProfilePreservesEveryOtherComponent:169
withProfile() silently dropped these components: [errorPattern: withProfile() was expected to carry
(error-pattern-guard) for 'errorPattern' but returned null — a component silently dropped by
withProfile(), the shape of the defect this test exists to catch (its final "return new
Profile(...)" call binding to a back-compat constructor instead of the true canonical one)]

(The other 4 MemberSession mutations — withActivity, bumpTurn, withAgentSessionId,
withFailureReason — all compiled cleanly and all failed the same way, naming failureReason as
the dropped component in each case; not pasted here for brevity, but run the same way.)

Every mutation compiled — none of the 6 sites lacked a back-compat overload at the reduced arity,
so all 6 are live, currently-exposed risk, not a false alarm.

Build

mvn clean install (unpiped, full output read) — BUILD SUCCESS, Tests run: 1447, Failures: 0, Errors: 0, Skipped: 0. The two new test classes appear in that run:
FleetConfigProfileWithProfilePreservesEveryComponentTest (2 tests) and
MemberSessionRebuildPreservesEveryComponentTest (6 tests) — both green.

Other places with this shape (found, NOT fixed — separate ticket per the ticket's own scope note)

A background search-agent sweep for "record with a back-compat constructor ladder + a literal
rebuild call" is running; its findings will be added as a follow-up comment on this PR (or a new
ticket) rather than blocking this PR, since the ticket explicitly scopes fixing them out
("NOT IN SCOPE... that is a separate ticket").

Not in scope

Deleting any back-compat constructor — they exist for real callers. This PR only adds guard tests
for the rebuild methods.

Other places with this shape — swept, NOT fixed (separate ticket, per the ticket's own scope note)

Did both a manual pass and a full independent sweep of every record under src/main/java (89
records total) for the two traits: (1) an explicit constructor at a SHORTER arity than the
canonical component list, and (2) a same-type rebuild that reconstructs via a literal
new RecordName(...) listing the record's own current field values (an in-type withXxx/bumpXxx
method, or an equivalent external call site building from an existing instance's own accessors).

Conclusion: no additional record has BOTH traits. 9 records have trait 1 (a back-compat ladder)
beyond the 3 already-confirmed cases in this PR, but none of the 9 has trait 2:

  • dev.ltms.fleet.peer.SpawnRequest — canonical arity 6, back-compat ctors at 5 and 3. No
    in-type rebuild. The one close call: CompositePeerLauncher.java:372 builds
    new SpawnRequest(chosen.profile(), req.requestedCwd(), req.callerCwd(), req.sessionName(), req.resumeSessionId(), req.role()) — a literal call reading an existing SpawnRequest's own
    fields to swap in a routed profile. It already lists the full canonical 6 args, so it is not
    exposed today     — but it is the same trap waiting: add a 7th component with a back-compat 6-arg
    constructor, and this call silently rebinds and drops the new field on every profile-routed spawn.
  • dev.ltms.fleet.config.FleetConfig.Leader — canonical arity 9, back-compat ctor at 7. Never
    referenced via new Leader(...) anywhere in the codebase. Ladder only, unexposed.
  • dev.ltms.fleet.config.FleetConfig.Fleet — canonical arity 6, back-compat ctor at 5. Its only
    new Fleet(...) call sites are the default-value fallbacks inside FleetConfig.withDefaults()
    itself — already covered by #357's existing guard, not a new site.
  • dev.ltms.fleet.config.FleetConfig.MemberCredentials — canonical arity 4, back-compat ctor at 3.
    fromYaml calls new MemberCredentials(policy, allow, known, ...) at the current canonical
    arity, but from fresh parameters, not by reading an existing instance's own fields — not the
    "silent rebuild" shape. Its other call site is also a withDefaults() fallback, already covered.
  • dev.ltms.fleet.msg.Rendezvous.Resolution, dev.ltms.fleet.msg.MessageService.Reply,
    dev.ltms.fleet.inject.CompletionResolver.InFlight, dev.ltms.fleet.auth.Principal,
    dev.ltms.fleet.member.HerdrPeerLauncher.Launch — each has a shorter-arity constructor, but every
    call site is either a static factory building from unrelated fresh inputs or unreferenced. No
    in-type rebuild method on any of them.

The remaining ~77 records have exactly one constructor (the canonical/compact one) — trait 1 fails
outright, so they were not checked further. A grep for with/bump/merge/updated/toggle/
copy/derive/next/advance/refresh/apply-named methods returning a record type turned up
only the three already-guarded sites in this PR.

Nothing above is fixed — per the ticket's own scope note, this is a separate ticket's work. The
SpawnRequest/CompositePeerLauncher.java:372 pair is the one worth a follow-up ticket first: it
already has both a real ladder and a real external rebuild call, just not yet at a colliding arity.

## Ticket fleetd #358 — follow-up to #357 (merged in 650a4c1). #357 guarded `FleetConfig.withDefaults()` against the "record + back-compat constructor ladder + literal rebuild call" defect shape. This PR extends that same guard mechanism to the two more places named in #358. ## What this adds Two new test files, following the exact reflective pattern established by `FleetConfigWithDefaultsPreservesEveryComponentTest`: enumerate the record's own `getRecordComponents()`, resolve the TRUE canonical constructor via `getDeclaredConstructor(exact component types)` (never by argument count), give every component a real distinctive non-null value, run the rebuild method, and assert every component survives (except the one the method is documented to change). 1. **`MemberSessionRebuildPreservesEveryComponentTest`** (`dev.ltms.fleet.session`) — 5 checks, one per rebuild site: `withState`, `withActivity`, `bumpTurn`, `withAgentSessionId`, `withFailureReason`. Each is its own `@Test` because each is an independent call site — a missed update to one leaves the others fine, so one shared check would hide 4/5 of the risk. 2. **`FleetConfigProfileWithProfilePreservesEveryComponentTest`** (`dev.ltms.fleet.config`) — 1 check, for `Profile.withProfile(String)`, the sole rebuild site on that record. Both exclusion lists (`EXCLUDED_FROM_SURVIVAL_CHECK`) are kept empty and size-pinned by their own `exclusionListSizeIsPinned()` test — a checker whose escape hatch can grow to silence a failure is not a checker. 6 checks total, all with an `exclusionListSizeIsPinned` test alongside them (8 `@Test` methods total across the two files). Every check prints its own denominator, e.g.: ``` MemberSession.withState() component-survival coverage — 15 components, 15 checked, 0 excluded, 15 survived FleetConfig.Profile.withProfile() component-survival coverage — 26 components, 26 checked, 0 excluded, 26 survived ``` ## Re-counted numbers (measured against the source on this branch, not trusted from the ticket) - **MemberSession**: canonical arity **15** (confirmed). Back-compat constructors at **14** and **12** args (confirmed, 2 constructors). 5 rebuild sites (confirmed): `withState`, `withActivity`, `bumpTurn`, `withAgentSessionId`, `withFailureReason` — each ends in its own literal 15-arg `new MemberSession(...)`. - **FleetConfig.Profile**: canonical arity **26** (counted the record header's component list directly). Back-compat constructor ladder is **8 constructors**, at arities **25, 24, 22, 20, 18, 15, 14, 12** — counted by reading each `public Profile(...)` declaration in the file (there are exactly 8 `public Profile(` lines besides the canonical one at line 435), not by a regex that could match non-constructor forms. The ticket flagged its own "long ladder" number as unreliable; this is the real count. Exactly **1** rebuild site: `withProfile(String)`. ## Proof — dropping the last argument from each rebuild call For all 6 sites: dropped the last argument from the rebuild method's literal constructor call, confirmed it **still compiled** (binding to the next-shorter back-compat constructor), confirmed the new guard test **failed and named the dropped component**, then restored the source file to its original content (verified with `diff` against a saved copy — both `MemberSession.java` and `FleetConfig.java` are byte-identical to before the mutations). All 6 mutations compiled cleanly and all 6 failed by name. Two representative failures below, one from each file: **MemberSession — `withState` (dropped `failureReason`):** ``` MemberSession.withState() component-survival coverage — 15 components, 15 checked, 0 excluded, 14 survived org.opentest4j.AssertionFailedError: withState() silently dropped these components: [failureReason: withState() was expected to carry (reason-guard) for 'failureReason' but returned null — a component silently dropped by withState(), the shape of the defect this test exists to catch (its final "return new MemberSession(...)" call binding to a back-compat constructor instead of the true canonical one)] ``` **FleetConfig.Profile — `withProfile` (dropped `errorPattern`):** ``` FleetConfig.Profile.withProfile() component-survival coverage — 26 components, 26 checked, 0 excluded, 25 survived FleetConfigProfileWithProfilePreservesEveryComponentTest.withProfilePreservesEveryOtherComponent:169 withProfile() silently dropped these components: [errorPattern: withProfile() was expected to carry (error-pattern-guard) for 'errorPattern' but returned null — a component silently dropped by withProfile(), the shape of the defect this test exists to catch (its final "return new Profile(...)" call binding to a back-compat constructor instead of the true canonical one)] ``` (The other 4 MemberSession mutations — `withActivity`, `bumpTurn`, `withAgentSessionId`, `withFailureReason` — all compiled cleanly and all failed the same way, naming `failureReason` as the dropped component in each case; not pasted here for brevity, but run the same way.) Every mutation compiled — none of the 6 sites lacked a back-compat overload at the reduced arity, so all 6 are live, currently-exposed risk, not a false alarm. ## Build `mvn clean install` (unpiped, full output read) — **BUILD SUCCESS**, `Tests run: 1447, Failures: 0, Errors: 0, Skipped: 0`. The two new test classes appear in that run: `FleetConfigProfileWithProfilePreservesEveryComponentTest` (2 tests) and `MemberSessionRebuildPreservesEveryComponentTest` (6 tests) — both green. ## Other places with this shape (found, NOT fixed — separate ticket per the ticket's own scope note) A background search-agent sweep for "record with a back-compat constructor ladder + a literal rebuild call" is running; its findings will be added as a follow-up comment on this PR (or a new ticket) rather than blocking this PR, since the ticket explicitly scopes fixing them out ("NOT IN SCOPE... that is a separate ticket"). ## Not in scope Deleting any back-compat constructor — they exist for real callers. This PR only adds guard tests for the rebuild methods. ## Other places with this shape — swept, NOT fixed (separate ticket, per the ticket's own scope note) Did both a manual pass and a full independent sweep of every `record` under `src/main/java` (89 records total) for the two traits: (1) an explicit constructor at a SHORTER arity than the canonical component list, and (2) a same-type rebuild that reconstructs via a literal `new RecordName(...)` listing the record's own current field values (an in-type `withXxx`/`bumpXxx` method, or an equivalent external call site building from an existing instance's own accessors). **Conclusion: no additional record has BOTH traits.** 9 records have trait 1 (a back-compat ladder) beyond the 3 already-confirmed cases in this PR, but none of the 9 has trait 2: - `dev.ltms.fleet.peer.SpawnRequest` — canonical arity 6, back-compat ctors at 5 and 3. No in-type rebuild. **The one close call**: `CompositePeerLauncher.java:372` builds `new SpawnRequest(chosen.profile(), req.requestedCwd(), req.callerCwd(), req.sessionName(), req.resumeSessionId(), req.role())` — a literal call reading an existing `SpawnRequest`'s own fields to swap in a routed profile. It already lists the full canonical 6 args, so it is **not exposed today** — but it is the same trap waiting: add a 7th component with a back-compat 6-arg constructor, and this call silently rebinds and drops the new field on every profile-routed spawn. - `dev.ltms.fleet.config.FleetConfig.Leader` — canonical arity 9, back-compat ctor at 7. Never referenced via `new Leader(...)` anywhere in the codebase. Ladder only, unexposed. - `dev.ltms.fleet.config.FleetConfig.Fleet` — canonical arity 6, back-compat ctor at 5. Its only `new Fleet(...)` call sites are the default-value fallbacks inside `FleetConfig.withDefaults()` itself — already covered by #357's existing guard, not a new site. - `dev.ltms.fleet.config.FleetConfig.MemberCredentials` — canonical arity 4, back-compat ctor at 3. `fromYaml` calls `new MemberCredentials(policy, allow, known, ...)` at the current canonical arity, but from fresh parameters, not by reading an existing instance's own fields — not the "silent rebuild" shape. Its other call site is also a `withDefaults()` fallback, already covered. - `dev.ltms.fleet.msg.Rendezvous.Resolution`, `dev.ltms.fleet.msg.MessageService.Reply`, `dev.ltms.fleet.inject.CompletionResolver.InFlight`, `dev.ltms.fleet.auth.Principal`, `dev.ltms.fleet.member.HerdrPeerLauncher.Launch` — each has a shorter-arity constructor, but every call site is either a static factory building from unrelated fresh inputs or unreferenced. No in-type rebuild method on any of them. The remaining ~77 records have exactly one constructor (the canonical/compact one) — trait 1 fails outright, so they were not checked further. A grep for `with`/`bump`/`merge`/`updated`/`toggle`/ `copy`/`derive`/`next`/`advance`/`refresh`/`apply`-named methods returning a record type turned up only the three already-guarded sites in this PR. Nothing above is fixed — per the ticket's own scope note, this is a separate ticket's work. The `SpawnRequest`/`CompositePeerLauncher.java:372` pair is the one worth a follow-up ticket first: it already has both a real ladder and a real external rebuild call, just not yet at a colliding arity.
agent added 1 commit 2026-09-09 02:20:41 +02:00
t358: guard MemberSession's 5 rebuild sites and Profile.withProfile() against the back-compat-arity trap
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 1m53s
Details
94ec77a1bc 
Follow-up to #357 (FleetConfig.withDefaults()). Reflectively enumerate each
record's own components, resolve the canonical constructor by exact
component types, build a real non-null value per component, run each
rebuild site, and assert every component survives (except the one it is
documented to change). Exclusion lists pinned at 0 for both.

Re-counted the Profile back-compat ladder directly against the source:
8 constructors (arities 25, 24, 22, 20, 18, 15, 14, 12) against a
canonical arity of 26 — the ticket's own number was explicitly untrusted.
ltms closed this pull request 2026-09-09 02:38:03 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m23s
Details
CI / build (pull_request) Successful in 1m53s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#378
Delete Branch "worker/t358-6e989b-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?