feat(coderabbit-config): flag unknown keys inside schema-open objects (BE-7786) (#188)

* feat(coderabbit-config): flag unknown keys inside schema-open objects (BE-7786)

The unknown-key check only reported keys jsonschema complained about, which
means only the five objects upstream's schema closes with an explicit
`additionalProperties: false`. 103 more objects declare `properties` and say
nothing about the rest — `reviews`, `chat`, `knowledge_base`,
`code_generation` and every individual tool config among them — so a typo
inside one (`reviews.profil`, `golangci-lint.enabld`) produced no finding in
either mode, even though CodeRabbit strips those keys exactly like a
root-level one.

Adds a conservative schema-walk that closes the gap. A node counts as closed
BY OMISSION only when it declares a `properties` block and none of the twelve
keywords that let an object accept a name it does not list; the walk descends
only through matched properties and `items`, never into a combinator branch,
an `additionalProperties` schema, or the value under a key it just reported,
and it stays silent where the document and the schema disagree about shape
(that is a type error, and jsonschema already says so). The two halves are
disjoint by construction — one needs the keyword present, the other needs it
absent — so nothing is reported twice.

Findings share one builder with the jsonschema path, so severity, wording,
path, line resolution and the "did you mean" hint are identical by
construction; non-string YAML keys are coerced for display, which also stops
an integer key rendering as a list index. The walk's findings count toward
MAX_FINDINGS with the jsonschema ones, and the walk is itself bounded by a
key-inspection budget (the document is PR-controlled and aliased YAML is
walkable as a much larger tree) that reports a partial scan rather than
letting it read as a clean one.

* fix(coderabbit-config): cap findings after fan-out, name keys by source spelling, bound list traversal (BE-7786)

Addresses the cursor-review panel findings on #188.

* The `islice` on `iter_errors` bounds the ERROR count, not the finding
  count: `additionalProperties: false` reports a whole mapping in ONE
  error and `_extra_keys` splits it into one finding per key, so 150
  unknown root keys emitted 150 annotations into a public run log with
  `truncated` still False — and drove `remaining` to 0, silently
  disabling the by-omission walk on top. Re-apply the cap to the
  findings themselves before anything is derived from their number.

* A walk that gave up early was only a note, and a note does not reach
  the exit code — so `strict_unknown_keys: true` exited 0 over keys
  nothing ever looked at. It is now a finding at the unknown-key
  severity: warn-only mode is unchanged, strict mode fails.

* List elements recursed without charging the walk budget, leaving array
  traversal outside the bound its own comment claimed. Charge per node
  visit as well as per key, so an alias-amplified nested list is covered.

* `bounded` is set where the walk actually bails rather than derived from
  the leftover budget: an exact landing on MAX_WALK_KEYS was a full walk
  reported as partial, and a MAX_WALK_DEPTH cutoff was a partial walk
  reported as clean.

* `str()` on a loaded key is not its source spelling — `true:`/`on:`/`yes:`
  all annotated `reviews.True`, `0x10:` `reviews.16`, `null:` `reviews.None`,
  each naming a key that appears nowhere in the file and each losing its
  line number, because the lookup then compared "True" against the node's
  "true". Resolve key nodes tag-aware and read both name and line off the
  matched node.

* The "did you mean" hint was written for the specific-name case; nested
  unknown keys are dominated by generic ones (`enabled` sits at ~72 places
  in the schema), so `reviews.tools.enabled` was answered with three wrong
  `issue_enrichment.*` homes. Prefer candidates under the offending key's
  own parent, and stay silent past three.

* Docstring/README: the two unknown-key halves being duplicate-free is a
  fact about THIS vendored schema, not a property of the two conditions —
  now stated as such, with `OpennessTest` pinning the premises ($ref
  absent, no properties node carrying a combinator).

* Document the by-omission staleness window: a property upstream ADDS
  reads as unknown until the schema refresh and each caller's pin land,
  which under strict mode is a hard failure on a key the repo was right
  to add.

* docs(coderabbit-config): document the partial-scan severity row; correct a stale test comment (BE-7786)

The caller doc's severity table did not carry the new behaviour: an
unknown-key scan that stops early warns by default and fails under
`strict_unknown_keys`, which is the same "I could not check must never
look like a pass" rule the exit-2 row above it states.

The comment on the non-string-key test still named `_key_line`, which the
previous commit replaced with `_key_node`, and still described `str()`
coercion as the fix when it is now only the fallback.

Author: mattmillerai

.github/coderabbit-config/README.md

@@ -30,22 +30,56 @@ a schema library calls "invalid":
 | YAML parse error | error | CodeRabbit cannot read the file at all. |
 | `maxLength` violation | error | File-rejecting. The schema carries **14** per-field caps, 50 → 20,000 chars. |
 | type / enum error | error | File-rejecting. |
-| unknown / additional property | **warning** | CodeRabbit *strips* an unrecognized key rather than rejecting the file. Only where the schema closes the object — see below. |
+| unknown / additional property | **warning** | CodeRabbit *strips* an unrecognized key rather than rejecting the file. Reported wherever the schema object accepts only the names it lists — see below. |
 | config path that is not a regular file, or > 512 KiB | exit 2 | "I could not check" must never look like a pass. A path resolving outside the repo root (symlinks resolved) is refused the same way. |
 
 `strict_unknown_keys: true` escalates the last row to an error for a repo that has
 cleaned up and wants to stay clean.
 
-**How far the unknown-key row reaches.** It fires where the schema CLOSES an
-object — the document root, `knowledge_base.mcp`, and the two tool objects that
-declare `additionalProperties: false`. Most nested objects (`reviews`, `chat`,
-`knowledge_base`, `code_generation`, the individual tool configs) are open in
-upstream's schema, so a typo *inside* one — `reviews.profil`, or
-`golangci-lint.enabld` — produces no finding, with or without
-`strict_unknown_keys`. That is a real gap and not a claim this checker makes:
-what it catches is a key in the wrong PLACE, which is the failure that actually
-recurred here. Widening it means cross-checking parsed keys against the schema
-index rather than waiting for jsonschema to complain.
+**How far the unknown-key row reaches.** It fires wherever a schema object accepts
+only the property names it lists — which upstream expresses two different ways,
+and both are checked:
+
+* **explicitly**, with `additionalProperties: false`. Five objects: the document
+  root, `knowledge_base.mcp`, `knowledge_base.linked_repositories[]`, and the
+  `htmlhint` / `stylelint` tool configs. jsonschema reports these.
+* **by omission** — a `properties` block and no statement at all about anything
+  else. **103** objects, including `reviews`, `chat`, `knowledge_base`,
+  `code_generation` and every individual tool config. jsonschema has no keyword to
+  fire on here, so a schema-walk in `_walk_unknown_keys` reports them. This is
+  what catches a typo *inside* an object — `reviews.profil`,
+  `golangci-lint.enabld` — and a key upstream has since REMOVED, such as
+  `reviews.tools.github-checks.timeout_ms`.
+
+The rule the walk applies is conservative: a node is closed by omission only when
+it declares a `properties` block and NONE of `additionalProperties`,
+`unevaluatedProperties`, `patternProperties`, `anyOf`, `oneOf`, `allOf`, `$ref`,
+`$dynamicRef`, `if`, `dependentSchemas`, `propertyNames` or `not`. Each of those
+is a way the object can legitimately accept a name its `properties` does not list,
+so reading any of them as "closed" would invent a finding — `additionalProperties`
+given a SCHEMA rather than `false` (`reviews.mutually_exclusive_groups`, where the
+group names are the user's to choose) is the live example. The walk descends only
+through matched properties and through `items`, never into a combinator branch or
+under a key it just reported, and where the document and the schema disagree about
+shape it stays silent, because that is a type error and jsonschema owns it.
+
+The two halves are disjoint at a single node — one needs the keyword present, the
+other needs it absent — and against **this** vendored schema that is enough to
+make the combined output duplicate-free. It is a fact about the schema rather than
+about the two conditions: jsonschema evaluates a document path against every
+applicable subschema, so a node the walk judges closed whose `$ref` target carried
+`additionalProperties: false` would be reported by both halves. `OpennessTest`
+pins the two premises that rule it out today — the schema carries no `$ref`, and
+no node declaring `properties` also carries a combinator — so a schema refresh
+that breaks either fails there rather than double-annotating a real PR.
+
+**The by-omission half is only as current as the vendored schema.** It reads those
+103 objects as closed against the copy committed here, so a property upstream ADDS
+is an unknown key until `refresh-coderabbit-schema.yml` lands the bump and each
+caller's pin moves. Warn-only mode absorbs that as a warning; a caller running
+`strict_unknown_keys: true` gets a hard failure on a key it was right to add. Same
+freshness, opposite sign, as catching a key upstream removed — see the caller doc's
+rollout section.
 
 A stripped key is not a cosmetic problem. The recurring instance is a `tools:`
 block written at the document root instead of under `reviews:` — the schema root