Skip to content

Review correctness sub-agent should check conditional branch completeness for forge-discriminated logic #1097

Description

@fullsend-ai-retro

What happened

On PR #6769, the PR introduced a jq conditional that clears ISSUE_NUMBER only for Jira events: if ._normalized_event.source.system == "jira" then empty else .... The qodo-code-review bot caught that this is incomplete — a normalized GitLab event would also populate ISSUE_NUMBER with its GitLab entity ID, which could address an unrelated GitHub issue. The fullsend review agent's correctness sub-agent (run 33273340273 and run 33317698843) did not flag this logic bug. Both review passes focused on syntax-level patterns (heredoc format, delimiter naming, coverage consistency) but missed the semantic gap in the conditional branching.

What could go better

The correctness sub-agent should detect incomplete conditionals when a PR introduces forge-discriminated logic. When code branches on a forge/system identifier (e.g., checking for "jira"), the reviewer should enumerate the other known forge types (GitHub, GitLab, Bitbucket) and verify the else branch is safe for all of them. This is a specific case of conditional branch completeness analysis. Confidence is high that the finding was valid — the PR only excludes Jira from ISSUE_NUMBER but the normalized event schema supports multiple external trackers. The review agent had the context needed to catch this: it read the normalized event documentation and ADR 0063, and it fetched the linked issue #6760 which explicitly discusses cross-forge semantics.

Proposed change

Enhance the correctness sub-agent's prompt in the review agent definition (in fullsend-ai/agents) to include guidance on forge-discriminated conditional completeness. When a PR introduces or modifies a condition that checks a forge/system identifier (e.g., source.system, forge, provider), the sub-agent should: (1) identify all known forge types from the codebase context (normalized event schema, platform documentation), (2) verify the conditional handles each type correctly or that the else/default branch is safe for all unlisted types, (3) flag any case where a forge-specific value could leak into a variable scoped to a different forge. This could be added as a checklist item in the correctness sub-agent's review dimensions or as a dedicated heuristic for workflow/infrastructure PRs.

Validation criteria

On the next 3 PRs that modify forge-discriminated conditional logic in workflow files or harness code, the review agent's correctness sub-agent should flag incomplete forge-type handling when the conditional only checks a subset of known forge types. False positive rate should stay below 20% — the guidance should not trigger on conditionals where the else branch is genuinely safe for all forge types.


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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions