fleetd #759: fix three role-model comments (Role, ConnectionIdentity, MemberPresence) #760

Closed
agent wants to merge 0 commits from worker/759-role-model-comments-5d8409-3 into main
Member

Fixes three of the five findings in #759. Comment-only change, no behaviour touched.

  • Role.PRIMARY: under loopback-trust the primary is a caller that resolves to NO herdr pane at all, not merely "not a worker pane" (that is OBSERVER).
  • ConnectionIdentity (class javadoc + callerTerminal): this class yields a terminal id for the calling pane, or null for no pane; it assigns no role - CallerResolver does.
  • MemberPresence class javadoc: presence is populated for the peers whose deliverability depends on a proven live MCP contact (worker, architect, observer), not workers only; the role list lives in FleetMcp.markTrackedCallerPresent, not restated here.

Findings 4 and 5 (in FleetMcp.java) are intentionally excluded - another worker is editing that file for #756/#758.

Build: mvn clean install - BUILD SUCCESS, Tests run: 2137, Failures: 0, Errors: 0, Skipped: 0 (surefire XML sum matches).

Fixes three of the five findings in #759. Comment-only change, no behaviour touched. - Role.PRIMARY: under loopback-trust the primary is a caller that resolves to NO herdr pane at all, not merely "not a worker pane" (that is OBSERVER). - ConnectionIdentity (class javadoc + callerTerminal): this class yields a terminal id for the calling pane, or null for no pane; it assigns no role - CallerResolver does. - MemberPresence class javadoc: presence is populated for the peers whose deliverability depends on a proven live MCP contact (worker, architect, observer), not workers only; the role list lives in FleetMcp.markTrackedCallerPresent, not restated here. Findings 4 and 5 (in FleetMcp.java) are intentionally excluded - another worker is editing that file for #756/#758. Build: mvn clean install - BUILD SUCCESS, Tests run: 2137, Failures: 0, Errors: 0, Skipped: 0 (surefire XML sum matches).
agent added 1 commit 2026-10-05 10:18:18 +02:00
fleetd #759: fix three role-model comments that describe the pre-CB-501 rule
CI / shell-tests (pull_request) Failing after 6s
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Failing after 2m7s
0f2ec7a6b5
Role.PRIMARY, ConnectionIdentity (class javadoc + callerTerminal), and
MemberPresence's class javadoc each state a role model the code no longer
implements. Comment-only change.
Owner

Merged locally to main as 39accf7, together with a follow-up commit of mine. Closing this PR because the commits are in; it was not merged through the forge.

What I verified rather than took on trust

The diff is comment-only. I filtered the diff for changed lines not inside a javadoc block and got zero. A zero match proves nothing on its own, so I ran the same filter against e42eab5, which added a test method: it reported 13 code lines. My first control attempt was itself vacuous — I pointed it at 02eff4c, which only touched Markdown, so *.java matched nothing and the filter looked like it worked when it had never been exercised.

{@link #OBSERVER} resolves — Role.java:58 declares it.

All three replacements are factually right. I re-read CallerResolver.resolve, Principal.ownsSession and markTrackedCallerPresent against each new sentence.

Build on the merged bytes: MVN_EXIT=0, and I summed the surefire XML myself rather than reading Maven's line — xml=178 tests=2137 failures=0 errors=0 skipped=0. Same count as bare main, which is what a comment-only change should produce. The merged tree hash d6d80a4e equals main's tree after the merge, so the bytes I tested are the bytes that shipped.

My follow-up commit, and why it was needed

You flagged a third copy of the same wrong rule in ConnectionIdentity's Caller record. You were right, and my brief was at fault: it named two spots in that file when there were three. I fixed it rather than leave the file half-corrected after declaring it done.

Two smaller things in your text:

  • {@code FleetMcp.markTrackedCallerPresent} — a name inside {@code} is invisible to the compiler, which is the rot mechanism this ticket is about. A {@link} was not the answer either: inject has no dependency on mcp, and rule 5 forbids new cross-package edges. So I replaced the pointer with the principle — deliverability rests on proving a live MCP contact rather than on a configured registry entry. That stays true whichever roles qualify, needs no cross-reference, and still explains why leads and collaborators are excluded.
  • (CB-113) survived in a line you rewrote. Rule 1 bans ticket keys in comments, and the line became yours when you touched it. Dropped.

Neither is a criticism of the judgement — scoping the MemberPresence first paragraph beyond the sentence I quoted was the right call, and you said so explicitly.

Still open on #759

Findings 4 and 5 are untouched as instructed — FleetMcp.java is held by another worker. They go in once that PR lands.

Your second observation, isLoopback's notebook-style javadoc, is real and I am adding it to #759 along with Caller#resolved, which carries the same thing: dates, fleetd #317, #305 and a narrative of what drifted. Out of scope here, correctly left alone.

Merged locally to `main` as `39accf7`, together with a follow-up commit of mine. Closing this PR because the commits are in; it was not merged through the forge. ## What I verified rather than took on trust **The diff is comment-only.** I filtered the diff for changed lines not inside a javadoc block and got zero. A zero match proves nothing on its own, so I ran the same filter against `e42eab5`, which added a test method: it reported 13 code lines. My first control attempt was itself vacuous — I pointed it at `02eff4c`, which only touched Markdown, so `*.java` matched nothing and the filter looked like it worked when it had never been exercised. **`{@link #OBSERVER}` resolves** — `Role.java:58` declares it. **All three replacements are factually right.** I re-read `CallerResolver.resolve`, `Principal.ownsSession` and `markTrackedCallerPresent` against each new sentence. **Build on the merged bytes:** `MVN_EXIT=0`, and I summed the surefire XML myself rather than reading Maven's line — `xml=178 tests=2137 failures=0 errors=0 skipped=0`. Same count as bare `main`, which is what a comment-only change should produce. The merged tree hash `d6d80a4e` equals `main`'s tree after the merge, so the bytes I tested are the bytes that shipped. ## My follow-up commit, and why it was needed You flagged a third copy of the same wrong rule in `ConnectionIdentity`'s `Caller` record. You were right, and my brief was at fault: it named two spots in that file when there were three. I fixed it rather than leave the file half-corrected after declaring it done. Two smaller things in your text: - **`{@code FleetMcp.markTrackedCallerPresent}`** — a name inside `{@code}` is invisible to the compiler, which is the rot mechanism this ticket is about. A `{@link}` was not the answer either: `inject` has no dependency on `mcp`, and rule 5 forbids new cross-package edges. So I replaced the pointer with the principle — *deliverability rests on proving a live MCP contact rather than on a configured registry entry*. That stays true whichever roles qualify, needs no cross-reference, and still explains why leads and collaborators are excluded. - **`(CB-113)`** survived in a line you rewrote. Rule 1 bans ticket keys in comments, and the line became yours when you touched it. Dropped. Neither is a criticism of the judgement — scoping the `MemberPresence` first paragraph beyond the sentence I quoted was the right call, and you said so explicitly. ## Still open on #759 Findings 4 and 5 are untouched as instructed — `FleetMcp.java` is held by another worker. They go in once that PR lands. Your second observation, `isLoopback`'s notebook-style javadoc, is real and I am adding it to #759 along with `Caller#resolved`, which carries the same thing: dates, `fleetd #317`, `#305` and a narrative of what drifted. Out of scope here, correctly left alone.
ltms closed this pull request 2026-10-05 10:27:25 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 6s
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Failing after 2m7s

Pull request closed

Sign in to join this conversation.