feat(dedup): bypass dedup for synchronize and reopened PR actions

[marcusrbrown]

Summary

• synchronize and reopened PR actions now bypass dedup entirely — no cache lookup occurs
• Other PR actions (review_requested, ready_for_review) continue to be deduped as before

Why

synchronize: Fires first on every push to a PR. In repos with required status checks on Fro Bot, this is the run that provides the check result. If dedup blocked it (e.g., a sentinel from a prior review_requested run), the required check would never pass and the PR would be unmergeable.

reopened: A PR that was closed and reopened is a meaningful state change. The previous sentinel is stale — the agent should re-evaluate the PR.

Investigation

Examined fro-bot workflow runs across marcusrbrown/mrbro.dev, marcusrbrown/vbs, marcusrbrown/gpt, and marcusrbrown/copiloting. Real-world dedup data is thin since v0.33.0 just shipped and most PR activity in these repos is from bots (blocked by workflow-level if: conditions before dedup runs). The vbs repo showed the clearest pattern: feature/phase6-cli-integration had 18+ runs alternating skipped/cancelled, all workflow-level gating on bot-authored PRs.

The bypass logic is based on the GitHub event model:

1. Push to PR → synchronize (first, provides status check)
2. Auto review request → review_requested (should be deduped)
3. Mark ready → ready_for_review (should be deduped)

Changes

• src/harness/phases/dedup.ts: Add DEDUP_BYPASS_ACTIONS set (synchronize, reopened), check before cache lookup
• src/harness/phases/dedup.test.ts: 2 new tests for bypass behavior
• dist/: Rebuilt

Verification

• 1087 tests passing (2 new)
• 0 lint errors, 0 type errors
• Build clean

[fro-bot]

Review Summary

This PR adds a bypass mechanism for the deduplication phase for two PR actions (synchronize and reopened). The implementation is correct, well-tested, and solves a real issue with status checks.

Implementation Analysis

Core Change

The change adds DEDUP_BYPASS_ACTIONS Set and checks it before cache lookup in runDedup():

const DEDUP_BYPASS_ACTIONS = new Set(['synchronize', 'reopened'])

// In runDedup:
if (triggerContext.action != null && DEDUP_BYPASS_ACTIONS.has(triggerContext.action)) {
  logger.debug('Dedup bypassed for action', {action: triggerContext.action})
  return {shouldProceed: true, entity}
}

Why This Works

1. synchronize bypass: This action fires first on every push and provides the required status check result. If dedup blocked it (due to a prior review_requested run's sentinel), the PR would be unmergeable. The bypass ensures status checks always run.

2. reopened bypass: A PR that was closed and reopened is a meaningful state change. The previous sentinel is stale—the agent should re-evaluate fresh.

3. Placement is optimal: The bypass check is placed BEFORE the cache lookup (line 65), which avoids unnecessary cache operations for these actions.

4. Entity is still returned: Even bypassed actions return the entity for downstream use—correct behavior.

Code Quality ✓

• Follows project patterns (Set for O(1) lookup, early return, debug logging)
• Proper null handling: triggerContext.action != null check before Set.has()
• Clean BDD-style test comments
• No type safety issues introduced

Test Coverage ✓

Two new tests verify:

• bypasses dedup for synchronize action — confirms no cache lookup occurs
• bypasses dedup for reopened action — confirms no cache lookup occurs

Both tests correctly assert restoreDeduplicationMarker was never called and the result allows execution to proceed.

Rationale Validation

The PR correctly identifies:

• opened doesn't need bypass (no prior marker for new PRs)
• review_requested / ready_for_review should be deduped (secondary actions after push)
• The event sequence: push → synchronize (first, provides status) → auto-review-request → review_requested (should dedup)

Verification

• ✓ 1087 tests passing (2 new)
• ✓ No lint errors
• ✓ No type errors
• ✓ Build clean
• ✓ dist/ rebuilt

Minor Observations (non-blocking)

1. The debug log message is helpful for observability in CI logs.
2. Tests verify behavior correctly but don't assert the debug log was called—minor improvement opportunity, not blocking.

---

Verdict: Approved. This is a well-scoped, minimal change that solves a real problem without introducing complexity or risk.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	23698374649
Cache	hit
Session	ses_2c8d85967ffelEmqZThbWZRCEV

[fro-bot]

Review Summary

This PR adds a bypass mechanism for the deduplication phase for two PR actions (synchronize and reopened). The implementation is correct, well-tested, and solves a real issue with status checks.

Implementation Analysis

Core Change

The change adds DEDUP_BYPASS_ACTIONS Set and checks it before cache lookup in runDedup():

```typescript
const DEDUP_BYPASS_ACTIONS = new Set(['synchronize', 'reopened'])

// In runDedup:
if (triggerContext.action != null && DEDUP_BYPASS_ACTIONS.has(triggerContext.action)) {
logger.debug('Dedup bypassed for action', {action: triggerContext.action})
return {shouldProceed: true, entity}
}
```

Why This Works

1. synchronize bypass: This action fires first on every push and provides the required status check result. If dedup blocked it (due to a prior review_requested run's sentinel), the PR would be unmergeable. The bypass ensures status checks always run.

2. reopened bypass: A PR that was closed and reopened is a meaningful state change. The previous sentinel is stale—the agent should re-evaluate fresh.

3. Placement is optimal: The bypass check is placed BEFORE the cache lookup (line 58), which avoids unnecessary cache operations for these actions.

4. Entity is still returned: Even bypassed actions return the entity for downstream use—correct behavior.

Code Quality ✓

• Follows project patterns (Set for O(1) lookup, early return, debug logging)
• Proper null handling: triggerContext.action != null check before Set.has()
• Clean BDD-style test comments
• No type safety issues introduced

Test Coverage ✓

Two new tests verify:

• bypasses dedup for synchronize action — confirms no cache lookup occurs
• bypasses dedup for reopened action — confirms no cache lookup occurs

Both tests correctly assert restoreDeduplicationMarker was never called and the result allows execution to proceed.

Rationale Validation

The PR correctly identifies:

• opened doesn't need bypass (no prior marker for new PRs)
• review_requested / ready_for_review should be deduped (secondary actions after push)
• The event sequence: push → synchronize (first, provides status) → auto-review-request → review_requested (should dedup)

Verification

• ✓ Tests passing (2 new)
• ✓ No lint errors
• ✓ No type errors
• ✓ Build clean
• ✓ dist/ rebuilt

Minor Observations (non-blocking)

1. The debug log message is helpful for observability in CI logs.
2. Tests verify behavior correctly but don't assert the debug log was called—minor improvement opportunity, not blocking.

---

Verdict: Approved. This is a well-scoped, minimal change that solves a real problem without introducing complexity or risk.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	23698876227
Cache	hit
Session	ses_2c8ba083bffew4OwB5pdYjUGS0