[mabry1985]

Companion to protoLabsAI/homelab-iac#297; refs protoLabsAI/homelab-iac#256.

Repeat reviews and retries currently switch replicas and re-prefill large prompts. The gateway provider sends x-litellm-session-id, derived from a SHA-256 hash of the resolved repository checkout root and feature ID. The key stays stable across retries and re-reviews and distinguishes identical feature IDs in different repositories. Moving a checkout starts a new affinity session.

Validation: pnpm typecheck, pnpm lint, pnpm test (1028 passed, 2 skipped), and pnpm build pass. Tests cover header presence/absence, retry stability, and repository/feature isolation.

Deployment requires gateway session affinity from homelab-iac#297. Duplicate-request suppression and volatile prompt ordering remain separate follow-ups.

Final direct review at c564aea1b6f8: inspected the complete diff, provider request construction, review call site, retry/deadline handling, and the added tests. The session key is stable for a resolved checkout root and feature, uses ASCII-safe SHA-256, stays unchanged across retries, and is only emitted when present. No merge-blocking defects found. This supplements the four clean automated LLM finders; the automated structural scan's 13/14-feature timeout remains recorded in its WARN verdict. CI is green and there are no unresolved review threads.

[protoreview]

QA panel review — PASS

code-review-structural · head 0940316d075a · formal

Low-risk, purely additive PR: threads an optional sessionId through review.ts and conditionally emits it as an x-litellm-session-id header in gateway.ts. All four LLM finders came back clean; the single finding is from the protoPatch structural engine — a pre-existing observability gap where findings exceeding maxFindingsPerFeature are silently dropped without a record. The verifier confirmed the finding (annotated n=1, no refutation). No panel disagreement, no verification gaps. The minor finding is not a blocker for this PR but is worth a follow-up ticket.

Findings

	Severity	Location	Finding	Verified
🟡	minor	src/review.ts:370	Findings silently truncated by maxFindingsPerFeature without observability	confirmed · nearby, not gating

findings JSON (machine-readable)

[
  {
    "file": "src/review.ts",
    "line": 370,
    "severity": "minor",
    "category": "data-loss",
    "claim": "Findings silently truncated by maxFindingsPerFeature without observability",
    "evidence": "At line 372-375, findings are sliced to config.review.maxFindingsPerFeature after mode filtering. Any findings beyond the limit are dropped without being recorded in the droppedFindings array. The droppedFindings array is only populated from providerOutput.droppedFindings (line 370) and validated.droppedFindings (line 393). There is no other mechanism to signal that valid, schema-passing findings were lost to the limit. Fix: after the slice, if any findings were dropped, append a synthetic DroppedFinding entry with layer: 'limit' and a message like 'N additional finding(s) exceeded maxFindingsPerFeature (M)' so the user can see the truncation.",
    "source": "protopatch",
    "verdict": "confirmed",
    "note": "Verifier confirmed the structural gap at head; no refuting evidence found. \u2014 nearby: in code this PR did not change (outside its changed lines and the functions they sit in) \u2014 reported, not gated (#232)",
    "nearby": true
  }
]

---

1 structural finding(s) are nearby notes, not part of the verdict: they sit in code this PR did not change (outside its changed lines and the functions those lines are in). Worth a look; not a request for this PR.

• src/review.ts:370 (minor) — Findings silently truncated by maxFindingsPerFeature without observability

[protoreview]

Promoting the PASS verdict for head 0940316d075a: all checks terminal-green, zero unresolved review threads. (approve-on-green)

Open findings carried by this approval — non-blocking, but they did not go away:

• minor src/review.ts:370 — Findings silently truncated by maxFindingsPerFeature without observability

