fleetd #273: validate exhaustedPattern regex at load, like errorPattern #278

Closed
agent wants to merge 0 commits from worker/fix-273-exhausted-pattern-9665b5-6 into main
Member

fleetd #273 — exhaustedPattern had no load-time check

FleetConfig.rejectMalformedErrorPattern compiles every profile's errorPattern
at config load. If it fails, it refuses to start and names the profile and the key.

exhaustedPattern is the sibling key. It had no such check. It was compiled
without a guard, in Fleetd.main (Fleetd.java, around line 347). So a value like
profiles.<name>.exhaustedPattern: "[" passed load(). Then it crashed the whole
daemon at boot. The error was a raw PatternSyntaxException. It did not name the
profile or the key.

What changed

  1. I renamed FleetConfig.rejectMalformedErrorPattern to
    rejectMalformedProfilePatterns. It now compiles both errorPattern and
    exhaustedPattern, for every profile. If either key fails, on any profile, all
    failures are collected. They are sorted, joined, and thrown as one
    IllegalStateException. The message still starts with refusing to start: ,
    and each entry still reads profiles.<name>.<key> ("<value>"): <message>, the
    same style as before.
  2. I updated every call site. There was exactly one: FleetConfig.load.
  3. I did not touch Fleetd.java's Pattern.compile call for exhaustedPattern.
    This was out of scope. It is now safe, because load() already rejects a bad
    value before that line ever runs.
  4. I added tests in FleetConfigTest, using the existing errorPattern tests as
    the model. They cover:
    • a bad exhaustedPattern is refused, and the profile and key are named
    • a bad errorPattern on one profile and a bad exhaustedPattern on another
      are both reported from one load() call, in one message
    • valid errorPattern and exhaustedPattern on the same profile still load
    • a blank exhaustedPattern becomes null, the same as when it is unset

Proof the tests catch the defect

I reverted only FleetConfig.java and kept the new tests. Then I ran
FleetConfigTest again. Two tests turned red, as expected:

  • aProfileWithAMalformedExhaustedPatternIsRejectedAtLoadNamingTheProfileAndKey —
    "Expected java.lang.IllegalStateException to be thrown, but nothing was thrown."
  • aMalformedErrorPatternAndAMalformedExhaustedPatternAreBothReportedFromOneLoad —
    the exhaustedPattern profile name and key were missing. The old, unchanged
    message only names the errorPattern side.

The other two new tests (valid patterns load fine, blank normalizes to null) still
passed. That is expected — they do not depend on this fix. I then restored the
fix. The full suite is green again (see Build below).

