Skip to content

feat(github-app): merge-readiness aggregate gate - #632

Closed
kiannidev wants to merge 4 commits into
JSONbored:mainfrom
kiannidev:feat/issue-551-merge-readiness-gate
Closed

feat(github-app): merge-readiness aggregate gate#632
kiannidev wants to merge 4 commits into
JSONbored:mainfrom
kiannidev:feat/issue-551-merge-readiness-gate

Conversation

@kiannidev

Copy link
Copy Markdown
Contributor

Summary

  • Adds a per-repo mergeReadinessGateMode setting (off/advisory/block) with migration 0026_merge_readiness_gate_mode.sql.
  • Introduces src/rules/merge-readiness-gate.ts to aggregate linked-issue, duplicate, readiness, and slop conditions into one composite gate evaluation.
  • Extends the queue processor to load slop findings and pass them into evaluateGateCheck when composite mode is enabled.

Closes #551

Test plan

  • npm run typecheck and unit/integration tests pass locally
  • Composite off/advisory/block coverage in test/unit/merge-readiness-gate.test.ts
  • Queue integration test for composite linked-issue + slop blockers (test/unit/queue.test.ts)
  • GitHub App check-run formatting test for composite blockers (test/unit/github-app.test.ts)
  • npm run ui:openapi:check

Made with Cursor

Add mergeReadinessGateMode to roll linked-issue, duplicate, readiness, and
slop signals into one Gittensory Gate conclusion with a blocking summary.

Co-authored-by: Cursor <cursoragent@cursor.com>
@superagent-security

Copy link
Copy Markdown
Contributor

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

@ghost ghost 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.

Gittensory review · advisory — a maintainer merges

0 actionable · 6 nitpick(s) · 23 file(s) · two independent AI reviewers

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

📋 Walkthrough

The PR adds a mergeReadinessGateMode setting across the database, API, and internal gate evaluation, allowing composite gating based on linked issues, duplicate PRs, quality scores, and slop findings. It updates schemas, types, and adds comprehensive unit and integration tests, but the queue processor lacks an import for listPullRequestFiles.

Changes

File Summary
apps/gittensory-ui/public/openapi.json Adds mergeReadinessGateMode property to RepositorySettings schema and required list.
migrations/0026_merge_readiness_gate_mode.sql Adds new column merge_readiness_gate_mode with default 'off' to repository_settings table.
src/api/routes.ts Extends repository settings validation and handling to include mergeReadinessGateMode.
src/db/repositories.ts Handles default, parsing, upserting, and retrieving of the new setting.
src/db/schema.ts Defines new column in sqlite schema.
src/openapi/schemas.ts Adds Zod schema entry for the new setting.
src/queue/processors.ts Integrates slop findings into gate evaluation and adds helper to load them (missing import for listPullRequestFiles).
src/rules/advisory.ts Adds composite gate handling, new helper functions, and updates evaluation flow.
src/rules/merge-readiness-gate.ts Implements merge‑readiness composite gate logic and mapping of slop findings.
src/types.ts Adds mergeReadinessGateMode to RepositorySettings type.
test/unit/merge-readiness-gate.test.ts Adds extensive unit tests for the new composite gate.
test/unit/... (multiple) Updates existing tests to include the new setting in default configurations.
🔍 Reviewer notes

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
This PR introduces a new merge‑readiness aggregate gate, adding a DB column, API schema, type definitions, evaluation logic, and extensive tests. The implementation is thorough but misses an import for listPullRequestFiles in the queue processor, causing a compile error.

Suggestions

  • Import listPullRequestFiles in src/queue/processors.ts (e.g., import { listPullRequestFiles } from "../github/pull-requests";).
  • Run the TypeScript compiler and tests after adding the import to ensure no further missing symbols.
  • Verify that the new public output does not contain any forbidden terms (tests already check this).

Worth double-checking

  • Missing import will prevent the app from building and deploying.
  • Potential runtime errors if the slop assessment integration behaves unexpectedly with large PRs.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a merge-readiness aggregate gate feature that combines linked issue, duplicate PR, quality, and slop checks into a single composite gate with three modes (off, advisory, block). Changes span database schema, API, types, rules logic, and tests. Implementation follows existing patterns, maintains backward compatibility via default 'off' mode, and includes comprehensive test coverage. No public/private boundary violations detected.

