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

Sweep: enum.name().toLowerCase() is the house idiom for wire tokens at 15 sites across 5 files, and no test pins any of the long-form ones #586

New Issue
Open
opened 2026-09-12 15:19:51 +02:00 by ltms · 0 comments
ltms commented 2026-09-12 15:19:51 +02:00
Owner
Copy Link
Copy Source

Sweep and classify. Change no production code. The output is a ranked list with a verdict per site.

Found while correcting #578. The fleet01 lead ran the first grep on a tree 91 commits behind main and flagged that the shape was not confined to one enum; every number below is re-measured on main at 204da67.

The shape

MessageService.Outcome leaks its constant spelling as its wire token, because readers call outcome().name().toLowerCase() instead of asking for a pinned wire name. #578 fixes that for one enum. The idiom is not confined to one enum.

grep -rn 'name()\s*\.\s*toLowerCase()' --include='*.java' over non-test sources: 15 hits, 5 files, covering at least six different enums.

file lines
mcp/FleetMcp.java 807, 1124, 1156, 1189, 1950, 1979
rest/FleetApp.java 772, 801, 835, 847
session/SessionManager.java 851, 880
msg/MessageService.java 1371, 1399
metrics/FleetMetrics.java 81

The fields these feed are status, phase, state, liveStatus, role — all of them things a caller reads.

Why this is worth a sweep and not a bulk edit

Measured: nothing pins the long-form vocabulary. Across all 144 test .java files on main, the quoted literal appears zero times for every one of these:

token test files
"timed_out_working" 0
"timed_out_queued" 0
"backend_exhausted" 0
"worker_failed" 0
"stale_turn" 0

Controls, so those zeros mean something: "working" matches 5 test files, "queued" 2, "replied" 1 — the search does reach test sources and does match quoted literals there. A control that returned 0 was discarded as useless rather than counted.

So no test goes red if a refactor changes any of these strings. Renaming an enum constant silently changes a REST field, an MCP text, or a metrics label.

The site that matters most, and the trap it sets

metrics/FleetMetrics.java:81:

// One gauge per state so a scrape shows the whole census even when a state is empty —
// an absent series and a zero series read very differently on a dashboard.
for (MemberSession.State state : MemberSession.State.values()) {
    String label = state.name().toLowerCase();
    m.gauge(SESSIONS, () -> countIn(sessions, state), "state", label);
}

The label set is generated by iterating the enum. There is no written list of tokens for a reviewer to find. Add a constant and a new gauge series appears; rename one and a series silently vanishes while a new one appears under a different name. The comment directly above reasons about exactly this harm and pins nothing.

Consequence for this sweep: any before/after inventory must be GENERATED from values(), not typed. A hand-written table cannot see this site, and this is the site whose tokens drift without anyone editing a string.

The classification each site needs

For every one of the 15, give a verdict:

  1. WIRE-FACING — the value reaches a REST body, an MCP tool result, or a metrics label. Renaming the constant is a breaking change to somebody outside this tree.
  2. INTERNAL — the value is only read inside fleetd, or only ever appears in a log line or a human-facing message. Renaming it is safe.
  3. UNCLEAR — say so. Do not guess. An honest "unclear, here is why" is worth more than a confident wrong verdict.

For each WIRE-FACING site, also report: the enum, the exact tokens it can emit today (generated, not typed), and whether any test pins them.

Two hazards that share this surface but are different problems

Keep them apart in the report:

  • Deliberate many-to-one. MessageService.sendOutcomeLabel collapses three constants into timeout on purpose. That is a decision, not drift. Do not report it as a defect.
  • Auto-enumerated one-to-one. FleetMetrics:81 above. One token per constant, but the set is produced rather than written.

Method

  • Re-measure everything. The line numbers above were taken on main at 204da67; locate each site fresh.
  • Pair every zero with a positive control, and report a control that fails as failed — a control returning 0 proves nothing and must be replaced, not counted.
  • Where you claim a value reaches an external surface, name the path: the method, the field, and the endpoint or tool.

Out of scope

  • MessageService.Outcome itself — that is #578, which is blocked on #571. Report its sites for completeness and change nothing.
  • Do not fix anything. Findings only. I file the fixes as their own tickets.

Related: #578 (the one-enum fix and its two corrections), #577 (the per-site sweep this escalation mirrors), #581, #582.