Caveat for reviewers: during the revert-and-restore step, I hit a real hazard
in this setup. git stash uses one stack shared across all worktrees of this
repo. It is not per-worktree. While I was mid-stash, another worker's session
(working on fleetd #274, in GitWorktrees.java) ran its own git stash at close
to the same time. Its change landed in my working tree and, for a moment,
overwrote my in-progress edit.

I recovered by retyping my own edit from memory. I checked it was byte-identical
to the lost stash content with diff. I then pushed the other worker's
GitWorktrees.java change back onto the stash stack, untouched, so nothing of
theirs was lost. Before every commit I ran git status and git diff --stat to
confirm only my two intended files were staged.

Also reported, not fixed — same defect shape elsewhere

The ticket asked for a list of other config values that are compiled or parsed
only after FleetConfig.load() returns, outside any load-time check. I did not
fix these:

  • Fleetd.java:154-155 — cfg.herdrSocket() goes straight into Path.of(...) at
    startup, with no guard. A string that is not a legal path throws an uncaught
    InvalidPathException. It does not name the herdrSocket key.
  • Fleetd.java:159-160 — the same pattern, one line down, for
    cfg.memberHerdrSocket().
  • HerdrPeerLauncher.java:1447 (memberScrubParentDir()) — cfg.worktreeRoot()
    goes into Path.of(...) lazily, at the first member spawn (later than daemon
    startup). A bad path string throws an uncaught InvalidPathException, and it
    does not name worktreeRoot.
  • Fleetd.java:719 — cfg.bind().host() and cfg.bind().port() go straight into
    app.start(host, port). A port out of range throws whatever the HTTP server
    throws. It is not checked or named as bind.port first.

I checked these too, and I am ruling them out — they do not match this defect's
shape: a bad broker/coordinator URI already falls back to an in-memory inbox, with
a clear log.warn, instead of crashing. SubscriptionGuard's baseUrl parsing is
already wrapped, and it throws a clear GuardException that names the profile.
quarantineCooldownSeconds is a typed number field, so a bad value already fails
inside Jackson's YAML.readValue, during load() itself — not a lazy parse
later. auth.tokenEnv() already throws a clear IllegalStateException that names
the setting.

Build

cd fleetd && mvn clean install

Full run, not piped: BUILD SUCCESS.
Overall: Tests run: 1279, Failures: 0, Errors: 0, Skipped: 0.
FleetConfigTest alone: Tests run: 123, Failures: 0, Errors: 0, Skipped: 0.

Ticket: fleetd #273

## fleetd #273 — exhaustedPattern had no load-time check `FleetConfig.rejectMalformedErrorPattern` compiles every profile's `errorPattern` at config load. If it fails, it refuses to start and names the profile and the key. `exhaustedPattern` is the sibling key. It had no such check. It was compiled without a guard, in `Fleetd.main` (`Fleetd.java`, around line 347). So a value like `profiles.<name>.exhaustedPattern: "["` passed `load()`. Then it crashed the whole daemon at boot. The error was a raw `PatternSyntaxException`. It did not name the profile or the key. ## What changed 1. I renamed `FleetConfig.rejectMalformedErrorPattern` to `rejectMalformedProfilePatterns`. It now compiles both `errorPattern` and `exhaustedPattern`, for every profile. If either key fails, on any profile, all failures are collected. They are sorted, joined, and thrown as one `IllegalStateException`. The message still starts with `refusing to start: `, and each entry still reads `profiles.<name>.<key> ("<value>"): <message>`, the same style as before. 2. I updated every call site. There was exactly one: `FleetConfig.load`. 3. I did not touch `Fleetd.java`'s `Pattern.compile` call for `exhaustedPattern`. This was out of scope. It is now safe, because `load()` already rejects a bad value before that line ever runs. 4. I added tests in `FleetConfigTest`, using the existing `errorPattern` tests as the model. They cover: - a bad `exhaustedPattern` is refused, and the profile and key are named - a bad `errorPattern` on one profile and a bad `exhaustedPattern` on another are both reported from one `load()` call, in one message - valid `errorPattern` and `exhaustedPattern` on the same profile still load - a blank `exhaustedPattern` becomes `null`, the same as when it is unset ## Proof the tests catch the defect I reverted only `FleetConfig.java` and kept the new tests. Then I ran `FleetConfigTest` again. Two tests turned red, as expected: - `aProfileWithAMalformedExhaustedPatternIsRejectedAtLoadNamingTheProfileAndKey` — "Expected java.lang.IllegalStateException to be thrown, but nothing was thrown." - `aMalformedErrorPatternAndAMalformedExhaustedPatternAreBothReportedFromOneLoad` — the `exhaustedPattern` profile name and key were missing. The old, unchanged message only names the `errorPattern` side. The other two new tests (valid patterns load fine, blank normalizes to null) still passed. That is expected — they do not depend on this fix. I then restored the fix. The full suite is green again (see Build below). **Caveat for reviewers:** during the revert-and-restore step, I hit a real hazard in this setup. `git stash` uses one stack shared across *all* worktrees of this repo. It is not per-worktree. While I was mid-stash, another worker's session (working on fleetd #274, in `GitWorktrees.java`) ran its own `git stash` at close to the same time. Its change landed in my working tree and, for a moment, overwrote my in-progress edit. I recovered by retyping my own edit from memory. I checked it was byte-identical to the lost stash content with `diff`. I then pushed the other worker's `GitWorktrees.java` change back onto the stash stack, untouched, so nothing of theirs was lost. Before every commit I ran `git status` and `git diff --stat` to confirm only my two intended files were staged. ## Also reported, not fixed — same defect shape elsewhere The ticket asked for a list of other config values that are compiled or parsed only *after* `FleetConfig.load()` returns, outside any load-time check. I did not fix these: - `Fleetd.java:154-155` — `cfg.herdrSocket()` goes straight into `Path.of(...)` at startup, with no guard. A string that is not a legal path throws an uncaught `InvalidPathException`. It does not name the `herdrSocket` key. - `Fleetd.java:159-160` — the same pattern, one line down, for `cfg.memberHerdrSocket()`. - `HerdrPeerLauncher.java:1447` (`memberScrubParentDir()`) — `cfg.worktreeRoot()` goes into `Path.of(...)` lazily, at the first member spawn (later than daemon startup). A bad path string throws an uncaught `InvalidPathException`, and it does not name `worktreeRoot`. - `Fleetd.java:719` — `cfg.bind().host()` and `cfg.bind().port()` go straight into `app.start(host, port)`. A port out of range throws whatever the HTTP server throws. It is not checked or named as `bind.port` first. I checked these too, and I am ruling them out — they do not match this defect's shape: a bad broker/coordinator URI already falls back to an in-memory inbox, with a clear `log.warn`, instead of crashing. `SubscriptionGuard`'s `baseUrl` parsing is already wrapped, and it throws a clear `GuardException` that names the profile. `quarantineCooldownSeconds` is a typed number field, so a bad value already fails inside Jackson's `YAML.readValue`, during `load()` itself — not a lazy parse later. `auth.tokenEnv()` already throws a clear `IllegalStateException` that names the setting. ## Build ``` cd fleetd && mvn clean install ``` Full run, not piped: `BUILD SUCCESS`. Overall: `Tests run: 1279, Failures: 0, Errors: 0, Skipped: 0`. `FleetConfigTest` alone: `Tests run: 123, Failures: 0, Errors: 0, Skipped: 0`. Ticket: fleetd #273
agent added 1 commit 2026-09-04 05:07:45 +02:00
fleetd #273: validate exhaustedPattern regex at load, like errorPattern
CI / build (pull_request) Successful in 1m36s
CI / contract (pull_request) Successful in 2m11s
f04e934b94
FleetConfig.rejectMalformedErrorPattern only compiled errorPattern eagerly
at config load. exhaustedPattern was compiled unguarded in Fleetd.main,
so profiles.<name>.exhaustedPattern: "[" passed load() and then crashed
the whole daemon at boot with a raw PatternSyntaxException naming neither
the profile nor the key.

Rename the validator to rejectMalformedProfilePatterns and extend it to
also compile every non-blank exhaustedPattern, reporting
profiles.<name>.exhaustedPattern ("<value>"): <message> in the same style
as errorPattern. Both keys are collected and reported together from a
single load. Fleetd.java's compile site is left as-is per scope — it is
now safe because load already rejects a bad value.

Added tests covering: a bad exhaustedPattern is refused; a bad pattern in
each key is reported together in one message; valid patterns still load;
a blank/absent exhaustedPattern is ignored.
ltms closed this pull request 2026-09-04 05:27:40 +02:00
Some checks are pending
CI / build (pull_request) Successful in 1m36s
CI / contract (pull_request) Successful in 2m11s

Pull request closed

Sign in to join this conversation.