From ccb27b40f30e2534edfece5e42ab006ac062c831 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 16:36:37 -0500 Subject: [PATCH 1/4] feat: fetchApprovalRules, the light batched approval-rules query --- packages/glance/package.json | 2 +- packages/glance/src/GitLabProvider.ts | 73 ++++++++++++++++++++ packages/glance/src/index.ts | 3 + packages/glance/src/types.ts | 28 ++++++++ packages/glance/tests/approval-rules.test.ts | 63 +++++++++++++++++ 5 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 packages/glance/tests/approval-rules.test.ts diff --git a/packages/glance/package.json b/packages/glance/package.json index 956b587..b00fbac 100644 --- a/packages/glance/package.json +++ b/packages/glance/package.json @@ -1,6 +1,6 @@ { "name": "@mattstack/glance", - "version": "0.19.0", + "version": "0.20.0", "description": "GitHub and GitLab API client: REST, GraphQL, and real-time ActionCable subscriptions", "type": "module", "main": "dist/index.js", diff --git a/packages/glance/src/GitLabProvider.ts b/packages/glance/src/GitLabProvider.ts index 7d8df8a..461382a 100644 --- a/packages/glance/src/GitLabProvider.ts +++ b/packages/glance/src/GitLabProvider.ts @@ -6,10 +6,12 @@ import type { BranchProtectionRule, CreatePullRequestInput, DiffStats, + FetchApprovalRulesOptions, InvalidationBatch, JobDetail, MergeabilityCheck, MergePullRequestInput, + MRApprovalRules, MRDetail, Pipeline, PipelineJob, @@ -607,6 +609,39 @@ interface MRProjectResponse { } | null; } +/** Approval-rules discovery fragment: deliberately minimal. Wide queries + carrying per-MR dashboard resolvers are the timeout class the scoped + sync exists to avoid; this one is iid + rules only. */ +const MR_APPROVAL_RULES_QUERY = ` + query GlanceMRApprovalRules($projectPath: ID!, $ua: Time, $first: Int!, $after: String) { + project(fullPath: $projectPath) { + mergeRequests(state: opened, updatedAfter: $ua, draft: false, first: $first, after: $after, sort: UPDATED_DESC) { + pageInfo { hasNextPage endCursor } + nodes { iid approvalState { rules { type approved section } } } + } + } + } +`; + +const MR_APPROVAL_RULES_BY_IID_QUERY = ` + query GlanceMRApprovalRulesByIid($projectPath: ID!, $iids: [String!]) { + project(fullPath: $projectPath) { + mergeRequests(iids: $iids) { + nodes { iid approvalState { rules { type approved section } } } + } + } + } +`; + +interface ApprovalRulesResponse { + project: { + mergeRequests: { + pageInfo?: { hasNextPage: boolean; endCursor: string | null }; + nodes: Array<{ iid: string; approvalState: { rules: Array<{ type: string; approved: boolean; section: string | null }> } | null }>; + } | null; + } | null; +} + // --------------------------------------------------------------------------- // GitLabProvider // --------------------------------------------------------------------------- @@ -925,6 +960,44 @@ export class GitLabProvider implements GitProvider { return filterSet ? prs.filter((pr) => filterSet.has(pr.state as MRState)) : prs; } + async fetchApprovalRules(options: FetchApprovalRulesOptions): Promise { + const projectPath = options.projectPath; + const mapNodes = (resp: ApprovalRulesResponse): MRApprovalRules[] => + (resp.project?.mergeRequests?.nodes ?? []).map((n) => ({ + iid: Number(n.iid), + rules: n.approvalState?.rules ?? [], + })); + + if (options.iids) { + const resp = await this.runQuery( + 'fetchApprovalRules.iids', MR_APPROVAL_RULES_BY_IID_QUERY, + { projectPath, iids: options.iids.map(String) }, + ); + return mapNodes(resp); + } + + parseUpdatedAfter(options.updatedAfter); + const first = options.pageSize ?? 100; + const out: MRApprovalRules[] = []; + let after: string | null = null; + do { + const resp: ApprovalRulesResponse = await this.runQuery( + 'fetchApprovalRules.project', MR_APPROVAL_RULES_QUERY, + { projectPath, ua: options.updatedAfter ?? null, first, after }, + ); + out.push(...mapNodes(resp)); + const conn = resp.project?.mergeRequests; + const next = conn?.pageInfo?.hasNextPage ? (conn.pageInfo.endCursor ?? null) : null; + // Stricter than fetchPullRequests.project's guard on purpose: hasNextPage + // with a null or repeated cursor is an infinite loop either way. + if (conn?.pageInfo?.hasNextPage && (next === null || next === after)) { + throw new Error(`fetchApprovalRules.project: non-advancing cursor '${next}' for ${projectPath}`); + } + after = next; + } while (after); + return out; + } + async fetchMRDiscussions(repositoryId: string, mrIid: number): Promise { const projectId = parseGitLabRepoId(repositoryId); return this.mrDetailFetcher.fetchDetail(projectId, mrIid); diff --git a/packages/glance/src/index.ts b/packages/glance/src/index.ts index d56bbf7..5c8c6ad 100644 --- a/packages/glance/src/index.ts +++ b/packages/glance/src/index.ts @@ -25,6 +25,9 @@ type _ProvidersConform = ProviderParameterDrift; export type { PullRequest, PullRequestsSnapshot, + ApprovalRuleLite, + MRApprovalRules, + FetchApprovalRulesOptions, MergeabilityCheck, CreatePullRequestInput, UpdatePullRequestInput, diff --git a/packages/glance/src/types.ts b/packages/glance/src/types.ts index a61bb0b..62b7155 100644 --- a/packages/glance/src/types.ts +++ b/packages/glance/src/types.ts @@ -616,6 +616,34 @@ export interface PullRequestsSnapshot { items: PullRequest[]; } +/** One approval rule as returned by GitLab's `approvalState`. */ +export interface ApprovalRuleLite { + type: string; + approved: boolean; + section: string | null; +} + +/** Approval rules for a single MR, keyed by IID rather than global ID. */ +export interface MRApprovalRules { + iid: number; + rules: ApprovalRuleLite[]; +} + +/** + * Options for `GitLabProvider.fetchApprovalRules`. `updatedAfter` and `iids` + * are mutually exclusive: the former drives windowed discovery, the latter + * the targeted events-heal path. + */ +export interface FetchApprovalRulesOptions { + projectPath: string; + /** ISO timestamp; windowed mode. Mutually exclusive with iids. */ + updatedAfter?: string; + /** Targeted mode: just these MRs (the events-heal path). */ + iids?: number[]; + /** Page size, default 100. */ + pageSize?: number; +} + /** Feed event emitted as `feed_event` (incremental) or inside `feed_snapshot` (initial batch). */ export interface FeedEvent { /** Stable event ID, e.g. "note-1234" or "projEvent-5678". */ diff --git a/packages/glance/tests/approval-rules.test.ts b/packages/glance/tests/approval-rules.test.ts new file mode 100644 index 0000000..843cc9c --- /dev/null +++ b/packages/glance/tests/approval-rules.test.ts @@ -0,0 +1,63 @@ +#!/usr/bin/env bun +/** + * fetchApprovalRules: the light batched rules query. Wide discovery must + * never carry the heavy dashboard fragment (the a6601b1 wedge class), so + * this method has its own minimal query: iid + rules{type,approved,section}. + */ +import { describe, expect, test } from 'bun:test'; +import { GitLabProvider } from '../src/GitLabProvider.ts'; + +function stubRunQuery(provider: GitLabProvider, pages: any[]) { + const calls: Array<{ op: string; query: string; vars: any }> = []; + let i = 0; + (provider as any).runQuery = async (op: string, query: string, vars: any) => { + calls.push({ op, query, vars }); + return pages[Math.min(i++, pages.length - 1)]; + }; + return calls; +} + +const node = (iid: number, rules: any[]) => ({ iid: String(iid), approvalState: { rules } }); +const page = (nodes: any[], hasNext: boolean, cursor: string | null) => ({ + project: { mergeRequests: { pageInfo: { hasNextPage: hasNext, endCursor: cursor }, nodes } }, +}); + +describe('fetchApprovalRules', () => { + test('windowed mode paginates and maps iids to numbers', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + const calls = stubRunQuery(p, [ + page([node(1, [{ type: 'CODE_OWNER', approved: false, section: 'Acme' }])], true, 'c1'), + page([node(2, [{ type: 'REGULAR', approved: true, section: null }])], false, null), + ]); + const out = await p.fetchApprovalRules({ projectPath: 'g/p', updatedAfter: '2026-07-26T00:00:00Z' }); + expect(out).toEqual([ + { iid: 1, rules: [{ type: 'CODE_OWNER', approved: false, section: 'Acme' }] }, + { iid: 2, rules: [{ type: 'REGULAR', approved: true, section: null }] }, + ]); + expect(calls).toHaveLength(2); + expect(calls[0]!.op).toBe('fetchApprovalRules.project'); + expect(calls[0]!.vars).toMatchObject({ projectPath: 'g/p', ua: '2026-07-26T00:00:00Z', first: 100, after: null }); + expect(calls[1]!.vars.after).toBe('c1'); + // The wide query must stay light: no dashboard-fragment fields. + expect(calls[0]!.query).not.toContain('diffStatsSummary'); + expect(calls[0]!.query).toContain('draft: false'); + }); + + test('targeted mode queries by iids and omits the window', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + const calls = stubRunQuery(p, [ + page([node(7, [{ type: 'CODE_OWNER', approved: true, section: 'Acme' }])], false, null), + ]); + const out = await p.fetchApprovalRules({ projectPath: 'g/p', iids: [7] }); + expect(out).toEqual([{ iid: 7, rules: [{ type: 'CODE_OWNER', approved: true, section: 'Acme' }] }]); + expect(calls[0]!.op).toBe('fetchApprovalRules.iids'); + expect(calls[0]!.vars).toMatchObject({ projectPath: 'g/p', iids: ['7'] }); + }); + + test('non-advancing cursor throws instead of looping', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + stubRunQuery(p, [page([node(1, [])], true, null)]); + await expect(p.fetchApprovalRules({ projectPath: 'g/p', updatedAfter: '2026-07-26T00:00:00Z' })) + .rejects.toThrow(/non-advancing cursor/); + }); +}); From 4238af65d23ae1948e4b3f75fc056ad3daa1a54c Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 23:15:25 -0500 Subject: [PATCH 2/4] fix: chunk by-iid approval-rules queries at 100; reject mixed selectors Co-Authored-By: Claude Fable 5 --- packages/glance/src/GitLabProvider.ts | 20 +++++++++---- packages/glance/src/types.ts | 4 +-- packages/glance/tests/approval-rules.test.ts | 30 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/packages/glance/src/GitLabProvider.ts b/packages/glance/src/GitLabProvider.ts index 461382a..03fb5ba 100644 --- a/packages/glance/src/GitLabProvider.ts +++ b/packages/glance/src/GitLabProvider.ts @@ -961,6 +961,10 @@ export class GitLabProvider implements GitProvider { } async fetchApprovalRules(options: FetchApprovalRulesOptions): Promise { + if (options.updatedAfter && options.iids) { + throw new Error('fetchApprovalRules: updatedAfter and iids are mutually exclusive'); + } + const projectPath = options.projectPath; const mapNodes = (resp: ApprovalRulesResponse): MRApprovalRules[] => (resp.project?.mergeRequests?.nodes ?? []).map((n) => ({ @@ -969,11 +973,17 @@ export class GitLabProvider implements GitProvider { })); if (options.iids) { - const resp = await this.runQuery( - 'fetchApprovalRules.iids', MR_APPROVAL_RULES_BY_IID_QUERY, - { projectPath, iids: options.iids.map(String) }, - ); - return mapNodes(resp); + const out: MRApprovalRules[] = []; + const chunkSize = 100; + for (let i = 0; i < options.iids.length; i += chunkSize) { + const chunk = options.iids.slice(i, i + chunkSize).map(String); + const resp = await this.runQuery( + 'fetchApprovalRules.iids', MR_APPROVAL_RULES_BY_IID_QUERY, + { projectPath, iids: chunk }, + ); + out.push(...mapNodes(resp)); + } + return out; } parseUpdatedAfter(options.updatedAfter); diff --git a/packages/glance/src/types.ts b/packages/glance/src/types.ts index 62b7155..38a8819 100644 --- a/packages/glance/src/types.ts +++ b/packages/glance/src/types.ts @@ -636,9 +636,9 @@ export interface MRApprovalRules { */ export interface FetchApprovalRulesOptions { projectPath: string; - /** ISO timestamp; windowed mode. Mutually exclusive with iids. */ + /** ISO timestamp; windowed mode. Mutually exclusive with iids... enforced at runtime. */ updatedAfter?: string; - /** Targeted mode: just these MRs (the events-heal path). */ + /** Targeted mode: just these MRs (the events-heal path). Mutually exclusive with updatedAfter... enforced at runtime. */ iids?: number[]; /** Page size, default 100. */ pageSize?: number; diff --git a/packages/glance/tests/approval-rules.test.ts b/packages/glance/tests/approval-rules.test.ts index 843cc9c..4c50f84 100644 --- a/packages/glance/tests/approval-rules.test.ts +++ b/packages/glance/tests/approval-rules.test.ts @@ -60,4 +60,34 @@ describe('fetchApprovalRules', () => { await expect(p.fetchApprovalRules({ projectPath: 'g/p', updatedAfter: '2026-07-26T00:00:00Z' })) .rejects.toThrow(/non-advancing cursor/); }); + + test('chunks iids into batches of 100', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + const iids = Array.from({ length: 150 }, (_, i) => i + 1); + const calls = stubRunQuery(p, [ + page(iids.slice(0, 100).map((id) => node(id, [])), false, null), + page(iids.slice(100, 150).map((id) => node(id, [])), false, null), + ]); + const out = await p.fetchApprovalRules({ projectPath: 'g/p', iids }); + expect(out).toHaveLength(150); + expect(calls).toHaveLength(2); + expect(calls[0]!.vars.iids).toHaveLength(100); + expect(calls[1]!.vars.iids).toHaveLength(50); + expect(calls[0]!.vars.iids[0]).toBe('1'); + expect(calls[0]!.vars.iids[99]).toBe('100'); + expect(calls[1]!.vars.iids[0]).toBe('101'); + expect(calls[1]!.vars.iids[49]).toBe('150'); + }); + + test('rejects when both updatedAfter and iids are provided', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + stubRunQuery(p, [page([], false, null)]); + await expect( + p.fetchApprovalRules({ + projectPath: 'g/p', + updatedAfter: '2026-07-26T00:00:00Z', + iids: [1, 2, 3], + }), + ).rejects.toThrow(/updatedAfter and iids are mutually exclusive/); + }); }); From 958d7a31b64ef8f4f484c1bc215f6ad8d34bc3d3 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 23:32:30 -0500 Subject: [PATCH 3/4] fix: validate fetchApprovalRules pageSize before the request loop Co-Authored-By: Claude Fable 5 --- packages/glance/src/GitLabProvider.ts | 5 ++++ packages/glance/tests/approval-rules.test.ts | 28 ++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/glance/src/GitLabProvider.ts b/packages/glance/src/GitLabProvider.ts index 03fb5ba..b9baef0 100644 --- a/packages/glance/src/GitLabProvider.ts +++ b/packages/glance/src/GitLabProvider.ts @@ -987,6 +987,11 @@ export class GitLabProvider implements GitProvider { } parseUpdatedAfter(options.updatedAfter); + if (options.pageSize !== undefined) { + if (!Number.isInteger(options.pageSize) || !Number.isFinite(options.pageSize) || options.pageSize <= 0) { + throw new Error('fetchApprovalRules: pageSize must be a positive integer'); + } + } const first = options.pageSize ?? 100; const out: MRApprovalRules[] = []; let after: string | null = null; diff --git a/packages/glance/tests/approval-rules.test.ts b/packages/glance/tests/approval-rules.test.ts index 4c50f84..769d3d0 100644 --- a/packages/glance/tests/approval-rules.test.ts +++ b/packages/glance/tests/approval-rules.test.ts @@ -90,4 +90,32 @@ describe('fetchApprovalRules', () => { }), ).rejects.toThrow(/updatedAfter and iids are mutually exclusive/); }); + + test('validates pageSize: rejects non-positive, fractional, and non-finite values', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + stubRunQuery(p, [page([], false, null)]); + const invalidValues = [0, -1, -100, 1.5, 3.14, Infinity, -Infinity, NaN]; + for (const value of invalidValues) { + await expect( + p.fetchApprovalRules({ + projectPath: 'g/p', + updatedAfter: '2026-07-26T00:00:00Z', + pageSize: value, + }), + ).rejects.toThrow(/pageSize must be a positive integer/); + } + }); + + test('accepts valid pageSize like 200 and passes it through as first', async () => { + const p = new GitLabProvider('https://gitlab.example', 't'); + const calls = stubRunQuery(p, [ + page([node(1, [])], false, null), + ]); + await p.fetchApprovalRules({ + projectPath: 'g/p', + updatedAfter: '2026-07-26T00:00:00Z', + pageSize: 200, + }); + expect(calls[0]!.vars.first).toBe(200); + }); }); From 3168e0eb50b2620f58837957cc5a7e470bb83221 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 23:39:53 -0500 Subject: [PATCH 4/4] fix: mixed-selector guard checks presence, not truthiness Co-Authored-By: Claude Fable 5 --- packages/glance/src/GitLabProvider.ts | 2 +- packages/glance/tests/approval-rules.test.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/glance/src/GitLabProvider.ts b/packages/glance/src/GitLabProvider.ts index b9baef0..677f9d2 100644 --- a/packages/glance/src/GitLabProvider.ts +++ b/packages/glance/src/GitLabProvider.ts @@ -961,7 +961,7 @@ export class GitLabProvider implements GitProvider { } async fetchApprovalRules(options: FetchApprovalRulesOptions): Promise { - if (options.updatedAfter && options.iids) { + if (options.updatedAfter !== undefined && options.iids !== undefined) { throw new Error('fetchApprovalRules: updatedAfter and iids are mutually exclusive'); } diff --git a/packages/glance/tests/approval-rules.test.ts b/packages/glance/tests/approval-rules.test.ts index 769d3d0..5a8125b 100644 --- a/packages/glance/tests/approval-rules.test.ts +++ b/packages/glance/tests/approval-rules.test.ts @@ -89,6 +89,13 @@ describe('fetchApprovalRules', () => { iids: [1, 2, 3], }), ).rejects.toThrow(/updatedAfter and iids are mutually exclusive/); + await expect( + p.fetchApprovalRules({ + projectPath: 'g/p', + updatedAfter: '', + iids: [1, 2, 3], + }), + ).rejects.toThrow(/updatedAfter and iids are mutually exclusive/); }); test('validates pageSize: rejects non-positive, fractional, and non-finite values', async () => {