Suggestions

  • Add a comment in migration 0026_merge_readiness_gate_mode.sql explaining the column's purpose
  • Consider extracting buildMergeReadinessBlockingSummary to a test utility if reused elsewhere
  • Verify slop findings reuse doesn't introduce redundant computation in high-throughput scenarios

Worth double-checking

  • Performance impact from loading slop findings for every PR when gate is enabled (mitigated by existing slop signal reuse)
  • Ensure gateMode function export doesn't break internal callers (updated usage appears consistent)
  • Confirm OpenAPI required array addition aligns with schema nullability (column is NOT NULL with default)

Warning

Could not capture a preview of the changed route(s) — review the rendered page manually before merging.


🤖 Automated advisory review · a maintainer makes the final call.

@ghost ghost added the gittensory-review label Jun 12, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@ghost

ghost commented Jun 12, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #632 is no longer open. No action.

💰 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.

@ghost ghost added the gittensory:reviewed label Jun 12, 2026

@ghost ghost 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.

Gittensory review · advisory — a maintainer merges

0 actionable · 6 nitpick(s) · 23 file(s) · two independent AI reviewers

Suggested action: 🔍 Manual review. The changed route's preview could not be captured — review the rendered page manually before merging.

📋 Walkthrough

The PR adds a mergeReadinessGateMode column to repository settings, updates schemas and defaults, implements composite gate evaluation that aggregates linked‑issue, duplicate‑PR, quality, and slop findings, and extends the queue processor to load slop findings when needed. Comprehensive tests cover the new behavior.

Changes

