Skip to content

Correctness sub-agent should verify API resource identity uniqueness under concurrent use #993

Description

@fullsend-ai-retro

What happened

On PR #5957, the human reviewer (waynesun09's Aug 6 review) identified that GitHub reactions are keyed by (actor, subject, content), meaning two concurrent runs by the same bot with the same emoji would share the same reaction ID. Deleting the 'start' reaction in one run would delete the other run's reaction, causing race conditions. The review agent ran 14 iterations reviewing the reaction CRUD code and never flagged this. The author acknowledged it as a known limitation and documented it, but the agent should have caught it as a correctness concern.

What could go better

The correctness sub-agent does not currently check whether external API resources created/deleted by the code under review have sufficient identity to distinguish concurrent operations. Existing issues like #714 (staggered-arrival timing) and #503 (unsynchronized shared state) address in-code concurrency patterns, but neither covers the distinct concern of external API resource identity — where the identity model is imposed by the third-party API (e.g., GitHub's reaction dedup by actor+content), not by the code itself. This is a high-confidence finding: the human caught it on the first review pass while the agent missed it across 14 runs.

Proposed change

Add guidance to the correctness sub-agent definition in fullsend-ai/agents to include an 'external resource identity' check. When the agent reviews code that creates or deletes API resources (reactions, labels, status checks, deployments, comments), it should ask: 'If two instances of this code run concurrently with the same credentials, will they be able to distinguish their own resources from each other's?' The agent should check the API's uniqueness constraints (e.g., GitHub reactions are unique per actor+subject+content) and flag when the code relies on a resource identity that doesn't support concurrent disambiguation. This guidance should be added to the correctness sub-agent's checklist alongside the existing side-effect precondition checks (#298).

Validation criteria

The next review of a PR that implements create/delete patterns on external API resources (reactions, labels, status checks) should include a finding about concurrent operation identity if the resource's uniqueness model doesn't support concurrent disambiguation. Verify across 3 such PRs within 90 days of the change landing.


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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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