Skip to content

Commit 7eee6a5

Browse files
sarg3ntclaude
andcommitted
fix(logs): address Copilot review on capability presence vs availability (#112)
Copilot pointed out that defaultLogSourcesForBox conflated two distinct cases: 1. The agent doesn't surface a gear name at all (older agent that pre-dates the gear). 2. The agent reports the gear as unavailable. Both used IsAvailable() and collapsed into "return empty list", which violates the function comment's fail-open promise: a forward-compatibility gap would silently strip the source list. Use caps.Has() to distinguish presence from availability: - If the agent surfaces neither access-log nor logs by name → fail open to the legacy [haproxy, system] pair (older-agent path). - If the agent surfaces at least one and reports both unavailable → return empty (the only case where the JS shows an empty dropdown). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 08d9d88 commit 7eee6a5

1 file changed

Lines changed: 23 additions & 10 deletions

File tree

gearbox/internal/framework/handler/api_logs.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,17 @@ func (h *Handler) APILogSourcesHandler(w http.ResponseWriter, r *http.Request) {
118118
// - `system` is offered only when the `logs` gear is available, since
119119
// system journal access requires journalctl/tail.
120120
//
121-
// Fail-open: if capabilities aren't reachable (agent down, no API key,
122-
// older agent that pre-dates probing) the legacy pair is returned so
123-
// existing deployments continue to work through transient outages.
121+
// Fail-open posture (matches filterGearsByAgentCapabilities):
122+
// - Capabilities unreachable (agent down, no API key) → legacy pair.
123+
// - Agent didn't surface BOTH gears (older agent that pre-dates
124+
// access-log/logs probing) → legacy pair. We distinguish "the
125+
// agent didn't tell us about this gear" (caps.Has == false) from
126+
// "the agent said this gear is unavailable" (caps.Has == true,
127+
// IsAvailable == false) so a forward-compatibility gap doesn't
128+
// silently strip the source list.
129+
// - Agent surfaced both gears and reported both unavailable → empty
130+
// list. This is the only case the JS renders an empty dropdown
131+
// and skips the immediate fetch — better than 5xx storming.
124132
func (h *Handler) defaultLogSourcesForBox(boxID string) []map[string]string {
125133
legacy := []map[string]string{
126134
{"name": "haproxy", "display_name": "HAProxy"},
@@ -131,21 +139,26 @@ func (h *Handler) defaultLogSourcesForBox(boxID string) []map[string]string {
131139
return legacy
132140
}
133141

134-
hasAccessLog := caps.IsAvailable("access-log")
135-
hasLogs := caps.IsAvailable("logs")
136-
if !hasAccessLog && !hasLogs {
137-
// Agent surfaces a probe table but neither log gear is available.
138-
// Returning the empty list lets the JS render an empty dropdown
139-
// and skip the immediate fetch — better than 5xx storming.
140-
return []map[string]string{}
142+
knowsAccessLog := caps.Has("access-log")
143+
knowsLogs := caps.Has("logs")
144+
if !knowsAccessLog && !knowsLogs {
145+
// Agent doesn't surface either gear name yet — likely an older
146+
// agent that pre-dates this probe. Fail open.
147+
return legacy
141148
}
142149

150+
hasAccessLog := knowsAccessLog && caps.IsAvailable("access-log")
151+
hasLogs := knowsLogs && caps.IsAvailable("logs")
152+
143153
out := make([]map[string]string, 0, 2)
144154
if hasAccessLog || hasLogs {
145155
out = append(out, map[string]string{"name": "haproxy", "display_name": "HAProxy"})
146156
}
147157
if hasLogs {
148158
out = append(out, map[string]string{"name": "system", "display_name": "System"})
149159
}
160+
// Result may be empty here — agent explicitly reported both gears
161+
// unavailable. JS renders an empty dropdown rather than 5xx-storming
162+
// fetches for sources the agent can't serve.
150163
return out
151164
}

0 commit comments

Comments
 (0)