File Summary
apps/gittensory-ui/public/openapi.json Adds mergeReadinessGateMode enum to RepositorySettings schema.
migrations/0026_merge_readiness_gate_mode.sql Adds new column merge_readiness_gate_mode with default 'off' to repository_settings table.
src/api/routes.ts Validates and persists mergeReadinessGateMode in repository settings API.
src/db/repositories.ts Handles default, parsing, upserting, and returning of mergeReadinessGateMode.
src/db/schema.ts Adds column definition for merge_readiness_gate_mode.
src/openapi/schemas.ts Exports mergeReadinessGateMode in Zod schema for RepositorySettings.
src/queue/processors.ts Loads slop findings and passes them into gate check policy for composite gate evaluation.
src/rules/advisory.ts Delegates to merge‑readiness composite evaluation when enabled; adds helper exports.
src/rules/merge-readiness-gate.ts Implements composite gate logic, mapping slop findings, and evaluation functions.
src/types.ts Adds mergeReadinessGateMode to RepositorySettings type.
test/unit/merge-readiness-gate.test.ts Adds extensive unit tests for the new composite gate behavior.
test/unit/* Updates numerous tests to include default mergeReadinessGateMode settings.
🔍 Reviewer notes

Reviewer A · gpt-oss-120b — recommends ✅ merge
This PR introduces a new merge‑readiness aggregate gate, adds a DB column and migration, extends API schemas, updates repository settings handling, and implements composite gate logic with slop findings. The changes are well‑tested and do not introduce forbidden public content.

Suggestions

  • Confirm that listPullRequestFiles is imported in src/queue/processors.ts (or already present) to avoid a compile error.
  • Add a brief comment in the migration file noting the default value for future maintainers.
  • Update any internal documentation to reflect the new mergeReadinessGateMode setting.

Worth double-checking

  • If listPullRequestFiles is not imported, the queue processor will fail at runtime.
  • The new DB column requires the migration to be applied in all environments; ensure deployment scripts run it.

Reviewer B · mistral-small-3.1-24b-instruct — recommends ✅ merge
This PR adds a new 'merge-readiness' gate to the Gittensory platform, which aggregates multiple gate checks into a single composite gate. The change is comprehensive, including updates to the API schema, database migrations, and extensive test coverage. The implementation appears to be well-thought-out and follows the project's conventions.

Suggestions

  • Consider adding a brief description of the new 'merge-readiness' gate in the documentation to help users understand its purpose and functionality.
  • Ensure that the new gate's behavior is thoroughly documented in the code comments, especially in the merge-readiness-gate.ts file.
  • Review the test coverage to ensure that all edge cases are handled, particularly around the interaction between different gate modes.

Worth double-checking

  • Verify that the new gate does not introduce any performance regressions, especially in repositories with a large number of pull requests.
  • Ensure that the new gate's behavior is consistent across different repository settings and configurations.
  • Check that the new gate's output is properly sanitized to avoid leaking any forbidden terms.

Warning

Could not capture a preview of the changed route(s) — review the rendered page manually before merging.


🤖 Automated advisory review · a maintainer makes the final call.

@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 23 changed file(s) — two independent AI reviewers.

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
This PR adds a new merge‑readiness gate mode, updates the database schema, API, types, and queue processing, and introduces extensive tests. The feature is well‑covered, but a circular import between advisory.ts and merge‑readiness-gate.ts will cause runtime errors because advisory imports the new module before its own helper functions are defined.

Suggestions

  • Refactor to break the circular dependency: move shared helpers (buildQualityGateFinding, gateMode, isEvaluationBlockerFinding) to a separate utility module that both advisory.ts and merge‑readiness-gate.ts can import.
  • After refactoring, verify that all imports of GateCheckPolicy and related types are still correct and that any remaining direct calls to gateCheckPolicy include the new slopFindings argument where needed.
  • Run the full test suite after the refactor to ensure no new failures are introduced.

Worth double-checking

  • Circular import may cause undefined functions at runtime, breaking gate evaluation for merge‑readiness.
  • If the refactor is incomplete, other modules might still reference moved functions leading to compile errors.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
This PR adds a new merge-readiness aggregate gate feature that combines existing gates (linked issue, duplicate PR, quality) with a new slop-based gate into a single composite gate. The implementation includes database migration, schema updates, gate evaluation logic, and comprehensive tests. The change is well-structured, maintains the public/private boundary, and preserves existing behavior when the new gate is set to 'off'.

Suggestions

  • Add a comment in src/rules/merge-readiness-gate.ts clarifying that slopFindings are already converted to advisory findings and thus safe for public output.
  • Consider adding a test case for the edge case where mergeReadinessGateMode is 'advisory' and all sub-gates are 'off' but slopFindings exist (should pass with warnings).

Worth double-checking

  • Verify that the slop signal (src/signals/slop.ts) continues to adhere to the public/private boundary (no forbidden terms) as it feeds into the new gate.
  • Ensure the performance impact of loading slop assessments for every gate check is acceptable (already exists elsewhere in the codebase).

Warning

Could not capture a preview of the changed route(s) — review the rendered page manually before merging.

Before = production · After = this PR's preview deploy.

Route Viewport Before After
/ desktop before desktop after desktop
mobile before mobile after mobile

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Appreciate the ambition, but please hold — as written this reverses the safety fix from #644 and needs a redesign first.

Blocking concerns

  1. Re-introduces over-blocking (critical). evaluateMergeReadinessGateCheck (src/rules/merge-readiness-gate.ts) never reads confirmedContributor, and advisory.ts short-circuits into it before any contributor check — so mergeReadinessGateMode: "block" would block non-confirmed contributors again, exactly what #644 fixed.
  2. Resurrects action_required. It returns action_required for app/infra state; #644 deliberately made those neutral ("not evaluated yet") so contributors are never blocked on the app's own sync state.
  3. Two parallel gate engines. Forking a separate evaluator duplicates the blocker logic and is the root cause of (1)/(2). Please extend the single evaluateGateCheck path so the contributor gate is enforced in exactly one place.
  4. Tests lock in the regression (assert failure/action_required for non-confirmed authors) — rewrite against #644 semantics.
  5. Scope. +630 lines bundles settings/migration scaffolding with the engine; consider splitting them.

gateCheckPolicy's 3rd param also collides with #644's confirmedContributor. Happy to pair on the rebase.

@JSONbored JSONbored closed this Jun 13, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
@JSONbored JSONbored added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jun 13, 2026
@kiannidev

Copy link
Copy Markdown
Contributor Author

Hi, @JSONbored
Can you reopen this PR to split?
As you know, closing is affecting to the credibility...

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

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

feat(github-app): merge-readiness aggregate gate

2 participants