Sweep and classify. **Change no production code.** The output is a ranked list with a verdict per site. Found while correcting #578. The fleet01 lead ran the first grep on a tree 91 commits behind `main` and flagged that the shape was not confined to one enum; every number below is re-measured on `main` at `204da67`. ## The shape `MessageService.Outcome` leaks its **constant spelling** as its **wire token**, because readers call `outcome().name().toLowerCase()` instead of asking for a pinned wire name. #578 fixes that for one enum. The idiom is not confined to one enum. `grep -rn 'name()\s*\.\s*toLowerCase()' --include='*.java'` over non-test sources: **15 hits, 5 files**, covering at least six different enums. | file | lines | |---|---| | `mcp/FleetMcp.java` | 807, 1124, 1156, 1189, 1950, 1979 | | `rest/FleetApp.java` | 772, 801, 835, 847 | | `session/SessionManager.java` | 851, 880 | | `msg/MessageService.java` | 1371, 1399 | | `metrics/FleetMetrics.java` | 81 | The fields these feed are `status`, `phase`, `state`, `liveStatus`, `role` — all of them things a caller reads. ## Why this is worth a sweep and not a bulk edit **Measured: nothing pins the long-form vocabulary.** Across all 144 test `.java` files on `main`, the quoted literal appears zero times for every one of these: | token | test files | |---|---| | `"timed_out_working"` | **0** | | `"timed_out_queued"` | **0** | | `"backend_exhausted"` | **0** | | `"worker_failed"` | **0** | | `"stale_turn"` | **0** | Controls, so those zeros mean something: `"working"` matches 5 test files, `"queued"` 2, `"replied"` 1 — the search does reach test sources and does match quoted literals there. A control that returned 0 was discarded as useless rather than counted. So **no test goes red if a refactor changes any of these strings.** Renaming an enum constant silently changes a REST field, an MCP text, or a metrics label. ## The site that matters most, and the trap it sets `metrics/FleetMetrics.java:81`: ```java // One gauge per state so a scrape shows the whole census even when a state is empty — // an absent series and a zero series read very differently on a dashboard. for (MemberSession.State state : MemberSession.State.values()) { String label = state.name().toLowerCase(); m.gauge(SESSIONS, () -> countIn(sessions, state), "state", label); } ``` The label set is **generated by iterating the enum**. There is no written list of tokens for a reviewer to find. Add a constant and a new gauge series appears; rename one and a series silently vanishes while a new one appears under a different name. The comment directly above reasons about exactly this harm and pins nothing. **Consequence for this sweep: any before/after inventory must be GENERATED from `values()`, not typed.** A hand-written table cannot see this site, and this is the site whose tokens drift without anyone editing a string. ## The classification each site needs For every one of the 15, give a verdict: 1. **WIRE-FACING** — the value reaches a REST body, an MCP tool result, or a metrics label. Renaming the constant is a breaking change to somebody outside this tree. 2. **INTERNAL** — the value is only read inside `fleetd`, or only ever appears in a log line or a human-facing message. Renaming it is safe. 3. **UNCLEAR** — say so. Do not guess. An honest "unclear, here is why" is worth more than a confident wrong verdict. For each WIRE-FACING site, also report: the enum, the exact tokens it can emit today (generated, not typed), and whether any test pins them. ## Two hazards that share this surface but are different problems Keep them apart in the report: - **Deliberate many-to-one.** `MessageService.sendOutcomeLabel` collapses three constants into `timeout` on purpose. That is a decision, not drift. Do not report it as a defect. - **Auto-enumerated one-to-one.** `FleetMetrics:81` above. One token per constant, but the set is produced rather than written. ## Method - **Re-measure everything.** The line numbers above were taken on `main` at `204da67`; locate each site fresh. - Pair every zero with a positive control, and **report a control that fails as failed** — a control returning 0 proves nothing and must be replaced, not counted. - Where you claim a value reaches an external surface, name the path: the method, the field, and the endpoint or tool. ## Out of scope - `MessageService.Outcome` itself — that is #578, which is blocked on #571. Report its sites for completeness and change nothing. - Do not fix anything. Findings only. I file the fixes as their own tickets. Related: #578 (the one-enum fix and its two corrections), #577 (the per-site sweep this escalation mirrors), #581, #582.
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
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#586
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?