Skip to content

fix(settings): default linked issue gate to advisory#2727

Merged
JSONbored merged 4 commits into
mainfrom
fix/linked-issue-gate-advisory-default
Jul 3, 2026
Merged

fix(settings): default linked issue gate to advisory#2727
JSONbored merged 4 commits into
mainfrom
fix/linked-issue-gate-advisory-default

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

repository_settings.linked_issue_gate_mode's Drizzle schema default was "block" in src/db/schema.ts, while every other layer already agreed on "advisory": the application fallback in src/db/repositories.ts (?? "advisory" on both the no-row read path and the upsert path), .gittensory.yml.example, the tuning docs, the settings API Zod schema (.default("advisory")), and the gate-evaluation code's own comment in src/rules/advisory.ts ("Missing linked issue defaults to ADVISORY"). A missing linked issue must only be advisory — surfaced in the review panel, never a blocker — unless a repo explicitly opts in via linkedIssueGateMode: "block" or the "Require linked issue" toggle.

Why missing linked issue must not auto-block by default

Issues aren't always available (a small/self-evident fix legitimately has none), so treating a missing linked issue as a hard blocker by default would close/block PRs that were never supposed to be blocked. That's exactly what happened in production: several repos' repository_settings rows had linked_issue_gate_mode='block' despite no maintainer ever opting into it.

Root cause & migration behavior

The drift traces to two historical migrations:

  • migrations/0023_gate_quality_modes.sql introduced the column with DEFAULT 'block'.
  • migrations/0025_restore_gate_blocking_modes.sql later flipped any 'advisory' row back to 'block' for repos with gate_check_mode='enabled' — a "restore" that itself encoded the wrong default.

