Review agent should treat defensive-code removal with library-delegation claims as unverifiable

[fullsend-ai-retro]

What happened

On PR #5562, the author removed three custom validation/guard functions (validateEndpoint, isExporterNone, protocolFromEnv) and delegated their responsibilities to the OpenTelemetry Go SDK. The review bot flagged the validateEndpoint removal as MEDIUM severity (review run 30246434007), noting it "may cause less descriptive errors," and suggested verifying the SDK handles malformed endpoints. The author responded that the SDK handles it natively. The bot did not re-flag this finding in subsequent rounds (runs 30339832082 through 30882140217).

The human reviewer (waynesun09) then read the vendored otlptracehttp v1.44 source code and proved three author claims wrong:

1. The SDK does NOT read OTEL_TRACES_EXPORTER — removing isExporterNone() broke the documented kill switch
2. The SDK does NOT validate malformed endpoints — url.Parse succeeds on almost any string, and the SDK silently falls back to localhost:4318
3. The SDK's MaxElapsedTime does NOT bound shutdown hangs — context.WithTimeout was needed

All three were escalated to HIGH severity. The endpoint validation function was restored with improved logic. The PR was reclassified from fix(telemetry) to fix(telemetry)! (breaking change).

What could go better

The review agent correctly identified the risk in removing guard code but treated it as MEDIUM severity and suggested verification it could not perform. When the author claimed the SDK handles the removed functionality, the finding was effectively dropped — the bot did not re-flag it in 8 subsequent review rounds.

The core issue is that the review agent cannot verify claims about external library behavior (it cannot read vendored dependency source code or run tests). When it encounters a pattern where defensive code is removed and the justification is "the upstream library handles it," it should recognize this as an unverifiable claim and escalate rather than accept.

This is distinct from #4379 (which addresses the agent hedging its OWN uncertainty about library internals). Here, the gap is about the agent's response to AUTHOR claims that invoke library behavior — the agent should not downgrade or drop a finding because the author asserts the library provides equivalent functionality.

Confidence: HIGH that this pattern was consequential on this PR. MEDIUM confidence it recurs frequently enough to warrant a policy rule — but the pattern (removing guards in favor of SDK defaults) is common in SDK-consumer codebases, especially in the OpenTelemetry ecosystem.

Proposed change

Add a review policy rule or prompt instruction in policies/review.yaml or agents/review.md in the fullsend-ai/agents repo:

When a PR diff removes validation, guard, or defensive code (input validation functions, error-checking guards, boundary checks, exporter kill-switches) and the PR description or commit message justifies the removal by claiming an external library or SDK provides equivalent functionality:

1. Flag the removal as HIGH severity rather than MEDIUM
2. Mark the finding as "unverifiable by automated review" — the review agent cannot read vendored dependency source to confirm the claim
3. Explicitly recommend human verification against the specific library version in go.mod / package.json / equivalent
4. Do NOT drop or downgrade the finding in subsequent review rounds based on author responses that repeat the SDK-delegation claim without providing verifiable evidence (e.g., a link to specific SDK source code or documentation)

The heuristic to detect this pattern: the diff removes a function or code block that performs validation/checking, and the surrounding context (commit message, PR body, inline comments) contains phrases like "left to the SDK," "the library handles," "delegated to upstream," or "SDK provides natively."

Validation criteria

On the next 5 PRs across fullsend-enrolled repos where the diff removes validation or guard code with SDK/library-delegation justification:

1. The review agent should flag the removal as HIGH severity (not MEDIUM or LOW)
2. The review agent should include language indicating it cannot verify the library-delegation claim
3. The review agent should recommend human verification against the specific library version
4. The finding should persist across subsequent review rounds even if the author repeats the delegation claim

Baseline from this PR: the bot flagged the removal as MEDIUM, accepted the author's rebuttal, and did not re-flag it — the human had to catch the issue independently.

---

Generated by retro agent from fullsend-ai/fullsend#5562

[fullsend-ai-prioritize]

RICE Priority Score: 2.4

Score breakdown

Dimension	Score	Reasoning
Reach	2	The review agent operates across all fullsend-enrolled repos, so this policy improvement affects most active users. The pattern of removing defensive code in favor of SDK defaults is described as common in SDK-consumer codebases, particularly in the OpenTelemetry ecosystem. Score of 2 reflects platform-wide scope tempered by the specific trigger pattern not applying to every PR.
Impact	1.5	In the cited PR #5562, the agent's failure to escalate allowed three HIGH-severity issues to slip through (broken kill switch, silent endpoint fallback, shutdown hangs), requiring independent human catch. When this pattern triggers, the agent actively undermines review trust by dropping valid findings after author rebuttals it cannot verify. This is a significant pain point (1.5) but not a total blocker since human reviewers can still catch the issues.
Confidence	0.8	High confidence (0.8) because the issue is exceptionally well-documented with a concrete real-world example, clear root cause analysis, a specific proposed change (policy rule addition), and explicit validation criteria. The only uncertainty is recurrence frequency — the issue itself notes MEDIUM confidence that the pattern recurs frequently enough to warrant a rule.
Effort	1	The proposed change is adding a review policy rule to policies/review.yaml or agents/review.md — primarily a prompt/policy file change, not an architectural modification. However, it requires defining the detection heuristic (removed validation code + SDK-delegation language), specifying escalation and persistence behavior across review rounds, and validating across 5 PRs. This is medium effort (1).

Formula: (2 x 1.5 x 0.8) / 1 = 2.4