Skip to content

fix: keep candidate-owned conflicts off feature heads - #673

Merged
krasnoperov-coding-agent[bot] merged 2 commits into
mainfrom
feature/defer-dirty-pr-to-integration-candidate
Sep 14, 2026
Merged

krasnoperov-coding-agent[bot] merged 2 commits into
mainfrom
feature/defer-dirty-pr-to-integration-candidate

Conversation

@krasnoperov-coding-agent

Copy link
Copy Markdown
Contributor

Problem

Idle reconciliation can observe an approved feature PR as DIRTY while Merge Steward already owns its conflict through a published integration candidate. It then starts legacy queue_repair, rewrites the frozen feature head, invalidates approval, and supersedes the candidate being validated.

Fix

  • Read the self-describing merge-steward/<base>/pr-<number> candidate before dispatching legacy branch repair.
  • Keep the approved feature head frozen whenever the candidate exists.
  • Fail closed when candidate ownership cannot be read.
  • Allow legacy feature-branch repair only after an explicit candidate-ref 404.

Verification

  • Added regressions for an existing candidate and an unavailable candidate lookup.
  • Preserved explicit legacy behavior when the candidate is proven absent.
  • pnpm typecheck
  • pnpm lint
  • pnpm test (1069 passed)
  • pnpm build

@krasnoperov-coding-agent
krasnoperov-coding-agent Bot marked this pull request as ready for review September 14, 2026 19:54
review-quill[bot]
review-quill Bot previously requested changes Sep 14, 2026

@review-quill review-quill Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 🛑 Request changes — The behavior is covered, but the implementation worsens an orchestrator structure explicitly prohibited by the repository architecture guardrail.

Comment thread src/idle-reconciliation.ts Outdated
return;
}
if (issue.delegatedToPatchRelay && reactiveIntent?.runType === "queue_repair" && mergeConflictDetected) {
const candidate = pr.headRefOid && project.github?.repoFullName

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 This adds candidate-ownership policy and another GitHub phase directly to a 1,048-line reconciler that already mixes routing, persistence, API calls, and operator events. The architecture guardrail requires extracting by responsibility before adding behavior here; move this gate into a focused collaborator and leave the reconciler to sequence its result.

@review-quill
review-quill Bot dismissed their stale review September 14, 2026 20:00

Superseded by newer head ebc27f8; review-quill will re-review the latest commit separately.

@review-quill review-quill Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve — The ownership decision and candidate lookup are now isolated in a focused collaborator, resolving the prior architecture concern without changing the validated fail-closed behavior.

@krasnoperov-coding-agent krasnoperov-coding-agent Bot added queue:testing Merge-steward is validating speculative CI queue:merging Merge-steward is merging this PR and removed queue:testing Merge-steward is validating speculative CI labels Sep 14, 2026
@krasnoperov-coding-agent
krasnoperov-coding-agent Bot merged commit ebc27f8 into main Sep 14, 2026
2 of 3 checks passed
@krasnoperov-coding-agent
krasnoperov-coding-agent Bot deleted the feature/defer-dirty-pr-to-integration-candidate branch September 14, 2026 20:09
@krasnoperov-coding-agent krasnoperov-coding-agent Bot removed the queue:merging Merge-steward is merging this PR label Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants