diff --git a/bun.lock b/bun.lock index 2d9e0f0..0e6027b 100644 --- a/bun.lock +++ b/bun.lock @@ -13,6 +13,7 @@ "eslint": "^9.39.1", "eslint-config-prettier": "10.1.8", "eslint-plugin-prettier": "^5.1.3", + "json-schema-to-ts": "^3.1.1", "prettier": "3.8.1", "solid-js": "^1.9.9", "typescript": "^5.7.0", @@ -42,7 +43,7 @@ }, "packages/forgekeeper": { "name": "opencode-forgekeeper", - "version": "0.0.0", + "version": "2.0.0", "peerDependencies": { "@opencode/plugin": "^2.0.26", "@opentui/core": ">=0.5.8", @@ -59,7 +60,7 @@ }, "packages/terminal-progress": { "name": "opencode-terminal-progress", - "version": "1.0.2", + "version": "1.1.0", "peerDependencies": { "@opencode/plugin": "^2.0.0", }, @@ -173,6 +174,8 @@ "@babel/preset-typescript": ["@babel/preset-typescript@7.27.1", "", { "dependencies": { "@babel/helper-plugin-utils": "^7.27.1", "@babel/helper-validator-option": "^7.27.1", "@babel/plugin-syntax-jsx": "^7.27.1", "@babel/plugin-transform-modules-commonjs": "^7.27.1", "@babel/plugin-transform-typescript": "^7.27.1" }, "peerDependencies": { "@babel/core": "^7.0.0-0" } }, "sha512-l7WfQfX0WK4M0v2RudjuQK4u99BS6yLHYEmdtVPP7lKV013zr9DygFuWNlnbvQ9LR+LS0Egz/XAvGx5U9MX0fQ=="], + "@babel/runtime": ["@babel/runtime@7.29.10", "", {}, "sha512-NpugTzbKgGrNe45Ui32cN8vCFFMxVtP/9rtfwv0nP5FRDAjvYBxJcLVpYA9mr1jxP6cnIT7x9LUzuDTbWYYNRA=="], + "@babel/template": ["@babel/template@7.29.7", "", { "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/parser": "^7.29.7", "@babel/types": "^7.29.7" } }, "sha512-puq+Gf35oI24FeN11LkoUQFqv9uwNeWpxXZi/Ji3rRIoKAzKnxRaZ+Gkj0vKS9ZCiTESfng1N9LyOyXvo+m+Gg=="], "@babel/traverse": ["@babel/traverse@7.29.10", "", { "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.8", "@babel/helper-globals": "^7.29.7", "@babel/parser": "^7.29.9", "@babel/template": "^7.29.7", "@babel/types": "^7.29.8", "debug": "^4.3.1" } }, "sha512-B3lYAs2nkKskbgQ1nfy8Ma9ao9Q1rntTAQHrzzyDuj+TSO+X3le6PfWwqDvodsb+OumE10eMFfuMCTQSQ5iuIQ=="], @@ -671,6 +674,8 @@ "json-schema": ["json-schema@0.4.0", "", {}, "sha512-es94M3nTIfsEPisRafak+HDLfHXnKBhV3vU5eqPcS3flIWqcxJWgXHXiey3YrpaNsanY5ei1VoYEbOzijuq9BA=="], + "json-schema-to-ts": ["json-schema-to-ts@3.1.1", "", { "dependencies": { "@babel/runtime": "^7.18.3", "ts-algebra": "^2.0.0" } }, "sha512-+DWg8jCJG2TEnpy7kOm/7/AxaYoaRbjVB4LFZLySZlWn8exGs3A4OLJR966cVvU26N7X9TWxl+Jsw7dzAqKT6g=="], + "json-schema-traverse": ["json-schema-traverse@0.4.1", "", {}, "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg=="], "json-stable-stringify-without-jsonify": ["json-stable-stringify-without-jsonify@1.0.1", "", {}, "sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw=="], @@ -905,6 +910,8 @@ "treeverse": ["treeverse@3.0.0", "", {}, "sha512-gcANaAnd2QDZFmHFEOF4k7uc1J/6a6z3DJMd/QwEyxLoKGiptJRwid582r7QIsFlFMIZ3SnxfS52S4hm2DHkuQ=="], + "ts-algebra": ["ts-algebra@2.0.0", "", {}, "sha512-FPAhNPFMrkwz76P7cdjdmiShwMynZYN6SgOujD1urY4oNm80Ou9oMdmbR45LotcKOXoy7wSmHkRFE6Mxbrhefw=="], + "ts-api-utils": ["ts-api-utils@2.4.0", "", { "peerDependencies": { "typescript": ">=4.8.4" } }, "sha512-3TaVTaAv2gTiMB35i3FiGJaRfwb3Pyn/j3m/bfAvGe8FB7CF6u+LMYqYlDh7reQf7UNvoTvdfAqHGmPGOSsPmA=="], "tslib": ["tslib@2.8.1", "", {}, "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w=="], diff --git a/package.json b/package.json index 93e6e7d..754521a 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "eslint": "^9.39.1", "eslint-config-prettier": "10.1.8", "eslint-plugin-prettier": "^5.1.3", + "json-schema-to-ts": "^3.1.1", "prettier": "3.8.1", "solid-js": "^1.9.9", "typescript": "^5.7.0", diff --git a/packages/forgekeeper/src/github.ts b/packages/forgekeeper/src/github.ts index 1813443..a479c8f 100644 --- a/packages/forgekeeper/src/github.ts +++ b/packages/forgekeeper/src/github.ts @@ -13,6 +13,7 @@ import { } from './forge'; import type { RemoteProject, Repository } from './git'; import { gitlabTraits } from './gitlab'; +import { query, typed, type Transport, type Typed } from './graphql'; interface Connection { nodes: T[]; @@ -61,11 +62,12 @@ const fields = `number title url state isDraft mergeable reviewDecision headRefO // Pagination stops after this many pages; results are then marked partial. const MAX_PAGES = 50; -export type GraphQL = ( - host: string, - query: string, - variables: Record, -) => Promise>; +type Variables = Record; +export type GraphQL = Transport; + +// Every query selects fields of one repository. A repository that doesn't +// exist, or that the viewer can't see, is null. +type RepositoryData = { repository?: T | null }; export function ghGraphQL(run: Exec, cwd: string): GraphQL { return async (host, query, variables) => { @@ -74,7 +76,7 @@ export function ghGraphQL(run: Exec, cwd: string): GraphQL { args.push(typeof value === 'number' ? '-F' : '-f', `${name}=${value}`); const result = await run('gh', args, cwd); let body: { - data?: Record; + data?: object; errors?: Array<{ type?: string; message: string }>; }; try { @@ -188,7 +190,11 @@ export const githubTraits: ForgeTraits = { export class GitHubForge implements Forge { readonly traits = githubTraits; - constructor(readonly graphql: GraphQL) {} + private readonly graphql: Typed; + + constructor(graphql: GraphQL) { + this.graphql = typed(graphql); + } async findByNumber( projects: readonly RemoteProject[], @@ -198,12 +204,14 @@ export class GitHubForge implements Forge { const [owner, repo] = project.path.split('/'); const data = await this.graphql( project.host, - `query($owner:String!,$repo:String!,$number:Int!) { + query>( + `query($owner:String!,$repo:String!,$number:Int!) { repository(owner:$owner,name:$repo) { pullRequest(number:$number) { ${fields} } } }`, + ), { owner, repo, number: Number(number) }, ); - const node = (data.repository as { pullRequest?: Pull } | undefined)?.pullRequest; + const node = data.repository?.pullRequest; if (node) return this.normalize(project.host, node); } return undefined; @@ -215,18 +223,17 @@ export class GitHubForge implements Forge { const [owner, repo] = target.path.split('/'); const data = await this.graphql( target.host, - `query($owner:String!,$repo:String!,$branch:String!) { + query< + RepositoryData<{ + pullRequests?: Connection> | null; + }> + >(`query($owner:String!,$repo:String!,$branch:String!) { repository(owner:$owner,name:$repo) { pullRequests(first:100,states:OPEN,headRefName:$branch,orderBy:{field:UPDATED_AT,direction:DESC}) { nodes { number updatedAt headRepository { nameWithOwner } } - } } }`, + } } }`), { owner, repo, branch: repository.sourceBranch }, ); - const nodes = - ( - data.repository as - | { pullRequests?: Connection> } - | undefined - )?.pullRequests?.nodes ?? []; + const nodes = data.repository?.pullRequests?.nodes ?? []; for (const node of nodes) { // A fork's branch with the same name must not name this branch's PR. if ( @@ -243,15 +250,14 @@ export class GitHubForge implements Forge { const [owner, repo] = ref.project.split('/'); const data = await this.graphql( ref.host, - `query($owner:String!,$repo:String!,$number:Int!) { + query>( + `query($owner:String!,$repo:String!,$number:Int!) { repository(owner:$owner,name:$repo) { pullRequest(number:$number) { headRefName } } }`, + ), { owner, repo, number: Number(ref.iid) }, ); - const branch = ( - data.repository as { pullRequest?: { headRefName?: unknown } | null } | undefined - )?.pullRequest?.headRefName; - return typeof branch === 'string' && branch ? branch : undefined; + return data.repository?.pullRequest?.headRefName || undefined; } async findByBranch(repository: Repository): Promise { @@ -263,14 +269,15 @@ export class GitHubForge implements Forge { for (let page = 0; page < MAX_PAGES; page++) { const data = await this.graphql( target.host, - `query($owner:String!,$repo:String!,$branch:String!${after ? ',$after:String!' : ''}) { + query | null }>>( + `query($owner:String!,$repo:String!,$branch:String!${after ? ',$after:String!' : ''}) { repository(owner:$owner,name:$repo) { pullRequests(first:100,states:OPEN,headRefName:$branch${after ? ',after:$after' : ''}) { nodes { ${fields} } pageInfo { hasNextPage endCursor } } } }`, + ), { owner, repo, branch: repository.sourceBranch, ...(after ? { after } : {}) }, ); - const pulls = (data.repository as { pullRequests?: Connection } | undefined) - ?.pullRequests; + const pulls = data.repository?.pullRequests; if (!pulls) throw new ForgeError('request', 'GitHub repository unavailable'); for (const node of pulls.nodes) { // A fork's branch with the same name must not be reported as this branch's PR. @@ -301,15 +308,17 @@ export class GitHubForge implements Forge { for (let count = 1; page?.hasNextPage && page.endCursor && count < MAX_PAGES; count++) { const data = await this.graphql( host, - `query($owner:String!,$repo:String!,$number:Int!,$after:String!) { + query< + RepositoryData<{ pullRequest?: { reviewThreads?: Pull['reviewThreads'] | null } | null }> + >( + `query($owner:String!,$repo:String!,$number:Int!,$after:String!) { repository(owner:$owner,name:$repo) { pullRequest(number:$number) { reviewThreads(first:100,after:$after) { nodes { isResolved } pageInfo { hasNextPage endCursor } } } } }`, + ), { owner, repo, number: node.number, after: page.endCursor }, ); - const next = ( - data.repository as { pullRequest?: { reviewThreads?: Pull['reviewThreads'] } } | undefined - )?.pullRequest?.reviewThreads; + const next = data.repository?.pullRequest?.reviewThreads; if (!next) break; threads.push(...next.nodes); page = next.pageInfo; @@ -328,17 +337,17 @@ export class GitHubForge implements Forge { for (let count = 1; cursor?.hasNextPage && cursor.endCursor && count < MAX_PAGES; count++) { const data = await this.graphql( host, - `query($owner:String!,$repo:String!,$oid:GitObjectID!,$after:String!) { + query< + RepositoryData<{ + object?: { statusCheckRollup?: { contexts?: Connection | null } | null } | null; + }> + >(`query($owner:String!,$repo:String!,$oid:GitObjectID!,$after:String!) { repository(owner:$owner,name:$repo) { object(oid:$oid) { ... on Commit { statusCheckRollup { contexts(first:100,after:$after) { nodes { __typename ... on CheckRun { status conclusion detailsUrl } ... on StatusContext { state targetUrl } } pageInfo { hasNextPage endCursor } } - } } } } }`, + } } } } }`), { owner, repo, oid: node.headRefOid, after: cursor.endCursor }, ); - const next = ( - data.repository as - | { object?: { statusCheckRollup?: { contexts?: Connection } } } - | undefined - )?.object?.statusCheckRollup?.contexts; + const next = data.repository?.object?.statusCheckRollup?.contexts; if (!next) break; contexts.push(...next.nodes); cursor = next.pageInfo; diff --git a/packages/forgekeeper/src/gitlab-feedback.ts b/packages/forgekeeper/src/gitlab-feedback.ts index ae5e4ac..80c8ca3 100644 --- a/packages/forgekeeper/src/gitlab-feedback.ts +++ b/packages/forgekeeper/src/gitlab-feedback.ts @@ -1,6 +1,7 @@ // Reads a GitLab MR's comments for GitLab's `feedback` capability. import type { FeedbackSnapshot, ReviewComment } from './forge'; import type { GraphQL } from './gitlab'; +import { query, typed } from './graphql'; interface NoteNode { id: string; @@ -20,7 +21,17 @@ export const MAX_COMMENT_PAGES = 20; // ONLY_COMMENTS leaves out system notes such as "added 1 commit". Pages run // backwards from the newest comment, in creation order within each page. -export const COMMENTS_QUERY = `query($project: ID!, $iid: String!, $before: String) { +export const COMMENTS_QUERY = query<{ + currentUser?: { id: string } | null; + project?: { + mergeRequest?: { + notes?: { + pageInfo: { hasPreviousPage: boolean; startCursor: string | null }; + nodes: NoteNode[]; + } | null; + } | null; + } | null; +}>(`query($project: ID!, $iid: String!, $before: String) { currentUser { id } project(fullPath: $project) { mergeRequest(iid: $iid) { @@ -34,7 +45,7 @@ export const COMMENTS_QUERY = `query($project: ID!, $iid: String!, $before: Stri } } } -}`; +}`); // GitLab's global ID prefix changes with the note's type, for example when a // reply turns a comment into a discussion, so only the number identifies it. @@ -66,11 +77,12 @@ export function toComment(node: NoteNode): ReviewComment | undefined { // Reads the MR's newest comments, oldest first, or undefined when the MR // doesn't exist. export async function fetchComments( - graphql: GraphQL, + transport: GraphQL, host: string, project: string, iid: string, ): Promise { + const graphql = typed(transport); const pages: ReviewComment[][] = []; let viewerID: string | undefined; let before: string | undefined; @@ -84,10 +96,10 @@ export async function fetchComments( const variables: Record = { project, iid }; if (before) variables.before = before; const data = await graphql(host, COMMENTS_QUERY, variables); - viewerID ??= data?.currentUser?.id ?? undefined; - const connection = data?.project?.mergeRequest?.notes; + viewerID ??= data.currentUser?.id ?? undefined; + const connection = data.project?.mergeRequest?.notes; if (!connection) return page === 0 ? undefined : snapshot(false); - pages.push((connection.nodes as NoteNode[]).flatMap((node) => toComment(node) ?? [])); + pages.push(connection.nodes.flatMap((node) => toComment(node) ?? [])); if (!connection.pageInfo.hasPreviousPage) return snapshot(true); before = connection.pageInfo.startCursor ?? undefined; if (!before) return snapshot(false); diff --git a/packages/forgekeeper/src/gitlab.test.ts b/packages/forgekeeper/src/gitlab.test.ts index f26027d..2d51a58 100644 --- a/packages/forgekeeper/src/gitlab.test.ts +++ b/packages/forgekeeper/src/gitlab.test.ts @@ -399,6 +399,16 @@ describe('glabGraphQL', () => { (stdout: string, stderr = '', code = 0): Exec => async () => ({ stdout, stderr, code }); + // The error that the lookup throws. + const failure = async (exec: Exec): Promise => { + try { + await glabGraphQL(exec, '/repo')('gitlab.com', 'q', {}); + } catch (error) { + return error as ForgeError; + } + throw new Error('Expected the lookup to fail'); + }; + test('passes variables as raw strings', async () => { let seen: string[] = []; const exec: Exec = async (_file, args) => { @@ -421,30 +431,28 @@ describe('glabGraphQL', () => { }); test('classifies failures', async () => { - const fail = (exec: Exec) => - glabGraphQL(exec, '/repo')('gitlab.com', 'q', {}).catch((error) => error); - expect((await fail(run('', 'glab: 401 Unauthorized', 1))).kind).toBe('auth'); - expect((await fail(run('', 'spawn glab ENOENT', -1))).kind).toBe('missing-glab'); - expect((await fail(run('', 'glab: 429 Too Many Requests', 1))).kind).toBe('rate-limit'); - const graphqlError = await fail(run('{"errors":[{"message":"Field missing"}]}', '', 1)); + expect((await failure(run('', 'glab: 401 Unauthorized', 1))).kind).toBe('auth'); + expect((await failure(run('', 'spawn glab ENOENT', -1))).kind).toBe('missing-glab'); + expect((await failure(run('', 'glab: 429 Too Many Requests', 1))).kind).toBe('rate-limit'); + const graphqlError = await failure(run('{"errors":[{"message":"Field missing"}]}', '', 1)); expect(graphqlError).toBeInstanceOf(ForgeError); expect(graphqlError.message).toBe('Field missing'); }); test('fails the lookup when a field errored, even next to data', async () => { - const fail = (exec: Exec) => - glabGraphQL(exec, '/repo')('gitlab.com', 'q', {}).catch((error) => error); // A timed-out field is `null` next to its error, which isn't a missing MR. const partial = '{"data":{"p0":null},"errors":[{"message":"request timed out","path":["p0"]}]}'; - expect(await fail(run(partial))).toMatchObject({ + expect(await failure(run(partial))).toMatchObject({ kind: 'request', message: 'request timed out', }); - expect(await fail(run('{"data":{"p0":null}}', 'glab: connection reset', 1))).toMatchObject({ + expect(await failure(run('{"data":{"p0":null}}', 'glab: connection reset', 1))).toMatchObject({ message: 'connection reset', }); - expect(await fail(run('{"data":null}'))).toMatchObject({ message: 'GitLab returned no data' }); - expect(await fail(run('[1, 2]'))).toBeInstanceOf(ForgeError); + expect(await failure(run('{"data":null}'))).toMatchObject({ + message: 'GitLab returned no data', + }); + expect(await failure(run('[1, 2]'))).toBeInstanceOf(ForgeError); // A project or MR that doesn't exist is `null` without an error. expect(await glabGraphQL(run('{"data":{"p0":null}}'), '/repo')('gitlab.com', 'q', {})).toEqual({ p0: null, diff --git a/packages/forgekeeper/src/gitlab.ts b/packages/forgekeeper/src/gitlab.ts index 2ad73d2..5957d32 100644 --- a/packages/forgekeeper/src/gitlab.ts +++ b/packages/forgekeeper/src/gitlab.ts @@ -14,6 +14,7 @@ import { } from './forge'; import type { RemoteProject, Repository } from './git'; import { fetchComments } from './gitlab-feedback'; +import { query, typed, type Query, type Transport, type Typed } from './graphql'; interface DiscussionPage { pageInfo: { hasNextPage: boolean; endCursor: string | null }; @@ -44,6 +45,13 @@ interface MergeRequestNode { reviewers?: { nodes: ReviewerNode[] } | null; } +// What `mergeRequestNumbersQuery` selects. +interface MergeRequestNumber { + iid: string; + updatedAt: string | null; + sourceProject: { fullPath: string } | null; +} + interface ReviewerNode { type: string; username?: string; @@ -65,7 +73,13 @@ const MERGE_REQUEST_FIELDS = ` discussions(first: ${PAGE_SIZE}) { ${DISCUSSIONS} } `; -export function mergeRequestsQuery(projectCount: number): string { +// Queries that look up several projects give each an alias, `p0`, `p1`, and +// so on. A project that doesn't exist, or that the viewer can't see, is null. +type Projects = { [alias: `p${number}`]: T | null | undefined }; + +export function mergeRequestsQuery( + projectCount: number, +): Query> { const variables = Array.from({ length: projectCount }, (_, index) => `$p${index}: ID!`).join( ', ', ); @@ -76,10 +90,12 @@ export function mergeRequestsQuery(projectCount: number): string { mergeRequests(sourceBranches: [$branch], state: opened, first: 20) { nodes { ${MERGE_REQUEST_FIELDS} } } }`, ).join('\n'); - return `query($branch: String!, ${variables}) {\n${projects}\n}`; + return query(`query($branch: String!, ${variables}) {\n${projects}\n}`); } -export function mergeRequestQuery(projectCount: number): string { +export function mergeRequestQuery( + projectCount: number, +): Query> { const variables = Array.from({ length: projectCount }, (_, index) => `$p${index}: ID!`).join( ', ', ); @@ -88,11 +104,13 @@ export function mergeRequestQuery(projectCount: number): string { (_, index) => `p${index}: project(fullPath: $p${index}) { mergeRequest(iid: $iid) { ${MERGE_REQUEST_FIELDS} } }`, ).join('\n'); - return `query($iid: String!, ${variables}) {\n${projects}\n}`; + return query(`query($iid: String!, ${variables}) {\n${projects}\n}`); } // Only the fields that pick the newest MR from the source project. -export function mergeRequestNumbersQuery(projectCount: number): string { +export function mergeRequestNumbersQuery( + projectCount: number, +): Query> { const variables = Array.from({ length: projectCount }, (_, index) => `$p${index}: ID!`).join( ', ', ); @@ -103,18 +121,22 @@ export function mergeRequestNumbersQuery(projectCount: number): string { mergeRequests(sourceBranches: [$branch], state: opened, first: 20) { nodes { iid updatedAt sourceProject { fullPath } } } }`, ).join('\n'); - return `query($branch: String!, ${variables}) {\n${projects}\n}`; + return query(`query($branch: String!, ${variables}) {\n${projects}\n}`); } -export const SOURCE_BRANCH_QUERY = `query($project: ID!, $iid: String!) { +export const SOURCE_BRANCH_QUERY = query<{ + project?: { mergeRequest?: { sourceBranch?: string | null } | null } | null; +}>(`query($project: ID!, $iid: String!) { project(fullPath: $project) { mergeRequest(iid: $iid) { sourceBranch } } -}`; +}`); -export const DISCUSSIONS_QUERY = `query($project: ID!, $iid: String!, $after: String) { +export const DISCUSSIONS_QUERY = query<{ + project?: { mergeRequest?: { discussions?: DiscussionPage | null } | null } | null; +}>(`query($project: ID!, $iid: String!, $after: String) { project(fullPath: $project) { mergeRequest(iid: $iid) { discussions(first: ${PAGE_SIZE}, after: $after) { ${DISCUSSIONS} } } } -}`; +}`); export function classifyError(message: string, code: number): ForgeError { const text = message.trim().replace(/^glab:\s*/, '') || `glab exited with code ${code}`; @@ -130,18 +152,11 @@ export function classifyError(message: string, code: number): ForgeError { return new ForgeError('request', text.split('\n')[0]); } -// The `data` of a GraphQL response. Each query casts its fields to the node -// types it requested. -export type GraphQLData = Record; - -export type GraphQL = ( - host: string, - query: string, - variables: Record, -) => Promise; +type Variables = Record; +export type GraphQL = Transport; interface GraphQLResponse { - data?: GraphQLData | null; + data?: object | null; errors?: { message?: unknown }[]; } @@ -238,7 +253,11 @@ function duoState(state: string): AutomatedReviewState { export class GitLabForge implements Forge { readonly traits = gitlabTraits; - constructor(readonly graphql: GraphQL) {} + private readonly graphql: Typed; + + constructor(graphql: GraphQL) { + this.graphql = typed(graphql); + } async findByBranch(repository: Repository): Promise { const { source, targets, sourceBranch } = repository; @@ -250,7 +269,7 @@ export class GitLabForge implements Forge { const seen = new Set(); const nodes: MergeRequestNode[] = []; targets.forEach((_, index) => { - for (const node of (data?.[`p${index}`]?.mergeRequests?.nodes ?? []) as MergeRequestNode[]) { + for (const node of data[`p${index}`]?.mergeRequests?.nodes ?? []) { // Another project's branch with the same name must not be reported as this branch's MR. if (node.sourceProject?.fullPath.toLowerCase() !== sourcePath) continue; const key = `${node.targetProject.fullPath}!${node.iid}`; @@ -275,11 +294,7 @@ export class GitLabForge implements Forge { ); const sourcePath = source.path.toLowerCase(); - const nodes: Array<{ - iid: string; - updatedAt: string | null; - sourceProject: { fullPath: string } | null; - }> = targets.flatMap((_, index) => data?.[`p${index}`]?.mergeRequests?.nodes ?? []); + const nodes = targets.flatMap((_, index) => data[`p${index}`]?.mergeRequests?.nodes ?? []); return ( nodes // Another project's branch with the same name must not name this branch's MR. @@ -293,7 +308,7 @@ export class GitLabForge implements Forge { project: ref.project, iid: ref.iid, }); - const branch = data?.project?.mergeRequest?.sourceBranch; + const branch = data.project?.mergeRequest?.sourceBranch; return typeof branch === 'string' && branch ? branch : undefined; } @@ -307,7 +322,7 @@ export class GitLabForge implements Forge { projects.forEach((project, index) => (variables[`p${index}`] = project.path)); const data = await this.graphql(host, mergeRequestQuery(projects.length), variables); for (let index = 0; index < projects.length; index++) { - const node: MergeRequestNode | undefined = data?.[`p${index}`]?.mergeRequest ?? undefined; + const node = data[`p${index}`]?.mergeRequest; if (node) return this.normalize(host, node); } return undefined; @@ -323,7 +338,7 @@ export class GitLabForge implements Forge { iid: node.iid, after: page.endCursor, }); - const next: DiscussionPage | undefined = data?.project?.mergeRequest?.discussions; + const next = data.project?.mergeRequest?.discussions; if (!next) break; count += countUnresolved(next); page = next.pageInfo; diff --git a/packages/forgekeeper/src/graphql.ts b/packages/forgekeeper/src/graphql.ts new file mode 100644 index 0000000..f7e551f --- /dev/null +++ b/packages/forgekeeper/src/graphql.ts @@ -0,0 +1,31 @@ +// Types for the GraphQL calls that `gh` and `glab` make. A query's text is +// paired with the shape of the response `data` it asks for, so each call site +// gets typed results, and the response shape sits next to the query that +// determines it. +declare const shape: unique symbol; + +export type Query = string & { readonly [shape]?: Data }; + +// The shape is a claim about what the server returns for this text: nothing +// checks it at runtime, so write it from the query's selection. +export const query = (text: string): Query => text as Query; + +// Runs a query and returns the response's `data`. The forges' transports can +// only vouch that it parsed, so they return `unknown`. +export type Transport = ( + host: string, + query: string, + variables: Variables, +) => Promise; + +export type Typed = ( + host: string, + query: Query, + variables: Variables, +) => Promise; + +// The one place that trusts a query's declared shape. +export const typed = + (transport: Transport): Typed => + async (host: string, query: Query, variables: Variables) => + (await transport(host, query, variables)) as Data; diff --git a/packages/forgekeeper/src/human-review-watch.test.ts b/packages/forgekeeper/src/human-review-watch.test.ts index 407dbab..64b4e23 100644 --- a/packages/forgekeeper/src/human-review-watch.test.ts +++ b/packages/forgekeeper/src/human-review-watch.test.ts @@ -14,6 +14,7 @@ import { type FeedbackLog, type SeenComments, } from './human-review-watch'; +import { until } from './testing'; const MINUTE = 60_000; const mr = { @@ -381,7 +382,7 @@ describe('createFeedbackWatcher', () => { now: () => clock, }); const first = watcher.check('ses_1', mr); - await Bun.sleep(0); + await until(() => sent.length === 1); clock += FETCH_INTERVAL; await watcher.check('ses_1', mr); release(); diff --git a/packages/forgekeeper/src/limit.test.ts b/packages/forgekeeper/src/limit.test.ts index c775424..bf58c06 100644 --- a/packages/forgekeeper/src/limit.test.ts +++ b/packages/forgekeeper/src/limit.test.ts @@ -1,12 +1,24 @@ import { describe, expect, test } from 'bun:test'; import type { Exec } from './exec'; import { createLimiter, limitExec, mapLimit } from './limit'; +import { deferred, until, type Deferred } from './testing'; describe('mapLimit', () => { test('keeps the order of the items', async () => { - expect( - await mapLimit([3, 1, 2], 2, async (value) => (await Bun.sleep(value), value * 10)), - ).toEqual([30, 10, 20]); + // The tasks finish in the order 1, 2, 3, which isn't the order of the items. + const gates = new Map([3, 1, 2].map((value) => [value, deferred()])); + const started: number[] = []; + const mapped = mapLimit([3, 1, 2], 2, async (value) => { + started.push(value); + await gates.get(value)!.promise; + return value * 10; + }); + await until(() => started.length === 2); + gates.get(1)!.resolve(); + await until(() => started.length === 3); + gates.get(2)!.resolve(); + gates.get(3)!.resolve(); + expect(await mapped).toEqual([30, 10, 20]); expect(await mapLimit([], 4, async (value) => value)).toEqual([]); }); }); @@ -17,19 +29,28 @@ describe('createLimiter', () => { let running = 0; let peak = 0; const order: number[] = []; + const gates = new Map(); const task = (id: number) => limited(async () => { peak = Math.max(peak, ++running); order.push(id); - await Bun.sleep(1); + gates.set(id, deferred()); + await gates.get(id)!.promise; running--; // A task queued as a slot frees up must still wait its turn. if (id === 1) void task(99); }); - await Promise.all([1, 2, 3, 4, 5].map(task)); - await Bun.sleep(5); + const done = Promise.all([1, 2, 3, 4, 5].map(task)); + // Each task finishes once it has started, so every one that follows + // starts as a slot frees up. + for (const id of [1, 2, 3, 4, 5, 99]) { + await until(() => gates.has(id)); + gates.get(id)!.resolve(); + } + await done; + await until(() => running === 0); expect(peak).toBe(2); - expect(order.slice(0, 5)).toEqual([1, 2, 3, 4, 5]); + expect(order).toEqual([1, 2, 3, 4, 5, 99]); }); test('frees the slot of a failed task', async () => { @@ -43,14 +64,22 @@ describe('limitExec', () => { test('shares one limit across every command', async () => { let running = 0; let peak = 0; + const gates: Deferred[] = []; const run: Exec = async () => { peak = Math.max(peak, ++running); - await Bun.sleep(1); + const gate = deferred(); + gates.push(gate); + await gate.promise; running--; return { stdout: '', stderr: '', code: 0 }; }; const limited = limitExec(run); - await Promise.all(Array.from({ length: 50 }, () => limited('glab', [], '/repo'))); + const done = Promise.all(Array.from({ length: 50 }, () => limited('glab', [], '/repo'))); + for (let finished = 0; finished < 50; finished++) { + await until(() => gates.length > finished); + gates[finished].resolve(); + } + await done; expect(peak).toBe(4); }); }); @@ -68,7 +97,7 @@ describe('limitExec cancellation', () => { const limited = limitExec(run, 1, stopped.signal); const first = limited('first', [], '/repo'); const queued = limited('queued', [], '/repo'); - await Bun.sleep(0); + await until(() => ran.length === 1); stopped.abort(); finish(); expect((await first).code).toBe(0); diff --git a/packages/forgekeeper/src/locate.test.ts b/packages/forgekeeper/src/locate.test.ts index 61901dd..64f5df9 100644 --- a/packages/forgekeeper/src/locate.test.ts +++ b/packages/forgekeeper/src/locate.test.ts @@ -10,6 +10,7 @@ import { kinds, traits, type ForgeCatalog } from './forges'; import type { RemoteProject, Repository, RepositoryLookup } from './git'; import { locate } from './locate'; import { classifyTargets } from './target'; +import { deferred, until, type Deferred } from './testing'; // Finds `found` by its own number only, like a forge that has just that PR/MR. class FakeForge implements Forge { @@ -99,10 +100,13 @@ const mergeOf = (iid: string, overrides: Partial = {}) => }) as GitLabMergeRequest; // Finds requests by number, fails for the numbers in `failing`, and records -// how many lookups ran at once. +// how many lookups ran at once. With `hold` set, each lookup waits in `gates` +// until the test releases it. class FailingForge extends FakeForge { peak = 0; - private running = 0; + hold = false; + readonly gates: Deferred[] = []; + running = 0; constructor( private readonly requests: Map, @@ -113,7 +117,11 @@ class FailingForge extends FakeForge { override async findByNumber(_projects: readonly RemoteProject[], number: string) { this.peak = Math.max(this.peak, ++this.running); - await new Promise((resolve) => setTimeout(resolve, 1)); + if (this.hold) { + const gate = deferred(); + this.gates.push(gate); + await gate.promise; + } this.running--; if (this.failing.has(number)) throw new Error('glab failed'); return this.requests.get(number); @@ -250,7 +258,15 @@ describe('locate', () => { new Map(Array.from({ length: 10 }, (_, index) => [`${index + 1}`, mergeOf(`${index + 1}`)])), ); const urls = targets(...Array.from({ length: 10 }, (_, index) => mergeOf(`${index + 1}`).url)); - const lookup = await locate(catalog({ gitlab }), { directory: '/repo', targets: urls }); + gitlab.hold = true; + const located = locate(catalog({ gitlab }), { directory: '/repo', targets: urls }); + // The lookups fill the limit, and the rest wait for slots. + await until(() => gitlab.running === 4); + for (let released = 0; released < 10; released++) { + await until(() => gitlab.gates.length > released); + gitlab.gates[released].resolve(); + } + const lookup = await located; expect(lookup).toMatchObject({ kind: 'found' }); expect(lookup.kind === 'found' && lookup.requests.map((request) => request.iid)).toEqual([ '1', diff --git a/packages/forgekeeper/src/rpc.ts b/packages/forgekeeper/src/rpc.ts index d364d63..017952b 100644 --- a/packages/forgekeeper/src/rpc.ts +++ b/packages/forgekeeper/src/rpc.ts @@ -5,6 +5,13 @@ // Status keys are session IDs, or "" for the checkout outside a session. A // key is relative to the location the call or event names, because each // checkout's server plugin instance watches its own sessions. +// +// The schemas are plain JSON Schema, so OpenCode types their values as +// `unknown`. The types at the bottom of this file derive from them instead, +// so the schemas stay the single description of what crosses the RPC. +import type { FromSchema, JSONSchema } from 'json-schema-to-ts'; +import type { Snapshot } from './store'; + const urls = { url: { type: 'string' }, issueUrl: { type: 'string' }, @@ -25,13 +32,15 @@ const target = { } as const; const key = { type: 'string' } as const; -// A `Snapshot` from src/store.ts. -const snapshot = { type: 'object' } as const; +// A `Snapshot` from src/store.ts. Its title lets `Deserialize` map it to that +// type, so the schema doesn't repeat the lookup's shape. +const snapshot = { type: 'object', title: 'Snapshot' } as const; // `enabled` is false when the plugin's `reviewStatus` option is off. const status = { type: 'object', properties: { enabled: { type: 'boolean' }, snapshot }, required: ['enabled', 'snapshot'], + additionalProperties: false, } as const; export const ForgeSessionRpc = { @@ -63,8 +72,12 @@ export const ForgeSessionRpc = { }, output: { type: 'object', - properties: { enabled: { type: 'boolean' }, statuses: { type: 'object' } }, + properties: { + enabled: { type: 'boolean' }, + statuses: { type: 'object', additionalProperties: snapshot }, + }, required: ['enabled', 'statuses'], + additionalProperties: false, }, }, // Drops every lease the CLI holds, when it exits. @@ -75,7 +88,7 @@ export const ForgeSessionRpc = { required: ['clientID'], additionalProperties: false, }, - output: { type: 'object' }, + output: { type: 'object', additionalProperties: false }, }, // Looks the key up now and returns the result. refresh: { @@ -104,6 +117,7 @@ export const ForgeSessionRpc = { type: 'object', properties: { directory: { type: 'string' }, key, snapshot }, required: ['directory', 'key', 'snapshot'], + additionalProperties: false, }, }, // A toast for the CLIs, such as after telling a session about a review. @@ -118,6 +132,7 @@ export const ForgeSessionRpc = { variant: { type: 'string', enum: ['info', 'error'] }, }, required: ['message', 'variant'], + additionalProperties: false, }, }, }, @@ -130,11 +145,28 @@ export const HOME = ''; export const HEARTBEAT = 60_000; export const LEASE_TIME = 3 * HEARTBEAT; -export type TargetUrls = { url: string; issueUrl?: string }; -export type TargetOutput = { url?: string; issueUrl?: string; targets?: TargetUrls[] }; -export type Notice = { - sessionID?: string; - title?: string; - message: string; - variant: 'info' | 'error'; -}; +type Methods = typeof ForgeSessionRpc.methods; +type Events = typeof ForgeSessionRpc.events; +type Deserialize = { pattern: { type: 'object'; title: 'Snapshot' }; output: Snapshot }; +type Typed = FromSchema; + +export type MethodName = keyof Methods; +export type EventName = keyof Events; +export type Input = Typed; +export type Output = Typed; +export type EventData = Typed; + +export type TargetOutput = Output<'target'>; +export type TargetUrls = NonNullable[number]; +export type Notice = EventData<'notice'>; + +export type Handlers = { [M in MethodName]: (input: Input) => Promise> }; + +// OpenCode checks each call against the schemas before it reaches a handler, +// and types the input as `unknown`. This is the one place that narrows it. +export const implement = (handlers: Handlers) => ({ + target: (input: unknown) => handlers.target(input as Input<'target'>), + watch: (input: unknown) => handlers.watch(input as Input<'watch'>), + release: (input: unknown) => handlers.release(input as Input<'release'>), + refresh: (input: unknown) => handlers.refresh(input as Input<'refresh'>), +}); diff --git a/packages/forgekeeper/src/server.test.ts b/packages/forgekeeper/src/server.test.ts index 9516d17..470d7f9 100644 --- a/packages/forgekeeper/src/server.test.ts +++ b/packages/forgekeeper/src/server.test.ts @@ -14,6 +14,7 @@ import type { TitleLookups } from './lookups'; import { FETCH_INTERVAL, SETTLE_TIME } from './human-review-watch'; import { setupServer } from './server'; import type { Snapshot } from './store'; +import { flush } from './testing'; type Tool = { execute: (input: unknown, context: { sessionID: string }) => Promise }; type Hook = (event: { @@ -131,9 +132,9 @@ const defaultSessions = () => ], ]); -const settle = async () => { - for (let index = 0; index < 5; index++) await Bun.sleep(1); -}; +// Lets the plugin finish the work that the doubles' promises release. None of +// that work waits for a timer, so one turn of the event loop is enough. +const settle = flush; // Where the server stores a pending automated-review notification. const pendingRecord = (sessionID: string, url: string) => diff --git a/packages/forgekeeper/src/server.ts b/packages/forgekeeper/src/server.ts index 19301a1..6d21ea9 100644 --- a/packages/forgekeeper/src/server.ts +++ b/packages/forgekeeper/src/server.ts @@ -2,7 +2,7 @@ import type { Plugin } from '@opencode/plugin'; import { CONCURRENCY, mapLimit } from './limit'; import type { TitleLookups } from './lookups'; import { statusOptions } from './options'; -import { ForgeSessionRpc } from './rpc'; +import { ForgeSessionRpc, implement } from './rpc'; import { setupStatus, type Status, type StatusDependencies } from './status-server'; import type { Snapshot } from './store'; import { @@ -68,7 +68,7 @@ async function drain(tasks: Promise[], limit: number) { } // RPC values must be JSON, which has no `undefined`, such as a cleared error. -const json = (value: unknown) => JSON.parse(JSON.stringify(value)) as never; +const json = (value: T): T => JSON.parse(JSON.stringify(value)); // Stores each root session's targets, keeps its title prefix current, serves // the targets to the CLI over RPC, and, given `status`, watches the PRs/MRs @@ -119,32 +119,22 @@ export async function setupServer( }; let watcher: Status | undefined; - const rpc = await ctx.rpc.register(ForgeSessionRpc, { - target: async (input) => { - const { sessionID } = input as { sessionID: string }; - return targetOutput((await stateFor(sessionID)).targets); - }, - watch: async (input) => { - const { clientID, keys, visible } = input as { - clientID: string; - keys: string[]; - visible?: string; - }; - return watcher - ? { enabled: true, statuses: json(watcher.service.watch(clientID, keys, visible)) } - : { enabled: false, statuses: {} }; - }, - release: async (input) => { - watcher?.service.release((input as { clientID: string }).clientID); - return {}; - }, - refresh: async (input) => { - const { key } = input as { key: string }; - return watcher - ? { enabled: true, snapshot: json(await watcher.service.refresh(key)) } - : DISABLED; - }, - }); + const rpc = await ctx.rpc.register( + ForgeSessionRpc, + implement({ + target: async ({ sessionID }) => targetOutput((await stateFor(sessionID)).targets), + watch: async ({ clientID, keys, visible }) => + watcher + ? { enabled: true, statuses: json(watcher.service.watch(clientID, keys, visible)) } + : { enabled: false, statuses: {} }, + release: async ({ clientID }) => { + watcher?.service.release(clientID); + return {}; + }, + refresh: async ({ key }) => + watcher ? { enabled: true, snapshot: json(await watcher.service.refresh(key)) } : DISABLED, + }), + ); const settings = statusOptions(ctx.options); if (status && settings.enabled) { diff --git a/packages/forgekeeper/src/status-client.test.ts b/packages/forgekeeper/src/status-client.test.ts index 3766604..e74cb55 100644 --- a/packages/forgekeeper/src/status-client.test.ts +++ b/packages/forgekeeper/src/status-client.test.ts @@ -1,7 +1,8 @@ import { describe, expect, test } from 'bun:test'; -import { HOME } from './rpc'; +import { HOME, type Output } from './rpc'; import { createStatusClient } from './status-client'; import type { Snapshot } from './store'; +import { flush } from './testing'; const found = (iid: string): Snapshot => ({ loading: false, @@ -28,7 +29,7 @@ function harness() { }, refresh: async (directory, key) => ({ enabled: enabled.get(directory) ?? true, - snapshot: statuses.get(directory)?.[key], + snapshot: statuses.get(directory)?.[key] ?? { loading: false }, }), release: async (directory) => void released.push(directory), onChange: () => {}, @@ -43,7 +44,7 @@ function harness() { released, timers, fail: (value: boolean) => void (failing = value), - settle: () => Bun.sleep(0), + settle: () => flush(), }; } @@ -126,7 +127,7 @@ describe('createStatusClient', () => { }); test('keeps the newest snapshot when an older RPC response arrives after an event', async () => { - let answer = (_value: { enabled: boolean; statuses: Record }) => {}; + let answer = (_value: Output<'watch'>) => {}; const client = createStatusClient({ clientID: 'cli', watch: () => new Promise((resolve) => (answer = resolve)), @@ -136,11 +137,11 @@ describe('createStatusClient', () => { }); const place = { directory: '/a', key: 'ses_1' }; client.show(place); - await Bun.sleep(0); + await flush(); const done = { ...found('1'), revision: 12 }; client.receive('/a', 'ses_1', done); answer({ enabled: true, statuses: { ses_1: { loading: true, revision: 11 } } }); - await Bun.sleep(0); + await flush(); expect(client.snapshot(place)).toEqual(done); // A newer response replaces it, and one without a revision never does. client.receive('/a', 'ses_1', { loading: false }); diff --git a/packages/forgekeeper/src/status-client.ts b/packages/forgekeeper/src/status-client.ts index 5d8757e..3c546dc 100644 --- a/packages/forgekeeper/src/status-client.ts +++ b/packages/forgekeeper/src/status-client.ts @@ -1,4 +1,4 @@ -import { HEARTBEAT, HOME } from './rpc'; +import { HEARTBEAT, HOME, type Input, type Output } from './rpc'; import type { Snapshot } from './store'; // Where a status lives: the server plugin instance of `directory`, and a @@ -13,11 +13,8 @@ const placeID = (place: Place) => JSON.stringify([place.directory, place.key]); export interface StatusClientOptions { clientID: string; - watch: ( - directory: string, - input: { clientID: string; keys: string[]; visible?: string }, - ) => Promise<{ enabled: boolean; statuses: Record }>; - refresh: (directory: string, key: string) => Promise<{ enabled: boolean; snapshot: unknown }>; + watch: (directory: string, input: Input<'watch'>) => Promise>; + refresh: (directory: string, key: string) => Promise>; release: (directory: string, clientID: string) => Promise; // Called after any snapshot or setting changes, so the UI can render again. onChange: () => void; @@ -107,7 +104,7 @@ export function createStatusClient(options: StatusClientOptions) { failures.delete(directory); enabled.set(directory, result.enabled); for (const [key, snapshot] of Object.entries(result.statuses)) { - if (isShown({ directory, key })) apply({ directory, key }, snapshot as Snapshot); + if (isShown({ directory, key })) apply({ directory, key }, snapshot); } options.onChange(); } catch (error) { @@ -161,7 +158,7 @@ export function createStatusClient(options: StatusClientOptions) { try { const result = await options.refresh(place.directory, place.key); enabled.set(place.directory, result.enabled); - apply(place, result.snapshot as Snapshot); + apply(place, result.snapshot); options.onChange(); } catch (error) { unavailable(place.directory, error); diff --git a/packages/forgekeeper/src/status-service.test.ts b/packages/forgekeeper/src/status-service.test.ts index 15ea184..91b542d 100644 --- a/packages/forgekeeper/src/status-service.test.ts +++ b/packages/forgekeeper/src/status-service.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import type { GitLabMergeRequest, ReviewRequest } from './forge'; import { createStatusService, HOME } from './status-service'; import type { Lookup } from './store'; +import { flush } from './testing'; const mr = (overrides: Partial = {}): GitLabMergeRequest => ({ @@ -79,10 +80,10 @@ function harness({ clock = due[1].at; timers.delete(due[0]); due[1].callback(); - await Bun.sleep(0); + await flush(); } clock = end; - await Bun.sleep(0); + await flush(); }, }; } diff --git a/packages/forgekeeper/src/store.test.ts b/packages/forgekeeper/src/store.test.ts index ee80542..3a2d667 100644 --- a/packages/forgekeeper/src/store.test.ts +++ b/packages/forgekeeper/src/store.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import type { Repository } from './git'; import { ForgeError, type ReviewRequest } from './forge'; import { createStatusStore, type Lookup } from './store'; +import { flush } from './testing'; const repository = { head: '', @@ -53,7 +54,7 @@ function harness( timers.delete(id); timer.callback(); } - await Bun.sleep(0); + await flush(); }, }; } @@ -293,7 +294,7 @@ describe('createStatusStore', () => { const first = store.refresh('/a'); store.invalidate('/a'); resolvers[0](found(2)); - await Bun.sleep(0); + await flush(); expect(loaded).toEqual([]); expect(store.get('/a').lookup).toBeUndefined(); resolvers[1](found()); @@ -342,7 +343,7 @@ describe('createStatusStore', () => { const first = store.refresh('/a'); store.invalidate('/a'); resolve(found()); - await Bun.sleep(0); + await flush(); expect(calls).toBe(2); resolve({ kind: 'none', reason: 'Detached HEAD' }); await first; diff --git a/packages/forgekeeper/src/testing.ts b/packages/forgekeeper/src/testing.ts new file mode 100644 index 0000000..e2aa374 --- /dev/null +++ b/packages/forgekeeper/src/testing.ts @@ -0,0 +1,36 @@ +// Helpers for tests that need to control when asynchronous work finishes. + +export interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +} + +// A promise that the test settles itself, which holds work in flight until +// the test has seen what it needs to check. +export function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((done, fail) => { + resolve = done; + reject = fail; + }); + return { promise, resolve, reject }; +} + +// Lets every promise reaction that is already queued run, including those +// that the reactions queue in turn. It waits for no time, and for nothing +// that needs a timer or I/O, so it makes tests deterministic only for work +// built from promises. Prefer `until` when the test knows what to wait for. +export const flush = () => new Promise((resolve) => setImmediate(resolve)); + +// Waits until the condition holds. Gives up after a number of turns instead of +// an amount of time, so a slow machine can't fail it, and a condition that +// never comes true fails fast with a clear error. +export async function until(condition: () => boolean, turns = 1_000): Promise { + for (let turn = 0; turn < turns; turn++) { + if (condition()) return; + await flush(); + } + throw new Error(`Condition not met after ${turns} turns: ${condition}`); +} diff --git a/packages/forgekeeper/src/tui.tsx b/packages/forgekeeper/src/tui.tsx index 02628d0..a0ea7e4 100644 --- a/packages/forgekeeper/src/tui.tsx +++ b/packages/forgekeeper/src/tui.tsx @@ -17,7 +17,7 @@ import { } from './format'; import type { ReviewRequest } from './forge'; import { reference } from './forges'; -import { ForgeSessionRpc, HEARTBEAT, HOME, type Notice } from './rpc'; +import { ForgeSessionRpc, HEARTBEAT, HOME, type EventData, type Output } from './rpc'; import { createStatusClient, samePlace, type Place } from './status-client'; import type { Snapshot } from './store'; @@ -29,16 +29,12 @@ export default { const [version, setVersion] = createSignal(0); const client = createStatusClient({ clientID: crypto.randomUUID(), + // OpenCode types the results as `unknown`; the server returns what the + // schemas in rpc.ts describe. watch: async (directory, input) => - (await rpc.watch(input, { location: { directory } })) as { - enabled: boolean; - statuses: Record; - }, + (await rpc.watch(input, { location: { directory } })) as Output<'watch'>, refresh: async (directory, key) => - (await rpc.refresh({ key }, { location: { directory } })) as { - enabled: boolean; - snapshot: unknown; - }, + (await rpc.refresh({ key }, { location: { directory } })) as Output<'refresh'>, release: (directory, clientID) => rpc.release({ clientID }, { location: { directory } }), onChange: () => setVersion((value) => value + 1), }); @@ -382,16 +378,12 @@ export default { const stops = [ rpc.events.on('status', (event) => { - const { directory, key, snapshot } = event.data as { - directory: string; - key: string; - snapshot: Snapshot; - }; + const { directory, key, snapshot } = event.data as EventData<'status'>; client.receive(directory, key, snapshot); }), // Notices about a session go to the CLIs that have shown it. rpc.events.on('notice', (event) => { - const notice = event.data as Notice; + const notice = event.data as EventData<'notice'>; if (notice.sessionID && !client.hasShown(notice.sessionID)) return; context.ui.toast.show({ ...(notice.title ? { title: notice.title } : {}),