Lint: ban silent .get(literal) and 'k in t and t[k]' patterns at non-dynamic call sites

[FidoCanCode]

Why

The .get("key") pattern (and its sibling if "k" in d and d["k"]:) silently returns None when the key is missing, which has cost us real bugs:

• Bug: reply-promise marker recovery doesn't verify comment author — anyone can suppress fido replies #962 — recover_from_bodies reply-promise marker recovery used .get instead of indexing required fields, so missing markers silently passed through.
• Bug: switch_model hangs when called with _in_turn=True after preempt-cancelled drain #975 — _send_control_set_model checked obj.get("request_id") at the wrong nesting level. request_id is at obj["response"]["request_id"]. The .get returned None, the predicate never matched, every model switch hung. The bug ran undetected for the entire lifetime of Use control_request set_model for mid-session model switch instead of session restart (closes #852) #971.

The CLAUDE.md "Fail-fast / fail-closed" section already says no .get() defaults for required keys — index directly with payload["action"] so a missing key raises KeyError at the boundary. This issue makes that rule enforceable.

What to ban

Two AST patterns, both flagged at lint time:

1. Call(func=Attribute(attr="get"), args=[Constant(str)]) — .get("k") with a single literal key argument, no default.

2. BoolOp(And, [Compare(In, Constant(str), x), Subscript(x, Constant(str))]) — if "k" in t and t["k"]: and the not in mirror.

Exemptions

• Two args: d.get("k", default) is fine — explicit default, no silent-None.
• Non-literal key: d.get(k) where k is a Name (truly dynamic lookup) is fine.
• Per-line opt-out: # allow-silent-get: <reason> token-level comment on the same line. Reason is required so reviewers can sanity-check.

Implementation

scripts/check_no_silent_get.py, ~50 lines:

• AST-walk every .py file under src/, tests/.
• Collect violations of patterns above.
• Cross-reference with tokenize to find allow-silent-get comments and skip those lines.
• Exit 1 with path:line:col: silent-get: ... for each violation.

Wire into ./fido ci's lint stage so a git commit blocks if violations land.

Companion: a require() helper at boundaries

Boundary code (webhook payloads, claude responses, tasks.json) should use a positive helper instead of [] directly:

def require(d: Mapping[str, Any], key: str, *, ctx: str = "") -> Any:
    if key not in d:
        raise KeyError(f"{ctx or 'payload'} missing required field {key!r}")
    return d[key]

That gives a useful traceback message instead of a bare KeyError: 'request_id'. Add as a small helper in fido.types or similar; encourage migration in subsequent PRs.

Migration plan

1. Land the linter in warn-only mode (exit 0, but print violations).
2. Sweep existing .get(literal) callsites — convert to [] if required, .get(literal, default) if truly optional.
3. Flip linter to error mode in CI.
4. Open follow-ups for require() at boundary sites where messages would help.

Out of scope

• Type-checker-based enforcement (TypedDict + total=True). Worth doing eventually but a bigger lift.
• Dynamic-key call sites (the Name exemption). Those are harder to reason about and rarely the bug shape we keep hitting.

Related

• Bug: reply-promise marker recovery doesn't verify comment author — anyone can suppress fido replies #962 — silent .get in marker recovery
• Bug: switch_model hangs when called with _in_turn=True after preempt-cancelled drain #975 — silent .get in control_response request_id check
• CLAUDE.md "Fail-fast / fail-closed" — already documents the rule; this makes it enforceable