diff --git a/packages/adapter-github/package.json b/packages/adapter-github/package.json index b56493e..8d8c134 100644 --- a/packages/adapter-github/package.json +++ b/packages/adapter-github/package.json @@ -12,7 +12,8 @@ "test": "vitest run" }, "dependencies": { - "@aipm/core": "workspace:*" + "@aipm/core": "workspace:*", + "zod": "^4.4.3" }, "devDependencies": { "@aipm/tsconfig": "workspace:*", diff --git a/packages/adapter-github/src/adapter.ts b/packages/adapter-github/src/adapter.ts index b51c237..3586758 100644 --- a/packages/adapter-github/src/adapter.ts +++ b/packages/adapter-github/src/adapter.ts @@ -22,6 +22,13 @@ import { } from "./normalize.js"; import { GET_ISSUE, GET_PULL_REQUEST, LIST_THREADS_BY_REPO } from "./queries.js"; import { ghRest, type GhRestOptions } from "./rest.js"; +import { + repoNodeDataSchema, + repoThreadsDataSchema, + type GraphqlNode, + type GraphqlTimelineNode, +} from "./graphql-schema.js"; +import { z } from "zod"; export interface GitHubAdapterConfig { /** Installation token, or a provider that mints/caches one (see auth.ts). */ @@ -46,7 +53,7 @@ const COMMENTS_PAGE_SIZE = 100; */ export class GitHubAdapter implements Platform { readonly id = "github" as const; - private readonly rawByNativeId = new Map>(); + private readonly rawByNativeId = new Map(); constructor(private readonly config: GitHubAdapterConfig) {} @@ -105,10 +112,11 @@ export class GitHubAdapter implements Platform { acc: [], }; return asyncUnfold(seed, async (state) => { - const dataResult = await ghGraphQL( + const dataResult = await ghGraphQL( token.data, LIST_THREADS_BY_REPO, { owner, repo, issuesAfter: state.issuesAfter, prsAfter: state.prsAfter }, + repoThreadsDataSchema, { apiBaseUrl: this.config.apiBaseUrl, fetchImpl: this.config.fetchImpl }, ); if (!dataResult.ok) return { kind: "STOP", value: dataResult }; @@ -158,11 +166,12 @@ export class GitHubAdapter implements Platform { const { owner, repo, number } = parsedNativeId.data; const token = await this.resolveToken(); if (!token.ok) return token; - const response = await ghRest<{ url: string }>( + const response = await ghRest( token.data, "POST", `/repos/${owner}/${repo}/issues/${number}/comments`, { body }, + z.object({ url: z.string() }), this.restOpts(), ); if (!response.ok) return response; @@ -173,7 +182,14 @@ export class GitHubAdapter implements Platform { async editMessage(messageId: string, body: string) { const token = await this.resolveToken(); if (!token.ok) return token; - const edited = await ghRest(token.data, "PATCH", messageId, { body }, this.restOpts()); + const edited = await ghRest( + token.data, + "PATCH", + messageId, + { body }, + z.unknown(), + this.restOpts(), + ); if (!edited.ok) return edited; return Ok(undefined); } @@ -187,11 +203,12 @@ export class GitHubAdapter implements Platform { if (!token.ok) return token; const seed = 1; return asyncUnfold(seed, async (page) => { - const commentsResult = await ghRest>( + const commentsResult = await ghRest( token.data, "GET", `/repos/${owner}/${repo}/issues/${number}/comments?per_page=${COMMENTS_PAGE_SIZE}&page=${page}`, undefined, + z.array(z.object({ url: z.string(), body: z.string().optional() })), this.restOpts(), ); if (!commentsResult.ok) return { kind: "STOP", value: commentsResult }; @@ -213,6 +230,7 @@ export class GitHubAdapter implements Platform { "POST", `${messageId}/reactions`, { content: emoji }, + z.unknown(), this.restOpts(), ); if (!reacted.ok) return reacted; @@ -244,17 +262,18 @@ export class GitHubAdapter implements Platform { const seed: FetchNodeState = { cursor: undefined, acc: [] }; return asyncUnfold(seed, async (state) => { - const dataResult = await ghGraphQL( + const dataResult = await ghGraphQL( token.data, query, { owner, repo, number, timelineCount: TIMELINE_PAGE, afterTimeline: state.cursor }, + repoNodeDataSchema, { apiBaseUrl: this.config.apiBaseUrl, fetchImpl: this.config.fetchImpl }, ); if (!dataResult.ok) return { kind: "STOP", value: dataResult }; const data = dataResult.data; - const fetched = data.repository?.[field] as Record | null | undefined; + const fetched = data.repository?.[field]; if (!fetched) return { kind: "STOP", value: Ok(undefined) }; - const ti = fetched.timelineItems as TimelineConn | undefined; + const ti = fetched.timelineItems; const acc = [...state.acc, ...(ti?.nodes ?? [])]; const cursor = ti?.pageInfo?.hasNextPage ? ti.pageInfo.endCursor : undefined; if (cursor) return { kind: "CONTINUE", next: { cursor, acc } }; @@ -278,24 +297,6 @@ export class GitHubAdapter implements Platform { // --- shapes + helpers --------------------------------------------------------- -interface TimelineConn { - nodes?: Array; - pageInfo?: { hasNextPage?: boolean; endCursor?: string }; -} -interface RepoNodeData { - repository?: Record | null; -} -interface RepoConn { - nodes?: Array; - pageInfo?: { hasNextPage?: boolean; endCursor?: string }; -} -interface RepoThreadsData { - repository?: { - issues?: RepoConn<{ number: number }>; - pullRequests?: RepoConn<{ number: number; isDraft?: boolean }>; - } | null; -} - interface ListThreadsState { issuesAfter: string | undefined; prsAfter: string | undefined; @@ -304,7 +305,7 @@ interface ListThreadsState { interface FetchNodeState { cursor: string | undefined; - acc: Array; + acc: Array; } const shallowThread = ( diff --git a/packages/adapter-github/src/auth.ts b/packages/adapter-github/src/auth.ts index f27e3a5..c816e1a 100644 --- a/packages/adapter-github/src/auth.ts +++ b/packages/adapter-github/src/auth.ts @@ -2,6 +2,7 @@ // App JWT (RS256) -> installation access token -> KV-cached for ~1h. import { Err, Ok, Result } from "@aipm/core"; +import { z } from "zod"; /** Minimal KV surface so this module needn't depend on @cloudflare/workers-types. */ export interface KVLike { @@ -31,6 +32,12 @@ export interface InstallationTokenProviderConfig { const DEFAULT_BASE = "https://api.github.com"; const SKEW_SECONDS = 300; +// Boundary schemas: GitHub REST replies (loose — only the consumed field is +// declared) and the KV round-trip of our own CachedToken. +const installationSchema = z.looseObject({ id: z.number() }); +const installationTokenSchema = z.looseObject({ token: z.string(), expires_at: z.string() }); +const cachedTokenSchema = z.object({ token: z.string(), expiresAt: z.string() }); + // --- PEM / base64url ---------------------------------------------------------- export function pkcs8PemToArrayBuffer(pem: string): Result { @@ -118,7 +125,9 @@ export async function resolveRepoInstallationId( if (!res.ok) return Err(new Error(`installation lookup HTTP ${res.status}`)); const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - return Ok((parsed.data as { id: number }).id); + const validated = installationSchema.safeParse(parsed.data); + if (!validated.success) return Err(new Error(`installation lookup: ${validated.error.message}`)); + return Ok(validated.data.id); } export async function mintInstallationToken( @@ -138,8 +147,9 @@ export async function mintInstallationToken( if (!res.ok) return Err(new Error(`installation token HTTP ${res.status}`)); const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - const json = parsed.data as { token: string; expires_at: string }; - return Ok({ token: json.token, expiresAt: json.expires_at }); + const validated = installationTokenSchema.safeParse(parsed.data); + if (!validated.success) return Err(new Error(`installation token: ${validated.error.message}`)); + return Ok({ token: validated.data.token, expiresAt: validated.data.expires_at }); } // --- Provider (KV-cached) ----------------------------------------------------- @@ -159,14 +169,19 @@ export function installationTokenProvider( const cachedResult = await Result.from(() => config.kv.get(key)); if (!cachedResult.ok) return cachedResult; const cached = cachedResult.data; - if (cached) { - // Treat a corrupt/unparseable entry as a miss rather than wedging forever. - const parsed = Result.fromSync(() => JSON.parse(cached) as CachedToken); - if (parsed.ok) { - const { token, expiresAt } = parsed.data; - if (token && (Date.parse(expiresAt) - now()) / 1000 > SKEW_SECONDS) return Ok(token); - } - } + // Treat a corrupt/unparseable/invalid entry as a miss rather than wedging forever. + const validCachedToken = (() => { + if (!cached) return null; + const json = Result.fromSync(() => JSON.parse(cached)); + if (!json.ok) return null; + const parsed = cachedTokenSchema.safeParse(json.data); + if (!parsed.success) return null; + const token = parsed.data.token; + const stillValid = (Date.parse(parsed.data.expiresAt) - now()) / 1000 > SKEW_SECONDS; + if (!token || !stillValid) return null; + return token; + })(); + if (validCachedToken) return Ok(validCachedToken); const jwt = await mintAppJwt(config.privateKeyPem, config.clientId, now()); if (!jwt.ok) return jwt; diff --git a/packages/adapter-github/src/discover-links.ts b/packages/adapter-github/src/discover-links.ts index a4b4afb..3e6e71d 100644 --- a/packages/adapter-github/src/discover-links.ts +++ b/packages/adapter-github/src/discover-links.ts @@ -1,4 +1,7 @@ import type { Link, LinkKind } from "@aipm/core"; +import type { GraphqlNode } from "./graphql-schema.js"; + +type Ref = { number?: number; repository?: { nameWithOwner?: string } } | null | undefined; /** * Native link discovery from a fetched GraphQL issue/PR node (DESIGN §4). Live @@ -10,10 +13,7 @@ import type { Link, LinkKind } from "@aipm/core"; * from CrossReferencedEvent.willCloseTarget (that would duplicate it in the * opposite direction). */ -export function discoverLinksFromGraphql( - fromNativeId: string, - node: Record, -): Array { +export function discoverLinksFromGraphql(fromNativeId: string, node: GraphqlNode): Array { const links = new Map(); const key = (l: Link) => `${l.from}|${l.to}|${l.kind}`; const add = (from: string | undefined, to: string | undefined, kind: LinkKind) => { @@ -26,16 +26,17 @@ export function discoverLinksFromGraphql( }; // --- live connections (authoritative) --- - conn(node, "closingIssuesReferences").forEach((r) => add(fromNativeId, ref(r), "closes")); - conn(node, "closedByPullRequestsReferences").forEach((r) => add(ref(r), fromNativeId, "closes")); - add(fromNativeId, ref((node as { parent?: unknown }).parent), "sub_issue"); - conn(node, "subIssues").forEach((r) => add(ref(r), fromNativeId, "sub_issue")); - conn(node, "blockedBy").forEach((r) => add(fromNativeId, ref(r), "blocked_by")); - conn(node, "blocking").forEach((r) => add(ref(r), fromNativeId, "blocked_by")); + connNodes(node.closingIssuesReferences).forEach((r) => add(fromNativeId, ref(r), "closes")); + connNodes(node.closedByPullRequestsReferences).forEach((r) => + add(ref(r), fromNativeId, "closes"), + ); + add(fromNativeId, ref(node.parent), "sub_issue"); + connNodes(node.subIssues).forEach((r) => add(ref(r), fromNativeId, "sub_issue")); + connNodes(node.blockedBy).forEach((r) => add(fromNativeId, ref(r), "blocked_by")); + connNodes(node.blocking).forEach((r) => add(ref(r), fromNativeId, "blocked_by")); // --- timeline (event-only kinds + Connected/Disconnected negation) --- - conn(node, "timelineItems").forEach((ev) => { - const e = ev as Record; + connNodes(node.timelineItems).forEach((e) => { switch (e.__typename) { case "ConnectedEvent": add(fromNativeId, ref(e.subject), "refs"); @@ -59,13 +60,16 @@ export function linkNativeId(repoNameWithOwner: string, number: number): string return `${repoNameWithOwner}#${number}`; } -const conn = (node: Record, field: string): Array => { - const nodes = (node[field] as { nodes?: unknown } | undefined)?.nodes; - return Array.isArray(nodes) ? nodes : []; +const connNodes = (conn: { nodes?: Array | null } | null | undefined): Array => { + const nodes = conn?.nodes; + if (!nodes) return []; + return nodes; }; -const ref = (n: unknown): string | undefined => { - const x = n as { number?: number; repository?: { nameWithOwner?: string } } | null | undefined; - if (!x?.number || !x.repository?.nameWithOwner) return undefined; - return `${x.repository.nameWithOwner}#${x.number}`; +const ref = (n: Ref): string | undefined => { + if (!n) return undefined; + if (!n.number) return undefined; + const owner = n.repository?.nameWithOwner; + if (!owner) return undefined; + return `${owner}#${n.number}`; }; diff --git a/packages/adapter-github/src/graphql-schema.ts b/packages/adapter-github/src/graphql-schema.ts new file mode 100644 index 0000000..10afb52 --- /dev/null +++ b/packages/adapter-github/src/graphql-schema.ts @@ -0,0 +1,103 @@ +import { z } from "zod"; + +// Lenient schemas for the GitHub GraphQL reads (DESIGN §4). `looseObject` keeps +// every node tolerant of fields GitHub adds (and of the extra fields our tests +// pass), while every field we actually consume in normalize + discoverLinks is +// declared and optional. Parsing happens once at the boundary (ghGraphQL); every +// consumer downstream receives these typed nodes, never `unknown`. + +const loginSchema = z.object({ login: z.string().optional() }); +const reviewerSchema = z.object({ login: z.string().optional(), slug: z.string().optional() }); +const labelSchema = z.object({ name: z.string().optional() }); +const pageInfoSchema = z.object({ + hasNextPage: z.boolean().optional(), + endCursor: z.string().optional(), +}); + +const refSchema = z.object({ + number: z.number().optional(), + repository: z.object({ nameWithOwner: z.string().optional() }).optional(), +}); + +const connSchema = (node: S) => + z.object({ nodes: z.array(node).optional(), pageInfo: pageInfoSchema.optional() }); + +/** One timeline union node — wide + loose: every field read across the issue and + * PR timelines is optional, unknown variants/fields pass through untouched. */ +export const graphqlTimelineNodeSchema = z.looseObject({ + __typename: z.string().optional(), + actor: loginSchema.nullish(), + author: loginSchema.nullish(), + createdAt: z.string().optional(), + submittedAt: z.string().optional(), + body: z.string().optional(), + state: z.string().optional(), + stateReason: z.string().nullish(), + label: labelSchema.nullish(), + assignee: loginSchema.nullish(), + requestedReviewer: reviewerSchema.nullish(), + commit: z.object({ oid: z.string().optional() }).nullish(), + previousTitle: z.string().nullish(), + currentTitle: z.string().nullish(), + // link-only targets (discoverLinks) + source: refSchema.nullish(), + subject: refSchema.nullish(), + canonical: refSchema.nullish(), +}); +export type GraphqlTimelineNode = z.infer; + +/** A fetched issue/PR node with its (paginated) timeline. Loose so a node missing + * issue-only or PR-only fields still parses; consumers read declared fields. */ +export const graphqlNodeSchema = z.looseObject({ + number: z.number().optional(), + title: z.string().nullish(), + body: z.string().nullish(), + state: z.string().optional(), + stateReason: z.string().nullish(), + isDraft: z.boolean().optional(), + merged: z.boolean().optional(), + mergedAt: z.string().nullish(), + reviewDecision: z.string().nullish(), + createdAt: z.string().optional(), + updatedAt: z.string().optional(), + author: loginSchema.nullish(), + assignees: connSchema(loginSchema).nullish(), + labels: connSchema(labelSchema).nullish(), + reviews: connSchema(z.object({ author: loginSchema.nullish() })).nullish(), + reviewRequests: connSchema(z.object({ requestedReviewer: reviewerSchema.nullish() })).nullish(), + reviewThreads: connSchema( + z.object({ comments: connSchema(z.object({ author: loginSchema.nullish() })).nullish() }), + ).nullish(), + subIssuesSummary: z.unknown().optional(), + issueDependenciesSummary: z.unknown().optional(), + timelineItems: connSchema(graphqlTimelineNodeSchema).nullish(), + // live link connections (discoverLinks) + parent: refSchema.nullish(), + subIssues: connSchema(refSchema).nullish(), + blockedBy: connSchema(refSchema).nullish(), + blocking: connSchema(refSchema).nullish(), + closingIssuesReferences: connSchema(refSchema).nullish(), + closedByPullRequestsReferences: connSchema(refSchema).nullish(), +}); +export type GraphqlNode = z.infer; + +/** `repository.issue` / `repository.pullRequest` envelope for a single-node read. */ +export const repoNodeDataSchema = z.object({ + repository: z + .object({ issue: graphqlNodeSchema.nullish(), pullRequest: graphqlNodeSchema.nullish() }) + .nullish(), +}); +export type RepoNodeData = z.infer; + +/** Shallow issue/PR lists for repo sweeps. */ +export const repoThreadsDataSchema = z.object({ + repository: z + .object({ + issues: connSchema(z.object({ number: z.number() })).nullish(), + pullRequests: connSchema( + z.object({ number: z.number(), isDraft: z.boolean().optional() }), + ).nullish(), + }) + .nullish(), +}); +export type RepoThreadsData = z.infer; diff --git a/packages/adapter-github/src/graphql.ts b/packages/adapter-github/src/graphql.ts index c6769cc..942061d 100644 --- a/packages/adapter-github/src/graphql.ts +++ b/packages/adapter-github/src/graphql.ts @@ -1,4 +1,5 @@ import { Err, Ok, Result } from "@aipm/core"; +import { z } from "zod"; export interface GhGraphQLOptions { apiBaseUrl?: string; // default https://api.github.com @@ -6,21 +7,24 @@ export interface GhGraphQLOptions { fetchImpl?: typeof fetch; } -interface GraphQLError { - message: string; - type?: string; -} +const graphqlEnvelopeSchema = z.looseObject({ + data: z.unknown().optional(), + errors: z.array(z.looseObject({ message: z.string().optional() })).optional(), +}); /** * Minimal GitHub GraphQL client. Sends the `sub_issues` feature header (harmless * once GA, required while in preview) and returns `Err` on transport or `errors`. + * The envelope and its `data` payload are validated against `schema` at the + * boundary, so callers receive a typed value (or an Err), never narrowing `unknown`. */ -export async function ghGraphQL( +export async function ghGraphQL( token: string, query: string, variables: Record, + schema: S, opts: GhGraphQLOptions = {}, -): Promise> { +): Promise, Error>> { const base = opts.apiBaseUrl ?? "https://api.github.com"; const doFetch = opts.fetchImpl ?? fetch; const requestBody = Result.fromSync(() => JSON.stringify({ query, variables })); @@ -50,10 +54,15 @@ export async function ghGraphQL( const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - const json = parsed.data as { data?: T; errors?: Array }; - if (json.errors?.length) { - return Err(new Error(`GitHub GraphQL errors: ${json.errors.map((e) => e.message).join("; ")}`)); + const envelope = graphqlEnvelopeSchema.safeParse(parsed.data); + if (!envelope.success) return Err(new Error(`GitHub GraphQL: ${envelope.error.message}`)); + const errors = envelope.data.errors; + if (errors?.length) { + const messages = errors.map((e) => e.message ?? "unknown").join("; "); + return Err(new Error(`GitHub GraphQL errors: ${messages}`)); } - if (json.data === undefined) return Err(new Error("GitHub GraphQL: empty response")); - return Ok(json.data); + if (envelope.data.data === undefined) return Err(new Error("GitHub GraphQL: empty response")); + const validated = schema.safeParse(envelope.data.data); + if (!validated.success) return Err(new Error(`GitHub GraphQL: ${validated.error.message}`)); + return Ok(validated.data); } diff --git a/packages/adapter-github/src/normalize.ts b/packages/adapter-github/src/normalize.ts index 3635374..d847735 100644 --- a/packages/adapter-github/src/normalize.ts +++ b/packages/adapter-github/src/normalize.ts @@ -8,6 +8,7 @@ import { type ThreadType, type TimelineEvent, } from "@aipm/core"; +import type { GraphqlNode, GraphqlTimelineNode } from "./graphql-schema.js"; // --- helpers ------------------------------------------------------------------ @@ -29,10 +30,7 @@ export function isBotLogin(login: string | undefined, botAccounts: Array return normalized.endsWith("[bot]") || botAccounts.includes(normalized); } -const loginOf = (a: unknown): string | undefined => { - const login = (a as { login?: unknown } | null | undefined)?.login; - return typeof login === "string" ? login : undefined; -}; +const loginOf = (a: { login?: string } | null | undefined): string | undefined => a?.login; const MENTION = /(? | undefined { return out.size ? [...out] : undefined; } -const nodesOf = (conn: unknown): Array => { - const nodes = (conn as { nodes?: unknown } | null | undefined)?.nodes; - return Array.isArray(nodes) ? (nodes as Array) : []; +const nodesOf = (conn: { nodes?: Array | null } | null | undefined): Array => { + const nodes = conn?.nodes; + if (!nodes) return []; + return nodes; }; // --- webhook event -> routable ref -------------------------------------------- @@ -121,7 +120,7 @@ export function prState(node: { /** All distinct human (non-bot) logins touching a thread → participant handles. */ export function collectParticipantLogins( - node: Record, + node: GraphqlNode, opts: NormalizeOptions = {}, ): Array { const bots = opts.botAccounts ?? []; @@ -132,15 +131,11 @@ export function collectParticipantLogins( add(loginOf(node.author)); nodesOf(node.assignees).forEach((a) => add(loginOf(a))); - nodesOf>(node.timelineItems).forEach((n) => { - add(loginOf(n.actor) ?? loginOf(n.author)); - }); - nodesOf>(node.reviews).forEach((r) => add(loginOf(r.author))); - nodesOf>(node.reviewRequests).forEach((r) => { - add(loginOf((r as { requestedReviewer?: unknown }).requestedReviewer)); - }); - nodesOf>(node.reviewThreads).forEach((t) => { - nodesOf>(t.comments).forEach((c) => add(loginOf(c.author))); + nodesOf(node.timelineItems).forEach((n) => add(loginOf(n.actor) ?? loginOf(n.author))); + nodesOf(node.reviews).forEach((r) => add(loginOf(r.author))); + nodesOf(node.reviewRequests).forEach((r) => add(loginOf(r.requestedReviewer))); + nodesOf(node.reviewThreads).forEach((t) => { + nodesOf(t.comments).forEach((c) => add(loginOf(c.author))); }); return [...out]; } @@ -152,10 +147,10 @@ const clean = >(obj: T): Partial => /** Map filtered timeline union nodes to TimelineEvents. Link-only nodes (cross-ref, * connected, sub-issue, blocked-by, …) are dropped here and handled by discoverLinks. */ -export function normalizeTimeline(nodes: Array>): Array { +export function normalizeTimeline(nodes: Array): Array { return nodes.flatMap((n) => { const actor = loginOf(n.actor) ?? loginOf(n.author); - const at = (n.createdAt ?? n.submittedAt) as string | undefined; + const at = n.createdAt ?? n.submittedAt; const mapped = mapTimelineNode(n); if (!mapped || !at) return []; return [clean({ kind: mapped.kind, actor, at, data: mapped.data }) as TimelineEvent]; @@ -163,10 +158,10 @@ export function normalizeTimeline(nodes: Array>): Array< } function mapTimelineNode( - n: Record, + n: GraphqlTimelineNode, ): { kind: string; data: Record } | undefined { const t = n.__typename; - const name = (x: unknown) => (x as { name?: string } | null)?.name; + const name = (x: { name?: string } | null | undefined): string | undefined => x?.name; switch (t) { case "IssueComment": return { kind: "comment", data: clean({ body: n.body, mentions: mentionsOf(n.body) }) }; @@ -190,10 +185,7 @@ function mapTimelineNode( case "ConvertToDraftEvent": return { kind: "status", data: { state: "draft" } }; case "MergedEvent": - return { - kind: "status", - data: clean({ state: "merged", commit: (n.commit as { oid?: string } | null)?.oid }), - }; + return { kind: "status", data: clean({ state: "merged", commit: n.commit?.oid }) }; case "ClosedEvent": return { kind: "status", data: clean({ state: "closed", stateReason: n.stateReason }) }; case "ReopenedEvent": @@ -218,16 +210,19 @@ function mapTimelineNode( } } -const reviewerHandle = (r: unknown): string | undefined => { - const rr = r as { login?: string; slug?: string } | null | undefined; - return rr?.login ?? rr?.slug ?? undefined; +const reviewerHandle = ( + r: { login?: string; slug?: string } | null | undefined, +): string | undefined => { + if (!r) return undefined; + if (r.login) return r.login; + return r.slug; }; -function baseMeta(node: Record, repoFullName: string): Record { +function baseMeta(node: GraphqlNode, repoFullName: string): Record { return clean({ repo: repoFullName, author: loginOf(node.author), - labels: nodesOf<{ name?: string }>(node.labels) + labels: nodesOf(node.labels) .map((l) => l.name) .filter(Boolean), assignees: nodesOf(node.assignees).map(loginOf).filter(Boolean), @@ -237,17 +232,17 @@ function baseMeta(node: Record, repoFullName: string): Record, + node: GraphqlNode, repoFullName: string, opts: NormalizeOptions = {}, ): Thread { return { platform: "github", - nativeId: `${repoFullName}#${node.number as number}`, + nativeId: `${repoFullName}#${node.number}`, type: "issue", - title: (node.title as string) ?? undefined, - body: (node.body as string) ?? undefined, - state: issueState(node as { state?: string }), + title: node.title ?? undefined, + body: node.body ?? undefined, + state: issueState(node), participants: collectParticipantLogins(node, opts), meta: clean({ ...baseMeta(node, repoFullName), @@ -255,22 +250,22 @@ export function normalizeIssueGraphql( subIssuesSummary: node.subIssuesSummary, issueDependenciesSummary: node.issueDependenciesSummary, }), - timeline: normalizeTimeline(nodesOf>(node.timelineItems)), + timeline: normalizeTimeline(nodesOf(node.timelineItems)), }; } export function normalizePrGraphql( - node: Record, + node: GraphqlNode, repoFullName: string, opts: NormalizeOptions = {}, ): Thread { return { platform: "github", - nativeId: `${repoFullName}#${node.number as number}`, + nativeId: `${repoFullName}#${node.number}`, type: "pr", - title: (node.title as string) ?? undefined, - body: (node.body as string) ?? undefined, - state: prState(node as Parameters[0]), + title: node.title ?? undefined, + body: node.body ?? undefined, + state: prState(node), participants: collectParticipantLogins(node, opts), meta: clean({ ...baseMeta(node, repoFullName), @@ -279,6 +274,6 @@ export function normalizePrGraphql( mergedAt: node.mergedAt, reviewDecision: node.reviewDecision, }), - timeline: normalizeTimeline(nodesOf>(node.timelineItems)), + timeline: normalizeTimeline(nodesOf(node.timelineItems)), }; } diff --git a/packages/adapter-github/src/rest.ts b/packages/adapter-github/src/rest.ts index b47df36..5f5324e 100644 --- a/packages/adapter-github/src/rest.ts +++ b/packages/adapter-github/src/rest.ts @@ -1,18 +1,24 @@ import { Err, Ok, Result } from "@aipm/core"; +import { z } from "zod"; export interface GhRestOptions { apiBaseUrl?: string; // default https://api.github.com fetchImpl?: typeof fetch; } -/** Minimal GitHub REST client. `pathOrUrl` may be a path or an absolute api url. */ -export async function ghRest( +/** + * Minimal GitHub REST client. `pathOrUrl` may be a path or an absolute api url. + * The response body is validated against `schema` at the boundary, so callers + * receive a typed value (or an Err) and never narrow `unknown` themselves. + */ +export async function ghRest( token: string, method: string, pathOrUrl: string, - body?: unknown, + body: unknown, + schema: S, opts: GhRestOptions = {}, -): Promise> { +): Promise, Error>> { const base = opts.apiBaseUrl ?? "https://api.github.com"; const url = pathOrUrl.startsWith("http") ? pathOrUrl : `${base}${pathOrUrl}`; const requestBody = body !== undefined ? Result.fromSync(() => JSON.stringify(body)) : null; @@ -38,8 +44,16 @@ export async function ghRest( // without importing this module's types. return Err(Object.assign(new Error(msg), { status: res.status })); } - if (res.status === 204) return Ok(undefined as T); + if (res.status === 204) { + const empty = schema.safeParse(undefined); + if (!empty.success) return Err(new Error(`GitHub REST ${method} ${url}: unexpected empty 204`)); + return Ok(empty.data); + } const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - return Ok(parsed.data as T); + const validated = schema.safeParse(parsed.data); + if (!validated.success) { + return Err(new Error(`GitHub REST ${method} ${url}: ${validated.error.message}`)); + } + return Ok(validated.data); } diff --git a/packages/adapter-llm/package.json b/packages/adapter-llm/package.json index 1417285..44cddd3 100644 --- a/packages/adapter-llm/package.json +++ b/packages/adapter-llm/package.json @@ -12,7 +12,8 @@ "test": "vitest run" }, "dependencies": { - "@aipm/core": "workspace:*" + "@aipm/core": "workspace:*", + "zod": "^4.4.3" }, "devDependencies": { "@aipm/tsconfig": "workspace:*", diff --git a/packages/adapter-llm/src/index.test.ts b/packages/adapter-llm/src/index.test.ts index 67baacd..c1d0765 100644 --- a/packages/adapter-llm/src/index.test.ts +++ b/packages/adapter-llm/src/index.test.ts @@ -8,8 +8,13 @@ import { } from "./index.js"; describe("extractText", () => { + const extracted = (res: unknown): string => { + const r = extractText(res); + if (!r.ok) throw r.error; + return r.data; + }; it("reads legacy {response} (llama)", () => { - expect(extractText({ response: "hi" })).toBe("hi"); + expect(extracted({ response: "hi" })).toBe("hi"); }); it("reads the Responses output[] shape (gpt-oss), skipping reasoning", () => { const res = { @@ -18,16 +23,20 @@ describe("extractText", () => { { type: "message", content: [{ type: "output_text", text: "the answer" }] }, ], }; - expect(extractText(res)).toBe("the answer"); + expect(extracted(res)).toBe("the answer"); }); it("reads output_text convenience field", () => { - expect(extractText({ output_text: "quick" })).toBe("quick"); + expect(extracted({ output_text: "quick" })).toBe("quick"); }); it("reads Chat Completions choices[]", () => { - expect(extractText({ choices: [{ message: { content: "chat" } }] })).toBe("chat"); + expect(extracted({ choices: [{ message: { content: "chat" } }] })).toBe("chat"); + }); + it("returns empty string on a valid response with no text", () => { + expect(extracted({ weird: true })).toBe(""); }); - it("returns empty string on an unknown shape", () => { - expect(extractText({ weird: true })).toBe(""); + it("errors on a non-object response", () => { + const r = extractText("nope"); + expect(r.ok).toBe(false); }); }); diff --git a/packages/adapter-llm/src/index.ts b/packages/adapter-llm/src/index.ts index abc9ec9..b93819e 100644 --- a/packages/adapter-llm/src/index.ts +++ b/packages/adapter-llm/src/index.ts @@ -7,6 +7,7 @@ import { type LlmAdapter, type LlmOptions, } from "@aipm/core"; +import { z } from "zod"; export interface WorkersAiConfig { ai: Ai; @@ -52,39 +53,60 @@ export class WorkersAiLlmAdapter implements LlmAdapter { const res = await Promise.race([completion, timer]); if (res === timedOut) return Ok(""); if (!res.ok) return res; - const text = extractText(res.data); - return Ok(text); + return extractText(res.data); } } /** - * Extract the assistant text across Workers AI response shapes: legacy - * `{response}`, OpenAI Responses (`output_text` / `output[].content[].text`), - * and Chat Completions (`choices[].message.content`). gpt-oss models return the - * Responses shape via /ai/run, llama returns `{response}`. + * Lenient schema for the Workers AI response shapes we read: legacy `{response}`, + * OpenAI Responses (`output_text` / `output[].content[].text`), and Chat + * Completions (`choices[].message.content`). Every field is optional, so a known + * shape with extra or missing keys still parses; only a non-object is rejected. */ -export function extractText(res: unknown): string { - const r = res as Record; - if (typeof r?.response === "string") return r.response; - if (typeof r?.output_text === "string") return r.output_text; - - if (Array.isArray(r?.output)) { - const output = r.output as Array>; - const parts = output.flatMap((item) => { - if (item?.type === "reasoning") return []; // skip chain-of-thought items - const content = Array.isArray(item?.content) - ? (item.content as Array>) - : []; - return content.flatMap((c) => (typeof c?.text === "string" ? [c.text] : [])); - }); - if (parts.length) return parts.join(""); - } - - const choice = (r?.choices as Array>)?.[0]; - const msg = choice?.message as { content?: unknown } | undefined; - if (typeof msg?.content === "string") return msg.content; +const workersAiResponseSchema = z.object({ + response: z.string().optional(), + output_text: z.string().optional(), + output: z + .array( + z.object({ + type: z.string().optional(), + content: z.array(z.object({ text: z.string().optional() })).optional(), + }), + ) + .optional(), + choices: z + .array(z.object({ message: z.object({ content: z.string().optional() }).optional() })) + .optional(), +}); - return ""; +/** + * Parse a Workers AI completion to its assistant text. gpt-oss returns the + * Responses shape via /ai/run, llama returns `{response}`. A non-object response + * is an Err; a valid response carrying no text yields Ok("") (callers skip empty). + */ +export function extractText(res: unknown): Result { + const parsed = workersAiResponseSchema.safeParse(res); + if (!parsed.success) { + return Err(new Error(`unparseable Workers AI response: ${parsed.error.message}`)); + } + const r = parsed.data; + if (typeof r.response === "string") return Ok(r.response); + if (typeof r.output_text === "string") return Ok(r.output_text); + + const outputItems = r.output ?? []; + const outputParts = outputItems.flatMap((item) => { + if (item.type === "reasoning") return []; // skip chain-of-thought items + const itemContent = item.content ?? []; + return itemContent.flatMap((c) => (typeof c.text === "string" ? [c.text] : [])); + }); + if (outputParts.length) return Ok(outputParts.join("")); + + const choices = r.choices ?? []; + const firstChoice = choices[0]; + if (!firstChoice) return Ok(""); + const messageContent = firstChoice.message?.content; + if (typeof messageContent === "string") return Ok(messageContent); + return Ok(""); } /** A deterministic stub for tests / shadow runs without an AI binding. */ diff --git a/packages/adapter-slack/package.json b/packages/adapter-slack/package.json index a185b2b..f3c25cc 100644 --- a/packages/adapter-slack/package.json +++ b/packages/adapter-slack/package.json @@ -12,7 +12,8 @@ "test": "vitest run --passWithNoTests" }, "dependencies": { - "@aipm/core": "workspace:*" + "@aipm/core": "workspace:*", + "zod": "^4.4.3" }, "devDependencies": { "@aipm/tsconfig": "workspace:*", diff --git a/packages/adapter-slack/src/adapter.ts b/packages/adapter-slack/src/adapter.ts index e7f0d45..bafa9af 100644 --- a/packages/adapter-slack/src/adapter.ts +++ b/packages/adapter-slack/src/adapter.ts @@ -11,6 +11,7 @@ import type { } from "@aipm/core"; import { Err, Ok, Result } from "@aipm/core"; import { resolveSlackId } from "./identity.js"; +import { z } from "zod"; export interface SlackAdapterConfig { botToken: string; @@ -24,6 +25,27 @@ const DEFAULT_BASE = "https://slack.com/api"; export const isSlackUserId = (s: string | undefined): boolean => !!s && /^[UW][A-Z0-9]{6,}$/.test(s); +// --- Slack Web API response schemas (parse at the boundary; typed downstream) --- + +const slackEnvelopeSchema = z.object({ ok: z.boolean(), error: z.string().optional() }); + +const slackMessageSchema = z.object({ + user: z.string().optional(), + bot_id: z.string().optional(), + text: z.string().optional(), + ts: z.string(), +}); + +const conversationsRepliesSchema = z.object({ + messages: z.array(slackMessageSchema).optional(), +}); +const chatPostMessageSchema = z.object({ ts: z.string().optional() }); +const conversationsOpenSchema = z.object({ + channel: z.object({ id: z.string().optional() }).optional(), +}); +/** For calls where only the ok/error envelope matters (chat.update, reactions.add). */ +const slackOkSchema = z.object({}); + /** * SlackAdapter (DESIGN §3/§8). Verifies the signing secret (see verify.ts), * opens DMs, parses preference messages, and models Slack threads as Threads. @@ -49,11 +71,11 @@ export class SlackAdapter implements Platform { const parsed = parseSlackNativeId(nativeId); if (!parsed.ok) return parsed; const { channel, ts } = parsed.data; - const res = await this.get<{ messages?: Array }>("conversations.replies", { - channel, - ts, - limit: "200", - }); + const res = await this.get( + "conversations.replies", + { channel, ts, limit: "200" }, + conversationsRepliesSchema, + ); if (!res.ok) return res; const messages = res.data.messages ?? []; const root = messages[0]; @@ -104,10 +126,11 @@ export class SlackAdapter implements Platform { const channelMeta = target.meta?.channelId; const channelId = typeof channelMeta === "string" ? channelMeta : undefined; if (channelId) { - const posted = await this.post<{ ts?: string }>("chat.postMessage", { - channel: channelId, - text: body, - }); + const posted = await this.post( + "chat.postMessage", + { channel: channelId, text: body }, + chatPostMessageSchema, + ); if (!posted.ok) return posted; return Ok({ id: `${channelId}/${posted.data.ts}` }); } @@ -118,11 +141,11 @@ export class SlackAdapter implements Platform { const parsed = parseSlackNativeId(threadNativeId); if (!parsed.ok) return parsed; const { channel, ts } = parsed.data; - const res = await this.post<{ ts?: string }>("chat.postMessage", { - channel, - thread_ts: ts, - text: body, - }); + const res = await this.post( + "chat.postMessage", + { channel, thread_ts: ts, text: body }, + chatPostMessageSchema, + ); if (!res.ok) return res; return Ok({ id: `${channel}/${res.data.ts}` }); } @@ -132,7 +155,7 @@ export class SlackAdapter implements Platform { const parsed = parseSlackNativeId(messageId); if (!parsed.ok) return parsed; const { channel, ts } = parsed.data; - const r = await this.post("chat.update", { channel, ts, text: body }); + const r = await this.post("chat.update", { channel, ts, text: body }, slackOkSchema); if (!r.ok) return r; return Ok(undefined); } @@ -144,11 +167,11 @@ export class SlackAdapter implements Platform { const parsed = parseSlackNativeId(threadNativeId); if (!parsed.ok) return parsed; const { channel, ts } = parsed.data; - const res = await this.get<{ messages?: Array }>("conversations.replies", { - channel, - ts, - limit: "200", - }); + const res = await this.get( + "conversations.replies", + { channel, ts, limit: "200" }, + conversationsRepliesSchema, + ); if (!res.ok) return res; const hit = (res.data.messages ?? []).find((m) => m.text?.includes(marker)); return Ok(hit ? `${channel}/${hit.ts}` : undefined); @@ -158,7 +181,11 @@ export class SlackAdapter implements Platform { const parsed = parseSlackNativeId(messageId); if (!parsed.ok) return parsed; const { channel, ts } = parsed.data; - const r = await this.post("reactions.add", { channel, timestamp: ts, name: emoji }); + const r = await this.post( + "reactions.add", + { channel, timestamp: ts, name: emoji }, + slackOkSchema, + ); if (!r.ok) return r; return Ok(undefined); } @@ -178,21 +205,20 @@ export class SlackAdapter implements Platform { async notifyPerson(identity: Identity, body: string): Promise> { const uid = identity.handles.slack; if (!uid) return Err(new Error(`no Slack id on identity ${identity.id}`)); - const opened = await this.post<{ channel?: { id?: string } }>("conversations.open", { - users: uid, - }); + const opened = await this.post("conversations.open", { users: uid }, conversationsOpenSchema); if (!opened.ok) return opened; const channel = opened.data.channel?.id; if (!channel) return Err(new Error("conversations.open returned no channel")); - const r = await this.post("chat.postMessage", { channel, text: body }); + const r = await this.post("chat.postMessage", { channel, text: body }, slackOkSchema); if (!r.ok) return r; return Ok(undefined); } - private async post( + private async post( method: string, body: Record, - ): Promise> { + schema: S, + ): Promise, Error>> { const base = this.config.apiBaseUrl ?? DEFAULT_BASE; const requestBody = Result.fromSync(() => JSON.stringify(body)); if (!requestBody.ok) return requestBody; @@ -211,15 +237,14 @@ export class SlackAdapter implements Platform { if (!res.ok) return Err(new Error(`Slack ${method} HTTP ${res.status}`)); const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - const json = parsed.data as T & { ok: boolean; error?: string }; - if (!json.ok) return Err(new Error(`Slack ${method} error: ${json.error ?? "unknown"}`)); - return Ok(json); + return parseSlackResponse(method, parsed.data, schema); } - private async get( + private async get( method: string, params: Record, - ): Promise> { + schema: S, + ): Promise, Error>> { const base = this.config.apiBaseUrl ?? DEFAULT_BASE; const url = `${base}/${method}?${new URLSearchParams(params).toString()}`; const fetched = await Result.from(() => @@ -232,18 +257,29 @@ export class SlackAdapter implements Platform { if (!res.ok) return Err(new Error(`Slack ${method} HTTP ${res.status}`)); const parsed = await Result.from(() => res.json()); if (!parsed.ok) return parsed; - const json = parsed.data as T & { ok: boolean; error?: string }; - if (!json.ok) return Err(new Error(`Slack ${method} error: ${json.error ?? "unknown"}`)); - return Ok(json); + return parseSlackResponse(method, parsed.data, schema); } } -interface SlackMessage { - user?: string; - bot_id?: string; - text?: string; - ts: string; -} +/** + * Validate a Slack Web API response: the ok/error envelope first (a `ok:false` + * body is a typed API error), then the endpoint payload. Parse failure is an Err + * so no malformed response reaches a caller. + */ +const parseSlackResponse = ( + method: string, + raw: unknown, + schema: S, +): Result, Error> => { + const envelope = slackEnvelopeSchema.safeParse(raw); + if (!envelope.success) return Err(new Error(`Slack ${method}: malformed response`)); + if (!envelope.data.ok) { + return Err(new Error(`Slack ${method} error: ${envelope.data.error ?? "unknown"}`)); + } + const payload = schema.safeParse(raw); + if (!payload.success) return Err(new Error(`Slack ${method}: ${payload.error.message}`)); + return Ok(payload.data); +}; /** `${channel}/${ts}` — channel ids have no '/', ts is `.`. */ function parseSlackNativeId(nativeId: string): Result<{ channel: string; ts: string }, Error> { diff --git a/packages/adapter-slack/src/identity.ts b/packages/adapter-slack/src/identity.ts index 91e3fba..431f155 100644 --- a/packages/adapter-slack/src/identity.ts +++ b/packages/adapter-slack/src/identity.ts @@ -3,6 +3,7 @@ // never DM'd (digest-only) — that routing fallback is phase-3, not here. import { asyncUnfold, Err, Ok, Result } from "@aipm/core"; +import { z } from "zod"; export interface SlackResolveConfig { botToken: string; @@ -14,13 +15,25 @@ export interface SlackResolveConfig { maxRetries?: number; } -interface SlackUser { - id: string; - name?: string; - deleted?: boolean; - is_bot?: boolean; - profile?: { display_name?: string; display_name_normalized?: string }; -} +const slackUserSchema = z.object({ + id: z.string(), + name: z.string().optional(), + deleted: z.boolean().optional(), + is_bot: z.boolean().optional(), + profile: z + .object({ + display_name: z.string().optional(), + display_name_normalized: z.string().optional(), + }) + .optional(), +}); + +const slackEnvelopeSchema = z.object({ ok: z.boolean(), error: z.string().optional() }); +const usersLookupByEmailSchema = slackEnvelopeSchema.extend({ user: slackUserSchema.optional() }); +const usersListSchema = slackEnvelopeSchema.extend({ + members: z.array(slackUserSchema).optional(), + response_metadata: z.object({ next_cursor: z.string().optional() }).optional(), +}); const DEFAULT_BASE = "https://slack.com/api"; const FATAL = new Set(["missing_scope", "invalid_auth", "account_inactive", "token_revoked"]); @@ -42,7 +55,7 @@ async function lookupByEmail( config: SlackResolveConfig, email: string, ): Promise> { - const res = await call<{ user?: SlackUser }>(config, "users.lookupByEmail", { email }); + const res = await call(config, "users.lookupByEmail", { email }, usersLookupByEmailSchema); if (!res.ok) return res; if (res.data.ok) return Ok(res.data.user?.id); if (res.data.error === "users_not_found") return Ok(undefined); // gap: log upstream @@ -55,10 +68,12 @@ async function lookupByHandle( ): Promise> { const seed = ""; return asyncUnfold(seed, async (cursor) => { - const listResult = await call<{ - members?: Array; - response_metadata?: { next_cursor?: string }; - }>(config, "users.list", { limit: "200", ...(cursor ? { cursor } : {}) }); + const listResult = await call( + config, + "users.list", + { limit: "200", ...(cursor ? { cursor } : {}) }, + usersListSchema, + ); if (!listResult.ok) return { kind: "STOP" as const, value: listResult }; const res = listResult.data; if (!res.ok) { @@ -82,16 +97,12 @@ async function lookupByHandle( }); } -interface SlackResponse { - ok: boolean; - error?: string; -} - -async function call( +async function call( config: SlackResolveConfig, method: string, params: Record, -): Promise> { + schema: S, +): Promise, Error>> { const base = config.apiBaseUrl ?? DEFAULT_BASE; const url = `${base}/${method}?${new URLSearchParams(params).toString()}`; const doFetch = config.fetchImpl ?? fetch; @@ -124,8 +135,12 @@ async function call( } const parsed = await Result.from(() => res.json()); if (!parsed.ok) return { kind: "STOP" as const, value: parsed }; - const body = parsed.data as SlackResponse & T; - return { kind: "STOP" as const, value: Ok(body) }; + const validated = schema.safeParse(parsed.data); + if (!validated.success) { + const parseError = new Error(`Slack ${method}: ${validated.error.message}`); + return { kind: "STOP" as const, value: Err(parseError) }; + } + return { kind: "STOP" as const, value: Ok(validated.data) }; }); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6940c83..8ae784c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -93,6 +93,9 @@ importers: '@aipm/core': specifier: workspace:* version: link:../core + zod: + specifier: ^4.4.3 + version: 4.4.3 devDependencies: '@aipm/tsconfig': specifier: workspace:* @@ -109,6 +112,9 @@ importers: '@aipm/core': specifier: workspace:* version: link:../core + zod: + specifier: ^4.4.3 + version: 4.4.3 devDependencies: '@aipm/tsconfig': specifier: workspace:* @@ -128,6 +134,9 @@ importers: '@aipm/core': specifier: workspace:* version: link:../core + zod: + specifier: ^4.4.3 + version: 4.4.3 devDependencies: '@aipm/tsconfig': specifier: workspace:*