Features: #339 and #341 — two heuristics that had lasting side effects

Dai Ha
2026-09-04 16:15:31 +07:00
parent 5a59747349
commit dbe3bebf35
+60
@@ -4221,3 +4221,63 @@ is `TIMED_OUT_WORKING` — not `TIMED_OUT_QUEUED`. Claiming "queued" while the t
the original bug with a smaller window. Note that `MessageService` reading that return value is
**not** currently pinned by a test: `InjectorTest` covers `cancel` itself, and mutating the caller's
use of it left all 1357 tests green. Open as fleetd #345.
---
## A member's prose about an error no longer records a credential outage
**What it does.** `CompletionResolver` used to notify `backendErrorSink` on any line of a member's
pane matching the backend-error pattern. The built-in fallback is `(?i)\bAPI Error\s*:` —
case-insensitive and unanchored — so a member that ended a turn without `fleet_reply` while merely
writing *about* an error matched it. The sink call now needs the pattern at the **start** of its
matched line, ignoring leading terminal chrome. Failing the send is unchanged: any match still fails
it and still carries the whole pane tail.
**On.** Always on (fleetd #339). The too-fast crash path still notifies on any match, because there
the crash signature is corroboration.
**Why it exists.** The sink is not cosmetic. Two backend errors on one credential within 60 seconds
put it into cooling-off, and a `fleet_spawn` naming a cooling profile is refused before it reaches
the backend. Profiles share credentials here, so a member's own prose could block spawns on a
profile that never had a problem. The code's comment already admitted the false match and argued
that *failing the send* is still right — a sound argument that covers `resolveFailure` and says
nothing about the sink call sitting in the same block.
**One thing to know for maintenance.** The first version of this check used a bare `lookingAt()`,
and that rejected a **genuine** error line rendered as `│ 503 Service Unavailable: ...` — the send
failed and the outage went unrecorded. That is the worse direction: an unrecorded outage leaves the
fleet spawning into a dead credential. It was caught on merge by a probe, not by the suite, because
every existing test put the error line with no chrome in front of it. The check now skips a leading
run of non-letter, non-digit characters, and `aRealErrorBehindTerminalChromeStillNotifiesTheSink`
pins it. **If you touch this check, test it against a chromed line, not only a clean one.**
It is still a heuristic: prose that *begins* with `API Error:` will still notify the sink. The same
free-text shape drives `ExhaustionSink`, where a quarantine runs 1800s against this cooldown's fixed
60s — open as fleetd #348, and unproven.
---
## Every distinct unprotected credential name gets its own warning
**What it does.** The `memberCredentials` gap WARN — the one naming environment variables that every
member pane inherits unblocked — was guarded by a single `AtomicBoolean` shared by **two** branches
that report **different** variable names. It is now a `Set<String>` of names already warned about, so
the guard is per name rather than per launcher.
**On.** Always on (fleetd #341).
**Why it exists.** `memberCredentials` is re-read on every spawn, so an operator can change the
policy with a reload and no restart. Spawn 1 under `deny-by-default` warned about one variable and
tripped the flag; after a reload, spawn 2's genuinely unprotected *different* variable was never
reported. The operator fixes the one name they were shown and reasonably believes the gap is closed.
The whole point of naming variables in these lines is so they can be acted on.
The same class already had the right reasoning written down: #192 split this flag from the
allow-list INFO guard precisely so a harmless report could not suppress a real one. That reasoning
was applied to INFO-versus-WARN and never to WARN-versus-WARN.
**One thing to know for maintenance.** The set has two duties and only one was pinned at first.
Measured on merge: replacing the `.filter(...::add)` with one that logs every name on every spawn
left all 1358 tests green — the noise control was correct and nothing held it there. Two tests now
cover both duties, including the **reverse** policy order (allow-list first, then deny-by-default),
because a guard fixed in one direction is not automatically fixed in the other.