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

fleetd #749: pin PackageCyclesTest's exceptions to exact edges, not whole packages #751

Closed
agent wants to merge 0 commits from worker/749-edge-baseline-28d1a0-3 into main
pull from: worker/749-edge-baseline-28d1a0-3
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-10-05 07:46:16 +02:00
Member
Copy Link
Copy Source

What

PackageCyclesTest's ignoreCycle() helper exempted an entire package pair from cycle detection, in both directions, forever. That meant any new dependency added later between an already-excepted pair (auth/mcp, mcp/msg, inject/msg, metrics/msg, msg/session) was silently exempted too -- widest exactly where msg makes the gate matter most. The javadoc claimed otherwise.

Fix

Replaced the package-wide ignore with a frozen baseline (BASELINE_EDGES) of the 45 exact origin class -> target class dependencies that exist today between those five pairs. SliceRule.ignoreDependency(String, String) ignores only those exact edges (exact fully-qualified-name match, confirmed empirically), so the pre-existing beFreeOfCycles() check still catches a brand new cycle anywhere else.

A brand new one-directional dependency added inside an already-excepted pair does not always form a cycle by itself (the opposite direction's old edges are all exempted), so beFreeOfCycles() alone cannot be trusted to catch it. A dedicated checkBaselineMatchesTodaysEdges() directly compares the live dependency set (scoped to the five baselined package pairs) against BASELINE_EDGES and fails, naming the exact origin class, target class and package pair, on either:

  • a live edge not in the baseline (a new dependency), or
  • a baseline entry with no matching live edge (a stale/removed dependency).

Stale-entry decision: fails the build, not just reports. An exact-edge baseline is supposed to mirror reality; letting a removed edge rot in it would let a cycle-removal step from ticket #131 land without anyone noticing the baseline (and the pair's cycle-exception) could now shrink.

Javadoc rewritten to describe only the current contract -- no history, no ticket numbers, no dates.

Scope: only PackageCyclesTest.java. No production code changed; none of the five existing cycles were touched or removed (that is separate, ticketed work).

Proof the gate bites (criterion 2)

Temporarily added an unused WorktreeException field to MessageService (a msg -> session edge not in the baseline), ran PackageCyclesTest alone, confirmed it failed naming the exact new edge, then reverted (confirmed git diff on the production file is empty). Separately confirmed the stale-entry path fails too, by temporarily adding a bogus baseline entry to the test file and reverting from a backup copy (confirmed byte-identical after revert).

Build

rm -rf target/surefire-reports then mvn clean install from fleetd/: MVN_EXIT=0, BUILD SUCCESS. Summed from the 177 regenerated target/surefire-reports/*.xml files: Tests run: 2118, Failures: 0, Errors: 0, Skipped: 0.

## What `PackageCyclesTest`'s `ignoreCycle()` helper exempted an entire package pair from cycle detection, in both directions, forever. That meant any **new** dependency added later between an already-excepted pair (auth/mcp, mcp/msg, inject/msg, metrics/msg, msg/session) was silently exempted too -- widest exactly where `msg` makes the gate matter most. The javadoc claimed otherwise. ## Fix Replaced the package-wide ignore with a frozen baseline (`BASELINE_EDGES`) of the 45 exact `origin class -> target class` dependencies that exist today between those five pairs. `SliceRule.ignoreDependency(String, String)` ignores only those exact edges (exact fully-qualified-name match, confirmed empirically), so the pre-existing `beFreeOfCycles()` check still catches a brand new cycle anywhere else. A brand new one-directional dependency added inside an already-excepted pair does not always form a *cycle* by itself (the opposite direction's old edges are all exempted), so `beFreeOfCycles()` alone cannot be trusted to catch it. A dedicated `checkBaselineMatchesTodaysEdges()` directly compares the live dependency set (scoped to the five baselined package pairs) against `BASELINE_EDGES` and fails, naming the exact origin class, target class and package pair, on either: - a live edge not in the baseline (a new dependency), or - a baseline entry with no matching live edge (a stale/removed dependency). **Stale-entry decision:** fails the build, not just reports. An exact-edge baseline is supposed to mirror reality; letting a removed edge rot in it would let a cycle-removal step from ticket #131 land without anyone noticing the baseline (and the pair's cycle-exception) could now shrink. Javadoc rewritten to describe only the current contract -- no history, no ticket numbers, no dates. Scope: only `PackageCyclesTest.java`. No production code changed; none of the five existing cycles were touched or removed (that is separate, ticketed work). ## Proof the gate bites (criterion 2) Temporarily added an unused `WorktreeException` field to `MessageService` (a `msg -> session` edge not in the baseline), ran `PackageCyclesTest` alone, confirmed it failed naming the exact new edge, then reverted (confirmed `git diff` on the production file is empty). Separately confirmed the stale-entry path fails too, by temporarily adding a bogus baseline entry to the test file and reverting from a backup copy (confirmed byte-identical after revert). ## Build `rm -rf target/surefire-reports` then `mvn clean install` from `fleetd/`: `MVN_EXIT=0`, `BUILD SUCCESS`. Summed from the 177 regenerated `target/surefire-reports/*.xml` files: Tests run: 2118, Failures: 0, Errors: 0, Skipped: 0.
agent added 1 commit 2026-10-05 07:46:16 +02:00
fleetd #749: pin PackageCyclesTest's exceptions to exact edges, not whole packages
CI / shell-tests (pull_request) Failing after 7s
Details
CI / contract (pull_request) Successful in 1m8s
Details
CI / build (pull_request) Failing after 2m25s
Details
6677ec8c63 
ignoreCycle() used to exempt every dependency between two packages, in both
directions, for the whole package. That meant a brand new dependency added
later between an already-excepted pair (auth/mcp, mcp/msg, inject/msg,
metrics/msg, msg/session) was silently exempted too, exactly where the
msg package makes the gate matter most.

Replace the package-wide ignore with a frozen baseline of the 45 exact
origin-class -> target-class edges that exist today between those five
pairs, and ignore only those via SliceRule.ignoreDependency(String, String).
A new dependency between a baselined pair is not in that set, so it is no
longer ignored and the existing beFreeOfCycles() check (or, when the new
edge alone would not form a cycle, a dedicated set-equality check) fails
and names the exact origin class, target class and package pair.

The set-equality check also fails on a baseline entry whose dependency no
longer exists in the code, so a removed edge cannot rot in the baseline
and mask the pair's eligibility for the ticket #131 removal steps. Rewrote
the javadoc to describe only the current contract.
ltms closed this pull request 2026-10-05 07:50:10 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 7s
Details
CI / contract (pull_request) Successful in 1m8s
Details
CI / build (pull_request) Failing after 2m25s
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#751
Delete Branch "worker/749-edge-baseline-28d1a0-3"

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?