migrations/0101_fix_linked_issue_gate_mode_default.sql backfills the rows those two migrations affected — conservatively: it only corrects a row that is currently 'block' AND has require_linked_issue = 0. A row with require_linked_issue = 1 is left untouched, since that boolean is an explicit maintainer opt-in that resolveEffectiveSettings (src/signals/focus-manifest.ts) itself promotes to 'block' whenever the gate would otherwise be 'off' (#797) — a 'block' value paired with that opt-in already reflects an intentional choice, not the drift this migration exists to correct. The migration is a plain data-only UPDATE (no DDL), portable identically to SQLite and the self-host Postgres backend (pg-dialect.ts only translates function calls, not literal values — confirmed no translation is needed for this statement).

Deliberately did NOT add a blanket resolver-level downgrade the way qualityGateMode: "block" is downgraded (#2267) — qualityGateMode can never legitimately be "block" (no code path enforces it), but linkedIssueGateMode: "block" is a legitimate, intentionally-supported opt-in, so an unconditional downgrade would silently defeat a real maintainer choice. A new test (focus-manifest.test.ts) pins this asymmetry explicitly.

Audit for similar drift

Cross-checked every other setting listed against src/db/schema.ts, src/db/repositories.ts's fallback object, .gittensory.yml.example, the settings API schema, and docs: duplicatePrGateMode, qualityGateMode, selfAuthoredLinkedIssueGateMode, slopGateMode, mergeReadinessGateMode, manifestPolicyGateMode, aiReviewMode, closeOwnerAuthors, agentDryRun, autonomyJson, and the label/PR-cap defaults. All already agree across every layer — this was an isolated single-column drift, not a systemic pattern.

What this does NOT change

The separate deterministic linked-issue hard rule (src/review/linked-issue-hard-rules.ts — closing a PR that links an ineligible issue: owner-assigned, missing a point-bearing label, maintainer-only label) shares no config field with linkedIssueGateMode and is structurally incapable of reading it (evaluateLinkedIssueHardRules's input type has no such field). A new regression test in linked-issue-hard-rules.test.ts pins this isolation end-to-end: an ineligible linked issue still violates the hard rule regardless of the (now-fixed) advisory gate default.

Validation

  • Focused tests first: migration-0101-linked-issue-gate-mode.test.ts (5 cases — the conservative WHERE clause, the explicit-opt-in exclusion, idempotency), repository-settings-linked-issue-defaults.test.ts (4 cases — new-row default, upsert-omits-field default, explicit block opt-in preserved), focus-manifest.test.ts (+2 regression cases), linked-issue-hard-rules.test.ts (+1 orthogonality regression) — all green.
  • npm run test:ci (full local gate, includes db:migrations:check, db:schema-drift:check, selfhost:env-reference:check, typecheck, test:coverage, test:workers, ui:*) — 7141 tests passed, fully green.
  • npm run test:coverage (unsharded) — 96.53% statements / 95.61% branches project-wide; the only src/** line touched is a default-value string + comments in schema.ts, already exercised by every settings-related test in the suite.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • Regenerated apps/gittensory-ui/src/lib/selfhost-env-reference.ts (picked up unrelated line-number drift from recently-merged main commits; included since the check requires it to be current). No OpenAPI/config-schema regeneration needed — no API route or Zod schema changed.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 3, 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 5ec41a8 Commit Preview URL

Branch Preview URL
Jul 03 2026, 10:34 AM

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.04%. Comparing base (da919ee) to head (5ec41a8).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2727   +/-   ##
=======================================
  Coverage   96.04%   96.04%           
=======================================
  Files         245      245           
  Lines       27380    27380           
  Branches     9945     9945           
=======================================
  Hits        26297    26297           
  Misses        460      460           
  Partials      623      623           
Files with missing lines Coverage Δ
src/db/schema.ts 69.46% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-03 10:38:34 UTC

9 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The diff realigns the linked-issue gate default to advisory in the Drizzle schema, documents the intended missing-issue behavior, and adds a conservative data migration plus regression coverage for defaulting and opt-in preservation. The migration is D1-safe SQL and avoids clobbering rows with evidence of a post-column settings write, which is the right bias for a setting where explicit block remains valid. I did not find a reachable correctness defect in the visible diff.

Nits — 4 non-blocking
  • nit: test/unit/repository-settings-linked-issue-defaults.test.ts:31 says the re-upsert omits linkedIssueGateMode, but `{ ...settings, repoFullName: "acme/round-trip" }` includes the field, so the test name overstates the path it covers.
  • nit: migrations/0102_fix_linked_issue_gate_mode_default.sql:1 has an unusually long rationale embedded in the migration; the key invariant is useful, but most of the review-history narrative would be easier to maintain in the PR or an incident note.
  • test/unit/repository-settings-linked-issue-defaults.test.ts:31: rename the test to say it round-trips an already-read settings object, or actually omit `linkedIssueGateMode` from the second upsert if that is the behavior you mean to pin.
  • migrations/0102_fix_linked_issue_gate_mode_default.sql:1: keep the cutoff proof and safe-failure rationale, but trim the multi-round review history so future migration readers can find the operational rule quickly.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 61 registered-repo PR(s), 52 merged, 478 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 61 PR(s), 478 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 61 PR(s), 478 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 3, 2026
@JSONbored
JSONbored force-pushed the fix/linked-issue-gate-advisory-default branch from 27c468f to d062c96 Compare July 3, 2026 09:48
@JSONbored
JSONbored force-pushed the fix/linked-issue-gate-advisory-default branch from d062c96 to afb3058 Compare July 3, 2026 10:09
JSONbored added 4 commits July 3, 2026 03:32
repository_settings.linked_issue_gate_mode's Drizzle schema default
was "block", drifting from every other layer that already said
"advisory" (the application fallback in src/db/repositories.ts,
.gittensory.yml.example, the tuning docs, the settings API schema,
and the gate-evaluation code's own comment). A missing linked issue
must only be advisory unless a repo explicitly opts in, via an
explicit linkedIssueGateMode: "block" or the "Require linked issue"
toggle.

The drift traces to two historical migrations: 0023 introduced the
column with DEFAULT 'block', and 0025 later flipped any 'advisory'
row back to 'block' for repos with an enabled gate. Migration 0101
backfills the rows those two migrations affected, conservatively:
only a row that is currently 'block' AND has no explicit
require_linked_issue opt-in is corrected, so a genuine maintainer
opt-in is left untouched.

Audited every other gate-mode/default setting listed for similar
drift (duplicatePrGateMode, qualityGateMode, selfAuthoredLinkedIssueGateMode,
slopGateMode, mergeReadinessGateMode, manifestPolicyGateMode,
aiReviewMode, closeOwnerAuthors, agentDryRun, autonomyJson, and the
label/PR-cap defaults) -- schema.ts, the application fallback, and
the documented defaults already agree everywhere else; this was an
isolated single-column drift.

The separate deterministic linked-issue hard rule (closing a PR
that links an INELIGIBLE issue -- owner-assigned, wrong label,
etc.) shares no config with linkedIssueGateMode and is unaffected;
a new regression test pins that isolation explicitly.
require_linked_issue = 0 alone doesn't prove linked_issue_gate_mode
= 'block' was default drift: the two fields are independently
settable from the maintainer settings UI, so a maintainer can
genuinely choose 'block' while leaving require_linked_issue off.
Narrow the backfill to rows where updated_at still equals
created_at -- the only rows that have never been written to since
creation, and therefore can only hold a migration-era default, not
a reviewed or reaffirmed choice.

Renumbered to 0102 after a numbering collision with an
already-merged migration.
…de column

updated_at = created_at alone missed a real class of drifted rows:
ones last written before migration 0023 added the column, which
have updated_at > created_at but still could not possibly have
chosen 'block' since the column didn't exist yet. Add a second,
independently provable path -- updated_at at or before the instant
0023 deployed (proxied by its merge commit time, which is always
at or before the real remote migration apply time) -- covering
those rows without touching any row genuinely written after the
column existed.

Also fixes stale 0101 references left over from the migration
renumbering.
…own deploy time

The updated_at = created_at signal could not tell a genuinely
untouched drifted row apart from a repo whose very first settings
save explicitly chose 'block' (a single INSERT sets both timestamps
to the same instant either way). Replace it with the single
condition already partially in use for the pre-0023 case:
updated_at at or before the instant migration 0023 deployed. Since
upsertRepositorySettings is the only writer and always resolves an
explicit value, any row untouched since before the column existed
can only hold a migration-era default, while any write after that
point -- first save or later -- reflects a real, resolvable choice.

Also documents why the raw SQLite column-level default is
intentionally left at 'block' (no ALTER COLUMN SET DEFAULT in
SQLite, no live path ever reaches it) and pins that behavior with a
regression test, and fixes a JSX whitespace bug in the tuning docs.
@JSONbored
JSONbored force-pushed the fix/linked-issue-gate-advisory-default branch from afb3058 to 5ec41a8 Compare July 3, 2026 10:33
@JSONbored
JSONbored merged commit 2dda626 into main Jul 3, 2026
15 checks passed
@JSONbored
JSONbored deleted the fix/linked-issue-gate-advisory-default branch July 3, 2026 10:42
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.

Development

Successfully merging this pull request may close these issues.

1 participant