feat(feature-flags): generalize driver-side cache

[cathleeny]

Description

Generalize the existing feature-flag cache for driver-owned flags on the Thrift path and before kernel/session initialization, once authenticated transport is available.

• Share raw flag values by workspace ID, with normalized host as a fallback; keep credentials and HTTP clients caller-owned.
• Add typed Boolean, int32, int64, double, string, and string-list getters. Integer validation uses standard-library fixed-width types.
• Keep the blocking initial fetch and background refresh, retain stale values on refresh failures, and avoid queuing duplicate refreshes.

How is this tested?

• Full non-real-kernel unit suite: 1,032 passed, 5 skipped, 1 deselected; 351 subtests passed.
• Focused telemetry/feature-flag suite: 58 passed.
• Black and mypy checks passed.
• Covers all six types, defaults/range validation, workspace sharing, and caller-owned refresh/authentication.

[peco-review-bot]

Verdict: 1 Low

Solid, well-tested generalization of the feature-flag cache — the typed getters correctly exclude bool-as-int, enforce fixed-width ranges, and reject NaN/inf, and the shared _CacheState + RLock refresh coordination is sound. One low-severity lifecycle note: get_instance is now created eagerly per non-kernel connection while remove_instance has no production caller, so the refresher executor/cache is never cleaned up.

[peco-review-bot]

Verdict: 1 Low

Looks solid overall — the typed getters are correct and well-covered (bool/int range via ctypes, double finiteness, strict list typing), and the workspace-keyed sharing with host fallback is sensible and tested. One low-severity lifecycle concern: remove_instance (which shuts down the shared refresh executor and evicts cache state) has no production caller, so the executor and per-workspace cache leak for the process lifetime. Minor nit below.

Nit (no anchor needed): the typed getters' default_value parameters (get_int32/get_int64/get_double/get_string/get_string_list, and _get_int) lack type hints, unlike the annotated return types; adding them would match the repo's type-hint convention (CONTRIBUTING.md).

[peco-review-bot]

Verdict: 1 Low

Looks good — a clean, well-tested generalization of the feature-flag cache (shared _CacheState per workspace, typed JSON getters, refresh dedup, pre-session reader). One low-severity note: get_bool is a stricter parser than the str(...).lower() == "true" telemetry gate it replaces, so the telemetry flag now depends on the server emitting a bare lowercase JSON boolean. Nit (summary-only): the typed getters annotate return types but leave default_value/integer_type/name params untyped in _get_int, get_int32/64, get_double, etc. — CONTRIBUTING.md asks for type hints; adding default_value: Optional[int] = None style annotations would keep them consistent with get_bool/get_string.

[jay-xiao446]

lg in general. left a comment

[peco-review-bot]

Verdict: 2 Low

Solid refactor that generalizes the feature-flag cache to a per-workspace _CacheState with caller-owned transport and typed getters; the type(value) is int/bool checks correctly exclude bools, and stale-on-refresh-failure semantics are preserved. Two low concerns noted inline: a latent stale-executor RuntimeError path after remove_instance shuts the pool down, and the first-fetch network call now running under the shared per-workspace lock. Nits (summary-only): _refresh_flags swallows all exceptions with no logging (the module imports no logger), so failed fetches are completely silent; and the get_double integer-OverflowError branch (float(value) on an out-of-float-range int) appears untested — the 1e400 case exercises the JSON-inf path instead.