Approving a WARN does not resolve its findings (issue #22).

[protoreview]

QA panel review — PASS

code-review-structural · head 0d775190d27b · formal

Low-risk, purely additive change (new session-affinity header on the gateway provider). The one actionable item is the missing CHANGELOG entry for the new x-litellm-session-id behavior. Verification refuted the protopatch test-coverage finding on state.ts (a dedicated state.test.ts exists at head and exercises isStaleLocalFeatureLock directly). No panel disagreement.
VERIFY_GAP: unverified=2
The verifier annotated only 1 of 3 synthesized findings; the CHANGELOG and gateway.ts findings carry no verdict and the overall verdict rests on unverified findings.

Findings

	Severity	Location	Finding	Verified
🟡	minor	CHANGELOG.md	The 0.8.2 changelog section is missing a provider(gateway) entry for the new x-litellm-session-id session-affinity behavior, breaking the repo's convention…
⚪	nit	src/providers/gateway.ts:247	Undici Agent instances cached in gatewayAgents are never closed, holding open file descriptors for the process lifetime.	⚠️ uncertain

findings JSON (machine-readable)

[
  {
    "file": "CHANGELOG.md",
    "line": 0,
    "severity": "minor",
    "category": "conventions",
    "claim": "The 0.8.2 changelog section is missing a **provider(gateway)** entry for the new x-litellm-session-id session-affinity behavior, breaking the repo's convention of documenting every notable provider change.",
    "evidence": "The 0.8.2 section currently has only two **state** entries (stale-lock reclaim, empty-lock reclaim). The PR adds a new HTTP header (x-litellm-session-id) sent by the gateway provider on every review call, keyed on a SHA-256 of the resolved checkout root and feature ID \u2014 a notable behavioral change that the changelog's established pattern (e.g. 0.6.2's **provider(gateway)** entries for timeout, retry, and max_tokens) would document."
  },
  {
    "file": "src/providers/gateway.ts",
    "line": 247,
    "severity": "nit",
    "category": "performance",
    "claim": "Undici Agent instances cached in gatewayAgents are never closed, holding open file descriptors for the process lifetime.",
    "evidence": "The module-level `gatewayAgents` Map (line 247) caches undici `Agent` instances created via `new Agent({ headersTimeout, bodyTimeout })` (line 252). Each undici Agent maintains an internal connection pool and holds open file descriptors for its sockets. These agents are never closed (no call to `agent.close()` or `agent.destroy()`).",
    "source": "protopatch",
    "verdict": "uncertain",
    "ungrounded": true,
    "note": "evidence not found at the reviewed head \u2014 downgraded to uncertain, cannot gate a merge (issue #25)"
  }
]

---

1 finding(s) excluded from the verdict by in-diff confinement (file not among this PR's changed paths):

• CHANGELOG.md (minor) — The 0.8.2 changelog section is missing a provider(gateway) entry for the new x-litellm-session-id session-affinity behavior, breaking the repo's convention

---

1 finding(s) downgraded to uncertain: the code they quote as evidence does not appear in the file at the reviewed head, nor in this PR's patch for it. A finding that cannot be grounded does not gate a merge (issue openclaw#25) — it still stands for a human to judge.

• src/providers/gateway.ts (nit) — quoted evidence not found at this head: new Agent({ headersTimeout, bodyTimeout })

---

1 structural finding(s) are nearby notes, not part of the verdict: they sit in code this PR did not change (outside its changed lines and the functions those lines are in). Worth a look; not a request for this PR.

• src/providers/gateway.ts:247 (nit) — Undici Agent instances cached in gatewayAgents are never closed, holding open file descriptors for the process lifetime.

[protoreview]

QA panel review — WARN

code-review-structural · head c564aea1b6f8 · formal

⚠️ Coverage incomplete — this is not a clean pass. 1 of 5 review lane(s) did not complete a full pass this round: find_structural (structural pass cut short: 13 of 14 features reviewed — timed out after 1500s (budget exceeded; findings from the finished features kept) (after 1 attempt)). Findings from the lanes that ran stand, but a defect only the missing lanes would catch may be missed, so a PASS is capped at WARN. Where the brief below implies full coverage, this line supersedes it. The next push re-runs the full panel.

No findings were raised, so there was nothing to verify. The panel's four LLM finders all returned clean passes on this small, purely additive change (new optional sessionId field, SHA-256 computation, conditional header in the gateway provider). The two standing items from round 2 (missing CHANGELOG entry, unclosed Undici Agents) are minor/nit and remain open but do not block. No coverage gaps: the changed surface is small and all lanes read it.

No findings from the lanes that ran — coverage was incomplete (see above), so this is not a clean review.

findings JSON (machine-readable)

[]

---

Addressed since the last round (1 minor/nit): CHANGELOG.md:0 — no longer raised, and the PR changed the code around them since they were raised.