fleetd #529: promote CapturedLog to a shared test helper, close the logger-level leak #533

2026-09-12T07:53:31+02:00

agent commented

2026-09-12 07:53:31 +02:00

Fixes fleetd #529.

The defect

ch.qos.logback.classic.Logger instances are cached per class and shared for the whole JVM, and
surefire reuses forks by default. A test that pins a shared logger's level to capture its output,
and restores only the appender in finally, leaves that level pinned for every test that runs
after it — in the same test class, or in a different class sharing the same fork.

What changed

Promoted CapturedLog (the private helper merged in PR #527 for fleetd #525) out of
SessionManagerTest into a shared test-scope class: dev.ltms.fleet.testing.CapturedLog.
Behaviour is unchanged for the three original entry points: at(Class, Level) captures the old
level and pins the new one, of(Class) attaches without changing the level, close() detaches
the appender and restores the captured level.
Two small, behaviour-preserving additions to CapturedLog, both needed by real call sites in
this sweep, neither changing what the three original methods do:
- at(String, Level) / of(String) — AuditLog logs through a named "audit" logger
  (LoggerFactory.getLogger("audit")), not a class. at(Class, Level) would have captured the
  wrong Logger instance (a different name resolves to a different cached logger).
- setLevel(Level) — lets a fixture that already pinned a coarser baseline (e.g.
  GitWorktreesTest's @BeforeEach pinning WARN) re-pin further for one test (to INFO) without
  changing what close() restores — always the level captured at construction, never an
  intermediate value.
Converted all 19 unrestored setLevel pins across the 9 files the ticket named, plus
SessionManagerTest itself (whose own CapturedLog moved out), to the shared helper:
FleetdAwaitHerdrTest (1), FleetdReplyInboxSelectionTest (1), AuditLogTest (1),
CompletionResolverTest (1), InjectorTest (4), LeadRolloverTest (1),
AmqpConnectionFailureLoggerTest (1), GitWorktreesTest (8), WorktreeSessionManagerTest (1).
Kept every existing intentional pin exactly as instructed: SessionManagerTest's two explicit
Level.INFO pins, and its @BeforeAll DEBUG baseline (pinSessionManagerLoggerToAKnownBaseline)
— I did not touch either.
Added an ordered proving test to WorktreeSessionManagerTest (see below).

Acceptance criteria — reported as run

1. `mvn -f fleetd/pom.xml clean install`

Ran unpiped, redirected to a file, echo $? on its own line:

$ mvn clean install > /tmp/claude-503-mvn-build-final.log 2>&1; echo "exit=$?"
exit=0

Verbatim summary line and result from that log:

[INFO] Tests run: 1701, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

(1701 = the pre-existing 1700 plus the one new proving test added below.)

2. Survey re-run — 0 files

I did not trust the ticket's table and re-derived my own survey before starting: grep -rn "setLevel(" src/test/java found 55 call sites; I classified each by hand as "restored" (a
captured-variable restore, or already the intentional SessionManagerTest baseline pair) or
"unrestored literal pin" (a bare Level.WARN/Level.INFO/Level.DEBUG argument with no restore
anywhere in the file). That classification produced exactly the 9 files fixed here — matching the
ticket's table by coincidence of measurement, not by trusting it.

Completion re-run, same methodology, automated as a small script (source shown for
reproducibility — classifies a file as leaking only if it has a literal setLevel(Level.X) pin,
no restore-to-a-captured-variable setLevel(<ident>) anywhere in the file, and does not use
CapturedLog):

$ python3 - <<'PY'
import re, subprocess
files = subprocess.run(["grep","-rl","setLevel(","src/test/java","--include=*.java"],
                        capture_output=True, text=True).stdout.splitlines()
leaking = []
for f in files:
    text = open(f).read()
    if f.endswith("testing/CapturedLog.java"):
        continue
    uses_captured_log = "CapturedLog" in text
    literal_pins = re.findall(r'setLevel\(\s*(?:ch\.qos\.logback\.classic\.)?Level\.\w+\s*\)', text)
    restores = re.findall(r'setLevel\(\s*([A-Za-z_][A-Za-z0-9_.]*)\s*\)', text)
    restores = [r for r in restores if not r.startswith("Level")]
    if literal_pins and not restores and not uses_captured_log:
        leaking.append((f, len(literal_pins)))
print("NONE (0 files)" if not leaking else leaking)
PY
NONE (0 files)

3. One proving test, ordered

Added to WorktreeSessionManagerTest:

@TestMethodOrder(MethodOrderer.OrderAnnotation.class) on the class.
@Order(1) on the existing releasePreservesDirtyWorktreeAndLogsWarn (the dirty-worktree
release test — it already pins SessionManager.class's logger to WARN via CapturedLog).
A new @BeforeAll/@AfterAll pair that captures this class's own true starting level for
SessionManager.class's logger and pins a distinctive baseline (TRACE) before any test runs —
mirroring SessionManagerTest's own pattern, so "restored" can be told apart from "happened to
already read WARN".
A new @Order(2) test, sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn,
asserting the logger's level is back to TRACE right after @Order(1) runs.

Ordering forced by: JUnit 5's @TestMethodOrder(MethodOrderer.OrderAnnotation.class), which
guarantees @Order(1) runs before @Order(2), and gives every unannotated method in the class the
lowest priority (so they run after both, in whatever relative order they already ran in) — the
identical mechanism SessionManagerTest already uses for its own #525 proving test.

This proves the within-class case only. JUnit 5 does not guarantee cross-class ordering by
default, and surefire's default class order is not something a single test can force. The
cross-class leak fleetd #525 actually measured — WorktreeSessionManagerTest's dirty-worktree test
pinning WARN and bleeding into a later-running SessionManagerTest in the same fork — is fixed by
the same CapturedLog mechanism this proving test exercises, but I did not write a test that
asserts the cross-class case itself, and I am stating plainly that it stays unproven by
construction, not dressing it up as proven.

Full-file run after adding the test (control, before any mutation):

$ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?"
exit=0
Tests run: 25, Failures: 0, Errors: 0, Skipped: 0

4. Two mutations, both expected red

Mutation (a): remove the level restore from CapturedLog.close().

Before mutating, shasum -a 256 of the pristine file:
486d6f5b5a30dc5ef7f75e5e10be353e720fb0de503825e88e8d96e30a61a2f7

Change: close() body reduced to only logger.detachAppender(appender); (the setLevel(originalLevel)
line removed).

Proof the mutation was actually applied — two greps with different search strings, each with a
control against the pristine copy, plus a grep -n re-read:

grep -c "logger.setLevel(originalLevel);" <pristine>   -> 1
grep -c "logger.setLevel(originalLevel);" <mutant>     -> 0

grep -c "setLevel(" <pristine>   -> 4
grep -c "setLevel(" <mutant>     -> 3

grep -n -A4 "public void close" <mutant>:
88:    public void close() {
89-        logger.detachAppender(appender);
90-    }
91-}

Test run with the mutation applied — red, assertion and exit code:

$ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?"
exit=1
Tests run: 25, Failures: 1, Errors: 0, Skipped: 0
WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn
  AssertionFailedError: ... expected: <TRACE> but was: <WARN>

Restored the file from the pristine copy and confirmed byte-identical with shasum -a 256
(both files hashed to the same 486d6f5b...a1a2f7), then re-ran the control:

$ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?"
exit=0
Tests run: 25, Failures: 0, Errors: 0, Skipped: 0

Mutation (a) killed. It survived nothing, so no extra harness-proof cell is needed for it.

Mutation (b): put back the bare detachAppender-only finally in WorktreeSessionManagerTest's
releasePreservesDirtyWorktreeAndLogsWarn.

Before mutating, shasum -a 256 of the pristine file:
a722a98d828c82e00415d2a924d341e177ded704262aaa5963ea2d09a683df94

Change: replaced the try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) { ... }
block with the original raw LoggerContext/ListAppender/addAppender/setLevel(WARN) +
try { ... } finally { sessionLog.detachAppender(appender); } shape (no level restore).

