fix(context-fetcher): never report an all-clear from a truncated or failed snapshot

[uuzzrm]

What & why

Closes #140

The snapshot health flags (snapshot_has_issues / snapshot_has_warnings) were derived from kubectl output that had already been hard-capped at 8 000 chars — no marker, and no awareness that the cap had been hit. On a cluster with enough pods to overflow the cap, an unhealthy pod past the cut was invisible to both the scan and the LLM, so the coordinator could report the cluster as clean when it wasn't. A kubectl failure did the same for the pod listing (zero pods parsed → no issues).

• _run_kubectl_snapshot now returns a small SnapshotOutput (text + truncated / unavailable flags) instead of a bare string.
• _scan_snapshot treats truncated/unavailable listings conservatively — no all-clear from partial or failed data.
• The snapshot text the LLM sees carries an explicit note when a listing was cut short or never arrived (mirrors the existing marker in run_kubectl).
• coordinator._verify_resolution inherits the same conservatism, so a fix is no longer marked "resolved" from a partial listing.

Type of change

• Bug fix
• New feature
• Docs
• Refactor / chore
• Performance
• Tests

Scope

• Version directory touched: v4/
• This change is scoped to a version whose contributions are open (v4/, or docs/typos in older versions).

Checklist

• New behavior has both a happy-path and an error-path test (tests/test_context_fetcher.py)
• No mutating ops touched — the HITL/RBAC invariants are unaffected
• Secret values are never logged or returned (n/a here)
• uv run pytest passes locally — see gate output below
• uv run ruff check passes locally
• uv run mypy passes locally — 171 source files, 0 errors
• Docs updated if behavior/CLI/flags changed — n/a, no user-facing behavior change

Notes for reviewers

Gates run (all from v4/):

• uv run ruff check packages/kubeintellect-server/app/ packages/ki-protocol/ — clean
• uv run mypy packages/kubeintellect-server/app packages/ki-protocol packages/kube-q/kube_q — 0 errors (171 source files)
• uv run python -m pytest tests/ -q — 1027 passed under PYTHONUTF8=1

One environment note: on my Windows box (GBK locale, code page 936) the default-locale run shows 46 failures in tests/test_playbooks.py. That is the pre-existing playbook loader encoding bug tracked in #136 — the loader uses path.read_text() without an explicit encoding, so only 2 of the playbooks load under a non-UTF-8 locale. It is unrelated to this change and green under a UTF-8 locale, which is what CI runs.

Heads-up: I used AI assistance to help draft and verify this change — happy to walk through any part of it in review.

[MSKazemi]

Thank you for this — the bug is real, the diagnosis is right, and I am not merging it yet. Two reasons, and the second one is the useful one.

1. It does not yet catch the failure it is aimed at

`_run_kubectl_snapshot` never checks `proc.returncode`. `subprocess.run` does not raise on a non-zero exit, so `unavailable` only fires on the exception path — missing binary, timeout, bad kubeconfig path. The common failures do not go through there: `Unauthorized`, connection refused, an expired token. Those exit non-zero, `proc.stdout or proc.stderr` picks up the stderr text, and it gets parsed as a pod table.

I ran your branch against exactly that case — kubectl exiting 1 with error: You must be logged in to the server (Unauthorized):

unavailable flag : False
truncated  flag  : False
has_issues       : False | pod_count: 0
=> ALL-CLEAR FROM A FAILED READ

Which is the thing your title promises to prevent. Your truncation half works correctly — that one I verified fires.

There is a second variant worth knowing about: a real connection failure prints as three lines, and the two E0820 … lines have enough whitespace-separated columns to be counted as pods, so you get pod_count=2 invented out of an error message.

2. A maintainer rework of the same function is in flight

This is on me, not you — it was not visible from outside, and you had no way to know. I have an unfinished local rework of _scan_snapshot covering this same bug, and it takes the opposite design decision on the central question:

• Yours: has_issues = pods_truncated or pods_unavailable — a failed read reports issues found.
• Mine: if not pods_ok: return False, False, 0 — a failed read reports nothing known, and callers distinguish "unknown" from "clean" via the flag they passed in.

Both are defensible. Yours fails loud, which is the safer default in isolation; mine keeps has_issues meaning strictly "I saw a problem" and moves the unknown-state signal to a separate channel, because has_issues also drives user-facing wording and I did not want an unreachable cluster described as an unhealthy one. I have not decided yet, and I am not going to pretend the decision is made just to close this out.

What happens next

I am keeping this open, not closing it. The concrete blocker is one design call that is mine to make, and I will make it and come back here — not leave it drifting.

If you would like to keep it, the single change that would make this fix the real bug is checking proc.returncode in _run_kubectl_snapshot and setting unavailable from it, plus a test using a fake process with returncode=1 and error text on stderr. Say the word and it is yours. If you would rather not spend more time on a PR whose seam I might rewrite underneath you, that is completely fair — say so and I will take it from here with your name on it either way. Your call, and neither answer is the wrong one.

One unrelated nit for whichever version lands: context_fetcher.py ends without a trailing newline in this branch.

Separately — #133 and #138 are both merged, with replies on each. And if you want #96 back, it is unclaimed and unassigned again.

— Mohsen