Skip to content

feat(review): merge-train FIFO gate for out-of-order merges#4035

Merged
JSONbored merged 3 commits into
mainfrom
feat/merge-train-fifo-gate
Jul 7, 2026
Merged

feat(review): merge-train FIFO gate for out-of-order merges#4035
JSONbored merged 3 commits into
mainfrom
feat/merge-train-fifo-gate

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Closes #4004.

A PR currently merges the instant its OWN gate clears, with zero awareness of an older sibling PR still open in the same repo — a live read-only production query proved this causes hundreds of out-of-order merges per repo (505 gittensory / 708 metagraphed / 1364 awesome-claude), and the conflicts that follow.

  • New settings.mergeTrainMode ("off" | "audit" | "enforce", default "off") — audit logs what would be held without holding anything (safe to validate the fix before enabling it for real); enforce actually defers a merge behind a still-viable older sibling.
  • A git-conflicted older sibling never blocks — it isn't about to merge, it's stuck.
  • Bounded by a 24h staleness cap (matches this codebase's own REGATE_REPAIR_ATTEMPT_LOOKBACK_MS precedent) so one genuinely stuck PR can never wedge the repo's newer ones forever.
  • The decision itself (src/review/merge-train.ts) is a pure, 100%-branch-covered function reusing data already fetched on every merge pass (listOtherOpenPullRequests, already ordered ascending by PR number) — no new query, no new persisted state beyond the one settings column.
  • Wired into executeAgentMaintenanceActions right before the actual merge mutation, threaded through the two call sites that can plan a merge action (the main per-PR maintenance pass, and the approval-queue accept-replay path).
  • Config-as-code parity in the same PR: migration, Drizzle schema, RepositorySettings type, OpenAPI schema, and both .gittensory.yml.example/config/examples/gittensory.full.yml (byte-sync verified via the existing config-templates.test.ts).

Recommended rollout: enable mergeTrainMode: audit first on each gated repo to observe how often it would fire with zero behavior change, then flip to enforce once validated.

Validation

  • npm run typecheck — clean
  • npm run db:migrations:check / db:schema-drift:check / docs:drift-check — clean
  • npm run ui:openapi:settings-parity — clean (98 fields matched)
  • npx vitest run test/unit/merge-train.test.ts test/unit/agent-action-executor.test.ts test/unit/config-templates.test.ts — 182/182 passing, 100% branch coverage on the new pure module

Notes

Companion tracking issue #4006 (the P1: event-driven re-gate trigger on sibling merge) is a natural follow-up to close the remaining "stale sibling" gap, but is out of scope for this PR.

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 7, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 81cc01e Commit Preview URL

Branch Preview URL
Jul 07 2026, 12:13 PM

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-07 12:27:56 UTC

15 files · 1 AI reviewer · no blockers · readiness 100/100 · CI failing · unstable

🛑 Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/selfhost/metrics.ts (matched src/selfhost/**).

Review summary
Adds an opt-in (default "off") FIFO merge-train gate that defers a merge behind a still-open, non-conflicted, non-stale older sibling PR in the same repo, with audit/enforce modes. The pure decision function in merge-train.ts is small, deterministic, and thoroughly branch-tested (tiebreaks, staleness cap, dirty-sibling exclusion). Wiring is complete end-to-end: both call sites that can plan a merge (main maintenance pass and approval-queue replay) thread mergeTrainMode/pullRequestCreatedAt through, and config-as-code parity (schema, migration, types, OpenAPI, both yml examples) is present and consistent. No correctness defect found on the reachable path.

Nits — 7 non-blocking
  • src/review/merge-train.ts:16 defines its own MERGE_TRAIN_MAX_WAIT_MS = 24h rather than importing REGATE_REPAIR_ATTEMPT_LOOKBACK_MS from src/queue/processors.ts — the comment claims it 'matches' that constant, but duplicating the literal instead of importing it means the two can silently drift apart later; import the shared constant or export a single source of truth.
  • codecov/patch reports 92.85% vs the 99% target on this diff — worth checking which branches in agent-action-executor.ts's new 8b block (e.g. the audit-mode recordAuditEvent path or the metrics incr call) are missing a direct assertion before merging.
  • The PR description states the gate reuses data 'already fetched on every merge pass... no new query,' but src/services/agent-action-executor.ts's new block calls `listOtherOpenPullRequests` fresh, gated only on mergeTrainMode !== "off" — it is in fact a new query whenever the gate is enabled (bounded and opt-in, so not a blocker, but the description overstates it).
  • Consider a short migration-level regression test for the new merge_train_mode column default the way other data migrations in this repo require (per the schema-migrations convention), even though this is a simple ADD COLUMN with a DEFAULT.
  • In src/services/agent-approval-queue.ts, double check `pr?.createdAt` can't silently be undefined in the normal replay path in a way that always forces the PR-number tiebreak — worth a one-line comment noting the fail-open intent if that's expected.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

CI checks failing

  • codecov/patch — 92.85% of diff hit (target 99.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4004
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 570 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 570 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 570 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.58%. Comparing base (3a74800) to head (81cc01e).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/db/repositories.ts 0.00% 0 Missing and 1 partial ⚠️
src/services/agent-action-executor.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4035      +/-   ##
==========================================
- Coverage   93.58%   93.58%   -0.01%     
==========================================
  Files         381      382       +1     
  Lines       35483    35511      +28     
  Branches    13014    13024      +10     
==========================================
+ Hits        33206    33232      +26     
- Misses       1617     1618       +1     
- Partials      660      661       +1     
Files with missing lines Coverage Δ
src/db/schema.ts 71.66% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)
src/queue/processors.ts 94.84% <ø> (ø)
src/review/merge-train.ts 100.00% <100.00%> (ø)
src/selfhost/metrics.ts 100.00% <ø> (ø)
src/services/agent-approval-queue.ts 99.17% <ø> (ø)
src/types.ts 100.00% <ø> (ø)
src/db/repositories.ts 96.62% <0.00%> (-0.06%) ⬇️
src/services/agent-action-executor.ts 96.85% <90.00%> (-0.25%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added 3 commits July 7, 2026 05:11
A PR currently merges the instant its OWN gate clears, with zero
awareness of an older sibling PR still open in the same repo -- a
live production query proved this causes hundreds of out-of-order
merges per repo (505/708/1364 across the three gated repos), and the
conflicts that follow.

Adds a new settings.mergeTrainMode ("off" | "audit" | "enforce",
default "off") gate: "audit" logs what would be held without holding
anything; "enforce" actually defers a merge behind a still-viable
older sibling, bounded by a 24h staleness cap so one stuck PR can
never wedge the repo's newer ones forever. A git-conflicted sibling
never blocks -- it isn't about to merge, it's stuck.

The decision itself (src/review/merge-train.ts) is a pure function
reusing data already fetched on every merge pass (listOtherOpenPullRequests,
already ordered ascending by PR number) -- no new query, no new
persisted state beyond the one settings column. Wired into
executeAgentMaintenanceActions right before the actual merge mutation,
threaded through the two call sites that can plan a merge action.
0119 was claimed by another merged PR (ai_slop_cache), and 0120 by
#4032, while this one was in flight. Renumbered to the next free
slot on top of the current branch tip.
@JSONbored
JSONbored force-pushed the feat/merge-train-fifo-gate branch from fb962f5 to 81cc01e Compare July 7, 2026 12:11
@JSONbored
JSONbored merged commit bb7f8ed into main Jul 7, 2026
12 of 13 checks passed
@JSONbored
JSONbored deleted the feat/merge-train-fifo-gate branch July 7, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

feat(review): merge-train gate for FIFO merge ordering

1 participant