Proof of application — two different-string greps, each with a control, plus a grep -n re-read:

grep -c "sessionLog.detachAppender(appender);" <pristine>   -> 0
grep -c "sessionLog.detachAppender(appender);" <mutant>     -> 1

grep -c "CapturedLog" <pristine>   -> 4
grep -c "CapturedLog" <mutant>     -> 3

grep -n -A10 "void releasePreservesDirtyWorktreeAndLogsWarn" <mutant> shows the bare
LoggerContext/ListAppender/addAppender/setLevel(WARN) block, no CapturedLog, in the mutated method.

Test run with the mutation applied — red, assertion and exit code:

$ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?"
exit=1
Tests run: 25, Failures: 1, Errors: 0, Skipped: 0
WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn
  AssertionFailedError: ... expected: <TRACE> but was: <WARN>

Restored from the pristine copy, confirmed byte-identical with shasum -a 256 (both hashed to
a722a98d...9a683df94), then re-ran the control:

$ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?"
exit=0
Tests run: 25, Failures: 0, Errors: 0, Skipped: 0

Mutation (b) killed. It also survived nothing, so no extra harness-proof cell is needed for it.

5/6. Mutation-application proof and survival handling

Covered inline above for each mutation: two greps with different search strings, each with a
pristine control, plus a grep -n re-read confirming the actual line content. Both mutants were
killed by the proving test, so per the ticket's own rule ("a killed mutation needs no extra
harness-proof cell"), neither needed further proof of what a survival would have meant — neither
survived.

