aitools: categorize install errors

[rclarey]

Stacked on #6481

Changes

Categorize aitools install errors, and emit those in telemetry and JSON output

Why

To better understand why installations failed

Tests

Added unit tests

[github-actions]

Approval status: pending

/cmd/aitools/ - needs approval

6 files changed
Suggested: @simonfaltum
Also eligible: @lennartkats-db, @parthban-db, @fjakobs, @Shridhad, @atilafassina, @keugenek, @igrekun, @pkosiec, @MarioCadenas, @pffigueiredo, @ditadi, @calvarjorge, @renaudhartert-db, @hectorcast-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/libs/aitools/ - needs approval

Files: libs/aitools/installer/errors.go, libs/aitools/installer/installer.go
Suggested: @simonfaltum
Also eligible: @lennartkats-db, @parthban-db, @fjakobs, @Shridhad, @atilafassina, @keugenek, @igrekun, @pkosiec, @MarioCadenas, @pffigueiredo, @ditadi, @calvarjorge, @renaudhartert-db, @hectorcast-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/libs/telemetry/ - needs approval

Files: libs/telemetry/protos/aitools_install.go
Suggested: @simonfaltum
Also eligible: @parthban-db, @renaudhartert-db, @hectorcast-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

_{Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db, @rugpanov) can approve all areas.
See OWNERS for ownership rules.}

[rugpanov]

Review — multi-reviewer pass

Reviewed the incremental diff (against the stacked parent aitools-install-output-json) with several independent reviewers and verified each finding against the code. Design and correctness are sound overall, and CI (including integration tests) is green. One blocking wire-format bug, plus a few low-risk cleanups.

🔴 Blocking — per-agent category serializes under the wrong JSON key

libs/telemetry/protos/aitools_install.go:53

type AitoolsAgentResult struct {
	Agent         AitoolsAgentType     `json:"agent"`
	ErrorCategory AitoolsErrorCategory `json:"errorCategory"`   // should be "error_category"
}

This is the only camelCase JSON tag in the whole libs/telemetry/protos package (111 other tags are snake_case), and it disagrees with its own sibling field on the parent event (error_category, line 71) and the existing precedents in ssh_tunnel.go / bundle_config_remote_sync.go. The marshaled payload becomes agent_results:[{"agent":"CODEX","errorCategory":...}], so the per-agent category won't map to the lumberjack proto's error_category field and is likely dropped on ingestion — which is exactly the per-agent signal this PR adds. Fix: json:"error_category".

🟡 Nice to have

• A sibling specific-skill failure is left uncategorized — libs/aitools/installer/installer.go:487. The "experimental skill; use --experimental" specific-failure still returns a plain fmt.Errorf, so it classifies as UNCATEGORIZED, while the two adjacent failures in the same isSpecific branch (not-found :475, version-incompatible :495) were converted to *SkillError. Consider a category (e.g. EXPERIMENTAL_SKILL) or making it a SkillError for consistency.

• Defensive nil check — cmd/aitools/telemetry.go:58. o.agent == nil in agentResultsField guards against a state that can't occur (every outcome is built from a non-nil plan agent). Per the repo convention on unjustified nil checks, consider removing it (and the synthetic nil test case) or adding a comment on why the invariant might break. (It does mirror the pre-existing pattern in agentsField, so it's at least locally consistent.)

• No test asserts the telemetry wire format — nothing marshals AitoolsInstallEvent/AitoolsAgentResult to JSON and checks the keys. A serialization assertion would have caught the blocking finding above.

• UNSUPPORTED_SCOPE on exit-0 skips — cmd/aitools/install.go:391,400,555. An ordinary scope-incompatible skip (status skipped, exit 0) still lands in agent_results with UNSUPPORTED_SCOPE. Looks intentional per the doc comment, but conflating expected skips with error rows may inflate error analytics — worth confirming the intended semantics.

⚪ Nits

• cmd/aitools/install.go:606 — if o.errorCategory != "" { … } is redundant given the omitempty tag, and inconsistent with the unconditional top-level set just below it.
• cmd/aitools/install.go:64 — the field skipError holds a category, not an error; mildly misleading next to errorCategory.
• libs/aitools/installer/errors.go:25 — SkillError.Error() produces a trailing space when Detail is empty (only reachable from a unit test today).