From dbe3bebf3559875eaa4b3e99f7475b3d67fff2ea Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 16:15:31 +0700 Subject: [PATCH] =?UTF-8?q?Features:=20#339=20and=20#341=20=E2=80=94=20two?= =?UTF-8?q?=20heuristics=20that=20had=20lasting=20side=20effects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 11-Features.md | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/11-Features.md b/11-Features.md index e889550..172c899 100644 --- a/11-Features.md +++ b/11-Features.md @@ -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` 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.