Item 4 — other unrestored shared static state (reported, not fixed)

fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java:55-59 —
captureFleetdLogs() attaches a fresh ListAppender to the shared Fleetd.class logger and
returns it, but the file never calls detachAppender anywhere (I counted 0 in this file). It is
called from three tests (lines 70, 117, 131), so up to three ListAppenders stay attached to
Fleetd.class's logger for the rest of the JVM fork's life. This is outside the 9-file scope
named in this ticket, so I did not fix it — I only counted addAppender/detachAppender calls
project-wide (46 vs 45) to find this one file with the mismatch, and did not investigate further
whether it causes an observable test failure anywhere.

I did not do an exhaustive sweep for every possible kind of shared-static-state leak (only this
appender-specific check plus a quick look at System.setProperty usage, which was all
paired/restored, and Thread.currentThread().interrupt() usage, which is the correct restore idiom
after a caught InterruptedException everywhere I checked except FleetdAwaitHerdrTest, which
already documents and handles its one deliberate case) — this is what I found within the scope of
this ticket, not a claim that it is the only such issue in the tree.

Hard constraints

No test writes a real .claude.json; nothing in this change touches that fixture pattern.
No environment variable value was printed anywhere in this work — only names/booleans (see the
GITEA_HOST/GITEA_TOKEN presence check in my worktree, reported as booleans only).
Staged only the 11 files changed (10 modified + 1 new CapturedLog.java); never used
git add -A. Did not touch scripts/, .mcp.json, wiki/, or fleetd.yaml.
Ran mvn directly via Bash; I have no IDE MCP tooling as a worker, so mvn output above is the
full verification I have.

Files changed

fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLog.java (new)
fleetd/src/test/java/dev/ltms/fleet/FleetdAwaitHerdrTest.java
fleetd/src/test/java/dev/ltms/fleet/FleetdReplyInboxSelectionTest.java
fleetd/src/test/java/dev/ltms/fleet/auth/AuditLogTest.java
fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java
fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java
fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java
fleetd/src/test/java/dev/ltms/fleet/msg/AmqpConnectionFailureLoggerTest.java
fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java
fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java
fleetd/src/test/java/dev/ltms/fleet/session/WorktreeSessionManagerTest.java

