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

CB-627: enforce package boundaries with an ArchUnit test instead of splitting into Maven modules #131

New Issue
Open
opened 2026-08-22 21:35:57 +02:00 by ltms · 1 comment
ltms commented 2026-08-22 21:35:57 +02:00
Owner
Copy Link
Copy Source

Part of CB-621 (#125). This replaces the 5-module Maven split, which CB-621 withdrew.

Why the module split was withdrawn

The split was proposed as a layout decision. It is really a refactor, and its boundaries contradict the real import graph. Three cycles, each verified in the code:

cycle evidence
auth <-> mcp auth/CallerResolver.java:3 imports mcp.ConnectionIdentity; mcp/BridgeMcp.java has 5 auth imports
msg <-> mcp msg/LeadHeartbeatLoop.java:5 and msg/ReplyPushLoop.java:5 import mcp.PrimaryRegistry; BridgeMcp imports msg
core <-> peer-herdr 20 files across ~8 packages import herdr, while member imports config, guard, herdr, peer, placement

The draft also named only 8 packages out of 16. Total is 16,795 LOC, so 7,307 LOC had no module, including the two largest packages:

msg 2665   member 2596   config 2225   session 1667   mcp 1460   inject 1136
herdr 992  auth 845   rest 605   peer 509   placement 465   lead 292
metrics 282   health 271   guard 72   logging 30

One deployable jar, one deployment, one team, ~16k LOC. A multi-module build buys nothing here and costs a parent pom, path churn on every open branch, and a broken working-directory: bridged in CI.

Scope

Add an ArchUnit test that fails the build on a package cycle. Start by recording the three cycles above as accepted exceptions, so the test goes green today and no new cycle can be added.

Then remove them one at a time, each in its own PR:

  1. auth <-> mcp: move ConnectionIdentity so that authz does not depend on the MCP layer.
  2. msg <-> mcp: PrimaryRegistry is used by loops in msg; move it or put an interface between them.
  3. core <-> herdr: Agent, AgentStatus and AgentControl are domain vocabulary, not adapter detail. They belong in the peer SPI. This is the one that would have to happen before any future module split.

Acceptance criteria

  • The test fails when a new cycle is introduced. Prove it by adding one on a scratch branch.
  • The accepted-exception list is explicit, and each entry names the ticket that will remove it.
  • mvn clean install green.
  • No package moves in the same PR as the test itself.

Note for later

Revisit a real module split only when a second consumer exists, and only after step 3 above lands. Until then this test gives the boundary discipline the split was wanted for, at none of its cost.

Part of CB-621 (#125). This replaces the 5-module Maven split, which CB-621 withdrew. ## Why the module split was withdrawn The split was proposed as a layout decision. It is really a refactor, and its boundaries contradict the real import graph. Three cycles, each verified in the code: | cycle | evidence | |---|---| | `auth` <-> `mcp` | `auth/CallerResolver.java:3` imports `mcp.ConnectionIdentity`; `mcp/BridgeMcp.java` has 5 `auth` imports | | `msg` <-> `mcp` | `msg/LeadHeartbeatLoop.java:5` and `msg/ReplyPushLoop.java:5` import `mcp.PrimaryRegistry`; `BridgeMcp` imports `msg` | | core <-> peer-herdr | 20 files across ~8 packages import `herdr`, while `member` imports `config`, `guard`, `herdr`, `peer`, `placement` | The draft also named only 8 packages out of 16. Total is 16,795 LOC, so **7,307 LOC had no module**, including the two largest packages: ``` msg 2665 member 2596 config 2225 session 1667 mcp 1460 inject 1136 herdr 992 auth 845 rest 605 peer 509 placement 465 lead 292 metrics 282 health 271 guard 72 logging 30 ``` One deployable jar, one deployment, one team, ~16k LOC. A multi-module build buys nothing here and costs a parent pom, path churn on every open branch, and a broken `working-directory: bridged` in CI. ## Scope Add an ArchUnit test that fails the build on a package cycle. Start by **recording the three cycles above as accepted exceptions**, so the test goes green today and no new cycle can be added. Then remove them one at a time, each in its own PR: 1. `auth` <-> `mcp`: move `ConnectionIdentity` so that authz does not depend on the MCP layer. 2. `msg` <-> `mcp`: `PrimaryRegistry` is used by loops in `msg`; move it or put an interface between them. 3. core <-> `herdr`: `Agent`, `AgentStatus` and `AgentControl` are domain vocabulary, not adapter detail. They belong in the `peer` SPI. This is the one that would have to happen before any future module split. ## Acceptance criteria - The test fails when a new cycle is introduced. Prove it by adding one on a scratch branch. - The accepted-exception list is explicit, and each entry names the ticket that will remove it. - `mvn clean install` green. - No package moves in the same PR as the test itself. ## Note for later Revisit a real module split only when a second consumer exists, and only after step 3 above lands. Until then this test gives the boundary discipline the split was wanted for, at none of its cost.
ltms added this to the 2.0 — one operation centre, many hosts milestone 2026-08-22 21:35:57 +02:00
ltms commented 2026-09-04 03:27:13 +02:00
Author
Owner
Copy Link
Copy Source

The test is merged as 2fa673d (PR #270). PackageCyclesTest fails the build on any new cycle, with five explicit, narrow exceptions. No packages moved, per this ticket's own rule.

This ticket's cycle list was wrong, and the code disagrees with it in both directions. The worker re-derived the graph instead of trusting the text above, which is the reason this is worth reading.

cycle status
auth <-> mcp real — step 1 stands
mcp <-> msg real — step 2 stands
core <-> herdr does not exist in main code
inject <-> msg new, not in this ticket
metrics <-> msg new
msg <-> session new

herdr has zero cross-package imports of any kind today — it is a leaf, so it cannot be in a cycle. It only appears if the scan includes test classes, which is test wiring rather than shipped architecture. The test therefore scans main code only (DO_NOT_INCLUDE_TESTS).

That matters for step 3 of the plan above. Step 3 was described as the one that must happen before any future module split; on today's code there is nothing to do. The real shape is different: msg is a hub, bidirectionally coupled to four packages — mcp, inject, metrics and session. If anything blocks a future split, it is msg, not herdr.

Three of the five exceptions are commented as needing their own follow-up step rather than being mapped onto steps 1–3. I left it that way deliberately — renumbering the plan is a design decision, and the evidence for it is only a few hours old.

Dependency: com.tngtech.archunit:archunit-junit5:1.5.0, test scope. I confirmed 1.5.0 is current (maven-metadata.xml reports <latest>1.5.0</latest>) — my own earlier check said 1.4.1 was newest and was wrong, because it used the search API with unsorted rows. Only notable transitive is slf4j-api:2.0.18, resolved down to the project's pinned 2.0.16; no guava, no separate ASM.

CVE clearance is still outstanding and I could not do it. CLAUDE.md requires jetbrains get_file_problems on pom.xml after any dependency change, for the Mend.io check. IntelliJ currently has four projects open and fleetd is not one of them, so the tool returns no target project. I did not open it, because that changes the operator's IDE state. This merge is gated on mvn clean install alone (1269 tests, 0 failures) — the Mend check still needs running.

Leaving this ticket open: the test has landed, the three removal PRs have not.

The test is merged as `2fa673d` (PR #270). `PackageCyclesTest` fails the build on any new cycle, with five explicit, narrow exceptions. No packages moved, per this ticket's own rule. **This ticket's cycle list was wrong, and the code disagrees with it in both directions.** The worker re-derived the graph instead of trusting the text above, which is the reason this is worth reading. | cycle | status | |---|---| | `auth <-> mcp` | real — step 1 stands | | `mcp <-> msg` | real — step 2 stands | | `core <-> herdr` | **does not exist in main code** | | `inject <-> msg` | **new, not in this ticket** | | `metrics <-> msg` | **new** | | `msg <-> session` | **new** | `herdr` has zero cross-package imports of any kind today — it is a leaf, so it cannot be in a cycle. It only appears if the scan includes test classes, which is test wiring rather than shipped architecture. The test therefore scans main code only (`DO_NOT_INCLUDE_TESTS`). That matters for step 3 of the plan above. Step 3 was described as the one that must happen before any future module split; on today's code there is nothing to do. The real shape is different: **`msg` is a hub, bidirectionally coupled to four packages** — `mcp`, `inject`, `metrics` and `session`. If anything blocks a future split, it is `msg`, not `herdr`. Three of the five exceptions are commented as needing their own follow-up step rather than being mapped onto steps 1–3. I left it that way deliberately — renumbering the plan is a design decision, and the evidence for it is only a few hours old. Dependency: `com.tngtech.archunit:archunit-junit5:1.5.0`, test scope. I confirmed 1.5.0 is current (`maven-metadata.xml` reports `<latest>1.5.0</latest>`) — my own earlier check said 1.4.1 was newest and was wrong, because it used the search API with unsorted rows. Only notable transitive is `slf4j-api:2.0.18`, resolved down to the project's pinned `2.0.16`; no guava, no separate ASM. **CVE clearance is still outstanding and I could not do it.** `CLAUDE.md` requires `jetbrains get_file_problems` on `pom.xml` after any dependency change, for the Mend.io check. IntelliJ currently has four projects open and `fleetd` is not one of them, so the tool returns no target project. I did not open it, because that changes the operator's IDE state. This merge is gated on `mvn clean install` alone (1269 tests, 0 failures) — the Mend check still needs running. Leaving this ticket open: the test has landed, the three removal PRs have not.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 2.0 — one operation centre, many hosts
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#131
Delete Branch "%!s()"

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?