Caveat for review

The cross-class ordering case (the exact interleaving fleetd #525 measured — WorktreeSessionManagerTest
running before SessionManagerTest in one fork) is fixed by construction (both now use the same
restoring CapturedLog), but it is not covered by an ordered, deterministic test — only the
within-class case is. If a reviewer wants that cross-class case proven too, it would need something
like a JUnit test-suite class ordering configuration, which I did not add since it goes beyond a
single test file's scope and the ticket explicitly allows stating this as unproven.

Fixes fleetd #529. ## The defect `ch.qos.logback.classic.Logger` instances are cached per class and shared for the whole JVM, and surefire reuses forks by default. A test that pins a shared logger's level to capture its output, and restores only the appender in `finally`, leaves that level pinned for every test that runs after it — in the same test class, or in a different class sharing the same fork. ## What changed 1. Promoted `CapturedLog` (the private helper merged in PR #527 for fleetd #525) out of `SessionManagerTest` into a shared test-scope class: `dev.ltms.fleet.testing.CapturedLog`. Behaviour is unchanged for the three original entry points: `at(Class, Level)` captures the old level and pins the new one, `of(Class)` attaches without changing the level, `close()` detaches the appender **and** restores the captured level. 2. Two small, behaviour-preserving additions to `CapturedLog`, both needed by real call sites in this sweep, neither changing what the three original methods do: - `at(String, Level)` / `of(String)` — `AuditLog` logs through a named `"audit"` logger (`LoggerFactory.getLogger("audit")`), not a class. `at(Class, Level)` would have captured the wrong `Logger` instance (a different name resolves to a different cached logger). - `setLevel(Level)` — lets a fixture that already pinned a coarser baseline (e.g. `GitWorktreesTest`'s `@BeforeEach` pinning WARN) re-pin further for one test (to INFO) without changing what `close()` restores — always the level captured at construction, never an intermediate value. 3. Converted all 19 unrestored `setLevel` pins across the 9 files the ticket named, plus `SessionManagerTest` itself (whose own `CapturedLog` moved out), to the shared helper: `FleetdAwaitHerdrTest` (1), `FleetdReplyInboxSelectionTest` (1), `AuditLogTest` (1), `CompletionResolverTest` (1), `InjectorTest` (4), `LeadRolloverTest` (1), `AmqpConnectionFailureLoggerTest` (1), `GitWorktreesTest` (8), `WorktreeSessionManagerTest` (1). 4. Kept every existing intentional pin exactly as instructed: `SessionManagerTest`'s two explicit `Level.INFO` pins, and its `@BeforeAll` `DEBUG` baseline (`pinSessionManagerLoggerToAKnownBaseline`) — I did not touch either. 5. Added an ordered proving test to `WorktreeSessionManagerTest` (see below). ## Acceptance criteria — reported as run ### 1. `mvn -f fleetd/pom.xml clean install` Ran unpiped, redirected to a file, `echo $?` on its own line: ``` $ mvn clean install > /tmp/claude-503-mvn-build-final.log 2>&1; echo "exit=$?" exit=0 ``` Verbatim summary line and result from that log: ``` [INFO] Tests run: 1701, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` (1701 = the pre-existing 1700 plus the one new proving test added below.) ### 2. Survey re-run — 0 files I did not trust the ticket's table and re-derived my own survey before starting: `grep -rn "setLevel(" src/test/java` found 55 call sites; I classified each by hand as "restored" (a captured-variable restore, or already the intentional `SessionManagerTest` baseline pair) or "unrestored literal pin" (a bare `Level.WARN`/`Level.INFO`/`Level.DEBUG` argument with no restore anywhere in the file). That classification produced exactly the 9 files fixed here — matching the ticket's table by coincidence of measurement, not by trusting it. Completion re-run, same methodology, automated as a small script (source shown for reproducibility — classifies a file as leaking only if it has a literal `setLevel(Level.X)` pin, no restore-to-a-captured-variable `setLevel(<ident>)` anywhere in the file, and does not use `CapturedLog`): ``` $ python3 - <<'PY' import re, subprocess files = subprocess.run(["grep","-rl","setLevel(","src/test/java","--include=*.java"], capture_output=True, text=True).stdout.splitlines() leaking = [] for f in files: text = open(f).read() if f.endswith("testing/CapturedLog.java"): continue uses_captured_log = "CapturedLog" in text literal_pins = re.findall(r'setLevel$\s*(?:ch\.qos\.logback\.classic\.)?Level\.\w+\s*$', text) restores = re.findall(r'setLevel$\s*([A-Za-z_][A-Za-z0-9_.]*)\s*$', text) restores = [r for r in restores if not r.startswith("Level")] if literal_pins and not restores and not uses_captured_log: leaking.append((f, len(literal_pins))) print("NONE (0 files)" if not leaking else leaking) PY NONE (0 files) ``` ### 3. One proving test, ordered Added to `WorktreeSessionManagerTest`: - `@TestMethodOrder(MethodOrderer.OrderAnnotation.class)` on the class. - `@Order(1)` on the existing `releasePreservesDirtyWorktreeAndLogsWarn` (the dirty-worktree release test — it already pins `SessionManager.class`'s logger to WARN via `CapturedLog`). - A new `@BeforeAll`/`@AfterAll` pair that captures this class's own true starting level for `SessionManager.class`'s logger and pins a distinctive baseline (`TRACE`) before any test runs — mirroring `SessionManagerTest`'s own pattern, so "restored" can be told apart from "happened to already read WARN". - A new `@Order(2)` test, `sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn`, asserting the logger's level is back to `TRACE` right after `@Order(1)` runs. **Ordering forced by**: JUnit 5's `@TestMethodOrder(MethodOrderer.OrderAnnotation.class)`, which guarantees `@Order(1)` runs before `@Order(2)`, and gives every unannotated method in the class the *lowest* priority (so they run after both, in whatever relative order they already ran in) — the identical mechanism `SessionManagerTest` already uses for its own #525 proving test. **This proves the within-class case only.** JUnit 5 does not guarantee cross-class ordering by default, and surefire's default class order is not something a single test can force. The cross-class leak fleetd #525 actually measured — `WorktreeSessionManagerTest`'s dirty-worktree test pinning WARN and bleeding into a later-running `SessionManagerTest` in the same fork — is fixed by the same `CapturedLog` mechanism this proving test exercises, but I did not write a test that asserts the cross-class case itself, and I am stating plainly that it stays unproven by construction, not dressing it up as proven. Full-file run after adding the test (control, before any mutation): ``` $ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?" exit=0 Tests run: 25, Failures: 0, Errors: 0, Skipped: 0 ``` ### 4. Two mutations, both expected red **Mutation (a): remove the level restore from `CapturedLog.close()`.** Before mutating, `shasum -a 256` of the pristine file: `486d6f5b5a30dc5ef7f75e5e10be353e720fb0de503825e88e8d96e30a61a2f7` Change: `close()` body reduced to only `logger.detachAppender(appender);` (the `setLevel(originalLevel)` line removed). Proof the mutation was actually applied — two greps with different search strings, each with a control against the pristine copy, plus a `grep -n` re-read: ``` grep -c "logger.setLevel(originalLevel);" <pristine> -> 1 grep -c "logger.setLevel(originalLevel);" <mutant> -> 0 grep -c "setLevel(" <pristine> -> 4 grep -c "setLevel(" <mutant> -> 3 grep -n -A4 "public void close" <mutant>: 88: public void close() { 89- logger.detachAppender(appender); 90- } 91-} ``` Test run with the mutation applied — **red**, assertion and exit code: ``` $ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?" exit=1 Tests run: 25, Failures: 1, Errors: 0, Skipped: 0 WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn AssertionFailedError: ... expected: <TRACE> but was: <WARN> ``` Restored the file from the pristine copy and confirmed byte-identical with `shasum -a 256` (both files hashed to the same `486d6f5b...a1a2f7`), then re-ran the control: ``` $ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?" exit=0 Tests run: 25, Failures: 0, Errors: 0, Skipped: 0 ``` Mutation (a) killed. It survived nothing, so no extra harness-proof cell is needed for it. **Mutation (b): put back the bare `detachAppender`-only `finally` in `WorktreeSessionManagerTest`'s `releasePreservesDirtyWorktreeAndLogsWarn`.** Before mutating, `shasum -a 256` of the pristine file: `a722a98d828c82e00415d2a924d341e177ded704262aaa5963ea2d09a683df94` Change: replaced the `try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) { ... }` block with the original raw `LoggerContext`/`ListAppender`/`addAppender`/`setLevel(WARN)` + `try { ... } finally { sessionLog.detachAppender(appender); }` shape (no level restore). Proof of application — two different-string greps, each with a control, plus a `grep -n` re-read: ``` grep -c "sessionLog.detachAppender(appender);" <pristine> -> 0 grep -c "sessionLog.detachAppender(appender);" <mutant> -> 1 grep -c "CapturedLog" <pristine> -> 4 grep -c "CapturedLog" <mutant> -> 3 grep -n -A10 "void releasePreservesDirtyWorktreeAndLogsWarn" <mutant> shows the bare LoggerContext/ListAppender/addAppender/setLevel(WARN) block, no CapturedLog, in the mutated method. ``` Test run with the mutation applied — **red**, assertion and exit code: ``` $ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?" exit=1 Tests run: 25, Failures: 1, Errors: 0, Skipped: 0 WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn AssertionFailedError: ... expected: <TRACE> but was: <WARN> ``` Restored from the pristine copy, confirmed byte-identical with `shasum -a 256` (both hashed to `a722a98d...9a683df94`), then re-ran the control: ``` $ mvn -q test -Dtest=WorktreeSessionManagerTest; echo "exit=$?" exit=0 Tests run: 25, Failures: 0, Errors: 0, Skipped: 0 ``` Mutation (b) killed. It also survived nothing, so no extra harness-proof cell is needed for it. ### 5/6. Mutation-application proof and survival handling Covered inline above for each mutation: two greps with different search strings, each with a pristine control, plus a `grep -n` re-read confirming the actual line content. Both mutants were killed by the proving test, so per the ticket's own rule ("a killed mutation needs no extra harness-proof cell"), neither needed further proof of what a survival would have meant — neither survived. ## Item 4 — other unrestored shared static state (reported, not fixed) - `fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java:55-59` — `captureFleetdLogs()` attaches a fresh `ListAppender` to the shared `Fleetd.class` logger and returns it, but the file never calls `detachAppender` anywhere (I counted 0 in this file). It is called from three tests (lines 70, 117, 131), so up to three `ListAppender`s stay attached to `Fleetd.class`'s logger for the rest of the JVM fork's life. This is outside the 9-file scope named in this ticket, so I did not fix it — I only counted `addAppender`/`detachAppender` calls project-wide (46 vs 45) to find this one file with the mismatch, and did not investigate further whether it causes an observable test failure anywhere. I did not do an exhaustive sweep for every possible kind of shared-static-state leak (only this appender-specific check plus a quick look at `System.setProperty` usage, which was all paired/restored, and `Thread.currentThread().interrupt()` usage, which is the correct restore idiom after a caught `InterruptedException` everywhere I checked except `FleetdAwaitHerdrTest`, which already documents and handles its one deliberate case) — this is what I found within the scope of this ticket, not a claim that it is the only such issue in the tree. ## Hard constraints - No test writes a real `.claude.json`; nothing in this change touches that fixture pattern. - No environment variable value was printed anywhere in this work — only names/booleans (see the `GITEA_HOST`/`GITEA_TOKEN` presence check in my worktree, reported as booleans only). - Staged only the 11 files changed (10 modified + 1 new `CapturedLog.java`); never used `git add -A`. Did not touch `scripts/`, `.mcp.json`, `wiki/`, or `fleetd.yaml`. - Ran `mvn` directly via Bash; I have no IDE MCP tooling as a worker, so `mvn` output above is the full verification I have. ## Files changed - `fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLog.java` (new) - `fleetd/src/test/java/dev/ltms/fleet/FleetdAwaitHerdrTest.java` - `fleetd/src/test/java/dev/ltms/fleet/FleetdReplyInboxSelectionTest.java` - `fleetd/src/test/java/dev/ltms/fleet/auth/AuditLogTest.java` - `fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java` - `fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java` - `fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java` - `fleetd/src/test/java/dev/ltms/fleet/msg/AmqpConnectionFailureLoggerTest.java` - `fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java` - `fleetd/src/test/java/dev/ltms/fleet/session/SessionManagerTest.java` - `fleetd/src/test/java/dev/ltms/fleet/session/WorktreeSessionManagerTest.java` ## Caveat for review The cross-class ordering case (the exact interleaving fleetd #525 measured — `WorktreeSessionManagerTest` running before `SessionManagerTest` in one fork) is fixed by construction (both now use the same restoring `CapturedLog`), but it is not covered by an ordered, deterministic test — only the within-class case is. If a reviewer wants that cross-class case proven too, it would need something like a JUnit test-suite class ordering configuration, which I did not add since it goes beyond a single test file's scope and the ticket explicitly allows stating this as unproven.

agent added 1 commit 2026-09-12 07:53:31 +02:00

fleetd #529 : promote CapturedLog to a shared test helper, close the logger-level leak

CI / contract (pull_request) Successful in 1m14s

Details

CI / build (pull_request) Successful in 1m39s

Details

8ea5c2bb1f

ch.qos.logback.classic.Logger instances are cached per class and shared for the
whole JVM, and surefire reuses forks. A test that pins a shared logger's level
and restores only the appender leaves that level pinned for every test that
runs after it, in the same class or a different one in the same fork.

Move CapturedLog (merged in #527 for #525) out of SessionManagerTest into
dev.ltms.fleet.testing.CapturedLog, and convert all 19 unrestored setLevel
pins across 9 files to it, so there is exactly one way to capture and pin a
logger in this test tree:
 - FleetdAwaitHerdrTest, FleetdReplyInboxSelectionTest, AuditLogTest,
   CompletionResolverTest, InjectorTest (4), LeadRolloverTest,
   AmqpConnectionFailureLoggerTest, GitWorktreesTest (8),
   WorktreeSessionManagerTest.

AuditLog logs through a named "audit" logger rather than a class, so
CapturedLog gains String-named at()/of() overloads alongside the existing
Class-based ones, plus a setLevel() method so a fixture that already pinned a
coarser baseline (GitWorktreesTest's @BeforeEach) can re-pin further for one
test without losing what close() restores.

Adds an ordered proving test to WorktreeSessionManagerTest asserting the
SessionManager logger level is back to a known baseline after the dirty-
worktree release test runs; this proves the within-class case only, since
JUnit does not guarantee cross-class ordering.

Every existing intentional pin (SessionManagerTest's two explicit INFO pins
and its @BeforeAll DEBUG baseline) is left untouched, per the ticket.

ltms merged commit af9589783e into main

2026-09-12 07:57:25 +02:00

ltms referenced this pull request

2026-09-12 07:59:46 +02:00

FleetdLeadMailboxSelectionTest leaks three ListAppenders onto the shared Fleetd logger, and the appender-balance invariant catches it #535

ltms referenced this pull request

2026-09-12 08:00:26 +02:00

Tests pin shared logger levels and never restore them: 9 files, 19 pins, one proven cross-class collision — share SessionManagerTest's CapturedLog #529

agent referenced this issue from a commit

2026-09-12 08:17:28 +02:00

fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog

agent referenced this pull request