diff --git a/mobile/src/tasks/github-pr-file-diff.ts b/mobile/src/tasks/github-pr-file-diff.ts index 1be684197d4..7be6693ae22 100644 --- a/mobile/src/tasks/github-pr-file-diff.ts +++ b/mobile/src/tasks/github-pr-file-diff.ts @@ -18,7 +18,7 @@ type DiffOperation = const EXACT_DIFF_CELL_LIMIT = 160_000 -function splitContentLines(value: string): string[] { +function splitContentLines(value: string | undefined): string[] { if (!value) { return [] } @@ -118,8 +118,8 @@ export function buildGitHubPrFileDiffLines( } export function buildGitHubPrFileDiffPreview( - originalContent: string, - modifiedContent: string, + originalContent: string | undefined, + modifiedContent: string | undefined, maxLines = Number.POSITIVE_INFINITY ): GitHubPrFileDiffPreview { const originalLines = splitContentLines(originalContent) diff --git a/mobile/src/tasks/mobile-task-item-comment-operations.ts b/mobile/src/tasks/mobile-task-item-comment-operations.ts index 171de189439..d6f1e661595 100644 --- a/mobile/src/tasks/mobile-task-item-comment-operations.ts +++ b/mobile/src/tasks/mobile-task-item-comment-operations.ts @@ -1,10 +1,20 @@ import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' -import { rpcUncheckedPayloadReader } from '../transport/rpc-reader-payload' +import { rpcResultVariant } from '../transport/rpc-operation-result-reader' +import { + linearCommentWrittenSchema, + reviewThreadResolvedSchema, + taskCommentWrittenSchema +} from './task-item-comment-reply-schema' // Writing comments and replies on a task item, over all three providers. Every one of these // answers with an accepted `{ ok, error, comment }` envelope the call site reads itself, and every // one keeps its own fallback copy for an envelope that carries no error text — so the acceptance // policy here only decides whether there is an envelope to read at all. +// +// The five that answer with a comment share one reader, because they share one reply convention; +// each schema lives in task-item-comment-reply-schema.ts with the consumer line behind it. + +const taskCommentWrittenReader = rpcResultVariant('task-comment-written', taskCommentWrittenSchema) export const githubIssueCommentWrite = bindDeferredRpcOperation( defineRpcOperation({ @@ -12,7 +22,7 @@ export const githubIssueCommentWrite = bindDeferredRpcOperation( method: 'github.addIssueComment', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-issue-comment') + read: taskCommentWrittenReader }) ) @@ -22,7 +32,7 @@ export const githubReviewCommentWrite = bindDeferredRpcOperation( method: 'github.addPRReviewComment', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-review-comment') + read: taskCommentWrittenReader }) ) @@ -32,7 +42,7 @@ export const githubReviewCommentReplyWrite = bindDeferredRpcOperation( method: 'github.addPRReviewCommentReply', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-review-comment-reply') + read: taskCommentWrittenReader }) ) @@ -42,7 +52,7 @@ export const gitlabIssueCommentWrite = bindDeferredRpcOperation( method: 'gitlab.addIssueComment', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-issue-comment') + read: taskCommentWrittenReader }) ) @@ -52,7 +62,7 @@ export const gitlabMergeRequestCommentWrite = bindDeferredRpcOperation( method: 'gitlab.addMRComment', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-mr-comment') + read: taskCommentWrittenReader }) ) @@ -63,7 +73,7 @@ export const linearIssueCommentWrite = bindDeferredRpcOperation( method: 'linear.addIssueComment', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-issue-comment') + read: rpcResultVariant('linear-comment-written', linearCommentWrittenSchema) }) ) @@ -74,6 +84,6 @@ export const githubReviewThreadResolve = bindDeferredRpcOperation( method: 'github.resolveReviewThread', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-review-thread-resolved') + read: rpcResultVariant('github-review-thread-resolved', reviewThreadResolvedSchema) }) ) diff --git a/mobile/src/tasks/mobile-task-item-detail-operations.ts b/mobile/src/tasks/mobile-task-item-detail-operations.ts index 1b92087ff58..2109d6a6c06 100644 --- a/mobile/src/tasks/mobile-task-item-detail-operations.ts +++ b/mobile/src/tasks/mobile-task-item-detail-operations.ts @@ -1,9 +1,19 @@ import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' -import { rpcUncheckedPayloadReader } from '../transport/rpc-reader-payload' +import { rpcResultVariant } from '../transport/rpc-operation-result-reader' +import { + githubAssignableUsersSchema, + githubRepoLabelsSchema, + githubWorkItemDetailSchema, + gitlabWorkItemDetailSchema, + linearIssueCommentsSchema, + linearIssueSchema, + linearTeamStatesSchema, + linearTeamsSchema +} from './task-item-detail-reply-schema' // What one task item's detail sheet reads: the provider's own detail payload, the Linear comment -// list beside it, and the label, assignee and workflow-state pickers the sheet opens. Every reply -// here is one the call site only re-typed, so the readers are unchecked. +// list beside it, and the label, assignee and workflow-state pickers the sheet opens. Each schema +// lives in task-item-detail-reply-schema.ts with the consumer line behind every requirement. export const githubItemDetailRead = bindDeferredRpcOperation( defineRpcOperation({ @@ -11,7 +21,7 @@ export const githubItemDetailRead = bindDeferredRpcOperation( method: 'github.workItemDetails', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-work-item-details') + read: rpcResultVariant('github-work-item-details', githubWorkItemDetailSchema) }) ) @@ -21,7 +31,7 @@ export const gitlabItemDetailRead = bindDeferredRpcOperation( method: 'gitlab.workItemDetails', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-work-item-details') + read: rpcResultVariant('gitlab-work-item-details', gitlabWorkItemDetailSchema) }) ) @@ -36,7 +46,7 @@ export const linearIssueRead = bindDeferredRpcOperation( method: 'linear.getIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-issue') + read: rpcResultVariant('linear-issue', linearIssueSchema) }) ) @@ -51,7 +61,7 @@ export const linearIssueCommentsRead = bindDeferredRpcOperation( method: 'linear.issueComments', acceptance: 'success-result-or-skip', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-issue-comments') + read: rpcResultVariant('linear-issue-comments', linearIssueCommentsSchema) }) ) @@ -61,7 +71,7 @@ export const githubRepoLabelListRead = bindDeferredRpcOperation( method: 'github.listLabels', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-labels') + read: rpcResultVariant('github-labels', githubRepoLabelsSchema) }) ) @@ -71,7 +81,7 @@ export const githubAssignableUserListRead = bindDeferredRpcOperation( method: 'github.listAssignableUsers', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-assignable-users') + read: rpcResultVariant('github-assignable-users', githubAssignableUsersSchema) }) ) @@ -85,7 +95,7 @@ export const linearTeamStateListRead = bindDeferredRpcOperation( method: 'linear.teamStates', acceptance: 'success-result-or-skip', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-team-states') + read: rpcResultVariant('linear-team-states', linearTeamStatesSchema) }) ) @@ -94,12 +104,16 @@ export const linearTeamStateListRead = bindDeferredRpcOperation( * its picker on a refusal and stays open; hydration in mobile-task-list-operations.ts cannot * proceed without the list and surfaces the host's message. One reader serves both. */ +/** Shared with hydration's leg in the list module: one team list, so the composer's picker and + * the saved-selection reconciler can never disagree about what a team row is. */ +export const linearTeamListReader = rpcResultVariant('linear-teams', linearTeamsSchema) + export const linearComposerTeamListRead = bindDeferredRpcOperation( defineRpcOperation({ name: 'linear.composer-team-list-or-skip', method: 'linear.listTeams', acceptance: 'success-result-or-skip', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-teams') + read: linearTeamListReader }) ) diff --git a/mobile/src/tasks/mobile-task-item-state-operations.ts b/mobile/src/tasks/mobile-task-item-state-operations.ts index fa09d81bc08..125493a348a 100644 --- a/mobile/src/tasks/mobile-task-item-state-operations.ts +++ b/mobile/src/tasks/mobile-task-item-state-operations.ts @@ -1,10 +1,35 @@ import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' -import { rpcUncheckedPayloadReader } from '../transport/rpc-reader-payload' +import { rpcResultVariant } from '../transport/rpc-operation-result-reader' +import { + githubPullRequestChecksSchema, + githubPullRequestFileContentsSchema, + hostedIssueCreatedSchema, + linearIssueCreatedSchema, + linearIssueUpdatedSchema, + taskItemMutationSchema, + taskMutationConfirmationSchema +} from './task-item-state-reply-schema' // The rest of a task item's writes and the PR reads that go with them: creating an item, editing // its metadata or state, reviewers, checks, file contents and viewed state, and merge. A mutation // whose reply is lost stays a transport rejection on the promise, so the screen reports the drop // rather than a failure the host never sent. +// +// Every reader here is checked, and each schema lives in task-item-state-reply-schema.ts with the +// consumer line behind every requirement. Nine of the writes share one reader because they share +// one reply convention; the acceptance and the name stay per operation, which is what a call site +// picks. + +/** + * One reader for the nine writes whose reply is a `{ ok, error }` envelope, not nine readers. + * + * The `ok === false` test and the `error` read are a single convention across both issue edits, + * both pull/merge-request edits, both state toggles, the reviewer request, the checks rerun and + * both merges — there is no input on which two of them would want different answers. Which + * sentence a caller shows on a refusal stays the caller's, because each keeps its own fallback + * copy. + */ +const taskItemMutationReader = rpcResultVariant('task-item-mutation', taskItemMutationSchema) export const githubIssueCreate = bindDeferredRpcOperation( defineRpcOperation({ @@ -12,7 +37,7 @@ export const githubIssueCreate = bindDeferredRpcOperation( method: 'github.createIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-created-issue') + read: rpcResultVariant('github-created-issue', hostedIssueCreatedSchema) }) ) @@ -22,7 +47,7 @@ export const gitlabIssueCreate = bindDeferredRpcOperation( method: 'gitlab.createIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-created-issue') + read: rpcResultVariant('gitlab-created-issue', hostedIssueCreatedSchema) }) ) @@ -33,7 +58,7 @@ export const linearIssueCreate = bindDeferredRpcOperation( method: 'linear.createIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-created-issue') + read: rpcResultVariant('linear-created-issue', linearIssueCreatedSchema) }) ) @@ -43,7 +68,7 @@ export const githubIssueUpdate = bindDeferredRpcOperation( method: 'github.updateIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-updated-issue') + read: taskItemMutationReader }) ) @@ -53,7 +78,7 @@ export const githubPullRequestUpdate = bindDeferredRpcOperation( method: 'github.updatePR', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-updated-pull-request') + read: taskItemMutationReader }) ) @@ -63,7 +88,7 @@ export const githubPullRequestStateUpdate = bindDeferredRpcOperation( method: 'github.updatePRState', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-updated-pull-request-state') + read: taskItemMutationReader }) ) @@ -73,7 +98,7 @@ export const gitlabIssueUpdate = bindDeferredRpcOperation( method: 'gitlab.updateIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-updated-issue') + read: taskItemMutationReader }) ) @@ -83,7 +108,7 @@ export const gitlabMergeRequestUpdate = bindDeferredRpcOperation( method: 'gitlab.updateMR', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-updated-merge-request') + read: taskItemMutationReader }) ) @@ -93,7 +118,7 @@ export const gitlabMergeRequestStateUpdate = bindDeferredRpcOperation( method: 'gitlab.updateMRState', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-updated-merge-request-state') + read: taskItemMutationReader }) ) @@ -103,7 +128,7 @@ export const linearIssueUpdate = bindDeferredRpcOperation( method: 'linear.updateIssue', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-updated-issue') + read: rpcResultVariant('linear-updated-issue', linearIssueUpdatedSchema) }) ) @@ -113,7 +138,7 @@ export const githubReviewerRequest = bindDeferredRpcOperation( method: 'github.requestPRReviewers', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-requested-reviewers') + read: taskItemMutationReader }) ) @@ -124,7 +149,7 @@ export const githubPullRequestChecksRead = bindDeferredRpcOperation( method: 'github.prChecks', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-pr-checks') + read: rpcResultVariant('github-pr-checks', githubPullRequestChecksSchema) }) ) @@ -134,7 +159,7 @@ export const githubPullRequestChecksRerun = bindDeferredRpcOperation( method: 'github.rerunPRChecks', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-rerun-pr-checks') + read: taskItemMutationReader }) ) @@ -144,7 +169,7 @@ export const githubPullRequestFileContentsRead = bindDeferredRpcOperation( method: 'github.prFileContents', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-pr-file-contents') + read: rpcResultVariant('github-pr-file-contents', githubPullRequestFileContentsSchema) }) ) @@ -155,7 +180,7 @@ export const githubPullRequestFileViewedWrite = bindDeferredRpcOperation( method: 'github.setPRFileViewed', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-pr-file-viewed') + read: rpcResultVariant('github-pr-file-viewed', taskMutationConfirmationSchema) }) ) @@ -165,7 +190,7 @@ export const githubPullRequestMerge = bindDeferredRpcOperation( method: 'github.mergePR', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-merged-pull-request') + read: taskItemMutationReader }) ) @@ -175,6 +200,6 @@ export const gitlabMergeRequestMerge = bindDeferredRpcOperation( method: 'gitlab.mergeMR', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-merged-merge-request') + read: taskItemMutationReader }) ) diff --git a/mobile/src/tasks/mobile-task-list-operations.ts b/mobile/src/tasks/mobile-task-list-operations.ts index 0069cd5f63c..dc64fe68320 100644 --- a/mobile/src/tasks/mobile-task-list-operations.ts +++ b/mobile/src/tasks/mobile-task-list-operations.ts @@ -1,5 +1,13 @@ import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation' -import { rpcUncheckedPayloadReader } from '../transport/rpc-reader-payload' +import { rpcResultVariant } from '../transport/rpc-operation-result-reader' +import { linearTeamListReader } from './mobile-task-item-detail-operations' +import { + githubWorkItemCountSchema, + gitlabTodoListSchema, + linearAccountConnectedSchema, + linearAccountStatusSchema, + taskRepoPreferenceWrittenSchema +} from './task-list-reply-schema' // What the Tasks list reads to fill itself for a provider, plus the one write that connects a // Linear account. The per-repo item searches themselves are the Smart picker's operations in @@ -18,7 +26,7 @@ export const linearAccountStatusRead = bindDeferredRpcOperation( method: 'linear.status', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-status') + read: rpcResultVariant('linear-account-status', linearAccountStatusSchema) }) ) @@ -33,7 +41,7 @@ export const linearWorkspaceTeamListRead = bindDeferredRpcOperation( method: 'linear.listTeams', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-teams') + read: linearTeamListReader }) ) @@ -44,7 +52,7 @@ export const githubWorkItemCountRead = bindDeferredRpcOperation( method: 'github.countWorkItems', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('github-work-item-count') + read: rpcResultVariant('github-work-item-count', githubWorkItemCountSchema) }) ) @@ -55,7 +63,7 @@ export const gitlabTodoListRead = bindDeferredRpcOperation( method: 'gitlab.todos', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('gitlab-todos') + read: rpcResultVariant('gitlab-todos', gitlabTodoListSchema) }) ) @@ -69,7 +77,7 @@ export const linearAccountConnect = bindDeferredRpcOperation( method: 'linear.connect', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('linear-connection') + read: rpcResultVariant('linear-account-connected', linearAccountConnectedSchema) }) ) @@ -83,6 +91,6 @@ export const taskRepoPreferenceWrite = bindDeferredRpcOperation( method: 'repo.update', acceptance: 'require-result-or-throw-message', barrier: 'after-caller-barrier', - read: rpcUncheckedPayloadReader('repo-updated') + read: rpcResultVariant('repo-updated', taskRepoPreferenceWrittenSchema) }) ) diff --git a/mobile/src/tasks/mobile-tasks-provider-detail-types.ts b/mobile/src/tasks/mobile-tasks-provider-detail-types.ts index bd07371c07d..da34c8ac5ef 100644 --- a/mobile/src/tasks/mobile-tasks-provider-detail-types.ts +++ b/mobile/src/tasks/mobile-tasks-provider-detail-types.ts @@ -213,11 +213,16 @@ export type GitHubDetailCheck = { url?: string | null } +/** Optional throughout because that is what the checked reader can promise: the recorded reply at + * both call sites carries none of these, so requiring one would refuse this surface's own control. + * Every reader already reaches them through `?.` or through splitContentLines' falsy test. */ export type GitHubPRFileContents = { - original: string - modified: string - originalIsBinary: boolean - modifiedIsBinary: boolean + original?: string + modified?: string + originalIsBinary?: boolean + modifiedIsBinary?: boolean + originalTooLarge?: boolean + modifiedTooLarge?: boolean } export type DetailPayload = diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index f91b59e05a4..ea1a8aab69b 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -26,11 +26,18 @@ const hash = (parts: string[] | string): string => // and style counts are all unchanged, and `semantics` is a pure deletion of four lines, none in: // two `rpc:` call signatures and the two method literals they carried. The render-token hash moves // because the picker's handler now names an operation instead of the client. -const SCREEN_RPC_SCREEN_HOOKS = '1b455d87ed00a1e70a5b3cac0110272e818da9a0d245e9043fc9d2649587831f' +// +// Step 7's first half moves four of the six again, and moves nothing else. Checked readers on the +// item and list operations delete the reply casts these consumers carried, plus the three shape +// tests the reader now answers for: both `Array.isArray(payload)` guards on the checks read and the +// `typeof count === 'number'` fallback on the item count. Hook, statement, declaration and render +// counts are unchanged, and the render-token hash does not move at all — nothing this family sees +// changed inside a JSX tree. `semantics` is a pure deletion of ten lines. +const SCREEN_RPC_SCREEN_HOOKS = 'fd1c59e2b923dcc54218c2b4df5155fcf98b3dcd7894849d145999202402ad23' const PRE_REFACTOR_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const SCREEN_RPC_STATEMENTS = '67ea80f265e4a2e25b3d7e7d9b93664a150a27b93dcfbc39cbdd551de7b4a653' -const MAIN_REBASED_DECLARATIONS = '6ad0397123e59fc1047a14049c86ff31d81723673a7a7f5c41677471aec58415' -const SCREEN_RPC_SEMANTICS = '7e40c7efa07993071e57db0fe1d46099a56e3831033b512480dd195d7a1dc24c' +const SCREEN_RPC_STATEMENTS = 'ac507dc6871bb3ec8cb97f2c5af1a5ac9a24d6d327ad31bbf1e02caf455617f5' +const MAIN_REBASED_DECLARATIONS = 'e0a402cdb6819dc685f5cc41c543bf5f1ac5366de064b90ad225a159bfcabfc5' +const SCREEN_RPC_SEMANTICS = '3e729bc760428e4701cdfa87c5e51c4301524d96a4e79bff7af9eb403153d76c' const PRE_REFACTOR_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' const SCREEN_RPC_RENDER_TREE = '46d5a3ce9d71a8281a1e7b17411fb1dd963a4f392a5d095bc126b6a7cff4b92d' @@ -59,7 +66,7 @@ describe('Mobile Tasks refactor parity', () => { it('preserves RPC calls, runtime strings, and JSX host signatures', () => { const semantics = readMobileTasksSemanticSource() - expect(semantics.split('\n')).toHaveLength(3_300) + expect(semantics.split('\n')).toHaveLength(3_290) expect(hash(semantics)).toBe(SCREEN_RPC_SEMANTICS) }) diff --git a/mobile/src/tasks/task-item-comment-reply-schema.test.ts b/mobile/src/tasks/task-item-comment-reply-schema.test.ts new file mode 100644 index 00000000000..79bd8a8a2d6 --- /dev/null +++ b/mobile/src/tasks/task-item-comment-reply-schema.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import type { z } from 'zod' +import { + linearCommentWrittenSchema, + reviewThreadResolvedSchema, + taskCommentWrittenSchema +} from './task-item-comment-reply-schema' + +function reads(schema: z.ZodType, value: unknown): T { + const parsed = schema.safeParse(value) + if (!parsed.success) { + throw new Error(`expected a readable reply: ${parsed.error.message}`) + } + return parsed.data +} + +function refuses(schema: z.ZodType, value: unknown): boolean { + return !schema.safeParse(value).success +} + +describe('the five writes that answer with a comment', () => { + it('reads the recorded GitHub, GitLab and review-reply envelopes unchanged', () => { + for (const comment of [ + { id: 902, author: 'You', body: 'a comment', createdAt: '2020-01-01T00:00:00.000Z' }, + { + id: 903, + author: 'You', + body: 'a reply', + createdAt: '2020-01-01T00:00:00.000Z', + path: 'src/index.ts', + line: 12, + threadId: 'thread-1' + } + ]) { + expect(reads(taskCommentWrittenSchema, { ok: true, comment }).comment).toMatchObject(comment) + } + }) + + it('keeps ok tri-state and leaves the refusal wording to the call site', () => { + expect(reads(taskCommentWrittenSchema, { ok: false }).error).toBeUndefined() + expect(reads(taskCommentWrittenSchema, { ok: false, error: 'no' }).error).toBe('no') + expect(reads(taskCommentWrittenSchema, {}).ok).toBeUndefined() + }) + + it('names a reply that is not the envelope, which main read members off', () => { + expect(refuses(taskCommentWrittenSchema, null)).toBe(true) + expect(refuses(taskCommentWrittenSchema, 'posted')).toBe(true) + }) +}) + +describe('the Linear comment write', () => { + it('reads the recorded reply and leaves a missing id to the local echo', () => { + expect(reads(linearCommentWrittenSchema, { ok: true, id: 'comment-9' }).id).toBe('comment-9') + expect(reads(linearCommentWrittenSchema, { ok: true }).id).toBeUndefined() + expect(refuses(linearCommentWrittenSchema, null)).toBe(true) + }) +}) + +describe('resolving a review thread', () => { + it('keeps a real false, and names a reply that is not a boolean', () => { + expect(reads(reviewThreadResolvedSchema, true)).toBe(true) + expect(reads(reviewThreadResolvedSchema, false)).toBe(false) + expect(refuses(reviewThreadResolvedSchema, 'true')).toBe(true) + expect(refuses(reviewThreadResolvedSchema, undefined)).toBe(true) + }) +}) diff --git a/mobile/src/tasks/task-item-comment-reply-schema.ts b/mobile/src/tasks/task-item-comment-reply-schema.ts new file mode 100644 index 00000000000..66ca519a338 --- /dev/null +++ b/mobile/src/tasks/task-item-comment-reply-schema.ts @@ -0,0 +1,43 @@ +import { z } from 'zod' +import { prFlag, prText } from '../session/github-pr-entity-reply-schema' +import { taskCommentWriteEnvelopeSchema } from './task-provider-entity-reply-schema' + +// What a comment write on a task item answers with, over all three providers. Checked against the +// handlers in src/main/runtime/rpc/methods/ — github-issue-methods.ts:34-39, +// github-pull-request-methods.ts, gitlab.ts, linear.ts:88-93 — and GitHubCommentResult in +// src/shared/github/comment-types.ts, which every GitHub and GitLab comment write returns. + +/** + * The five writes that answer with a comment: both GitHub issue-comment paths, the review comment + * and its reply, and both GitLab paths. + * + * Nothing past the container is required. Every call site reads `ok === false`, raises + * `error ?? `, and falls back to a locally built row when the reply carries no + * `comment` (use-mobile-tasks-hosted-comment-review-actions.tsx:103-111 is the shape of all five). + * The reply's comment is checked rather than adopted blind, so a row the timeline could not key or + * render leaves that same local echo in place instead of reaching the list as a blank bubble. + */ +export const taskCommentWrittenSchema = taskCommentWriteEnvelopeSchema + +/** + * Linear's comment write, which answers with an id rather than a comment. + * + * `id` is optional because use-mobile-tasks-linear-item-actions.tsx:58 reads + * `result.id ?? 'local-'`: a reply without one still puts the comment the user typed on the + * sheet, and that is the behaviour worth keeping. + */ +export const linearCommentWrittenSchema = z.looseObject({ + ok: prFlag('ok'), + error: prText('error'), + id: prText('id') +}) + +/** + * Resolving or reopening a review thread. + * + * The same boolean reader the file-viewed sync uses, and the session domain's two boolean + * mutations before it: `interpret(reply) !== true` is the rule at both call sites, so a reply that + * is not a boolean read as "the write did not happen" — indistinguishable from a host that refused + * it. A real `false` still reaches that rule and still shows the call site's own copy. + */ +export { githubPrMutationConfirmationSchema as reviewThreadResolvedSchema } from '../session/github-pr-mutation-reply-schema' diff --git a/mobile/src/tasks/task-item-detail-reply-schema.test.ts b/mobile/src/tasks/task-item-detail-reply-schema.test.ts new file mode 100644 index 00000000000..480e710522a --- /dev/null +++ b/mobile/src/tasks/task-item-detail-reply-schema.test.ts @@ -0,0 +1,206 @@ +import { describe, expect, it } from 'vitest' +import type { z } from 'zod' +import { + githubAssignableUsersSchema, + githubRepoLabelsSchema, + githubWorkItemDetailSchema, + gitlabWorkItemDetailSchema, + linearIssueCommentsSchema, + linearIssueSchema, + linearTeamStatesSchema, + linearTeamsSchema +} from './task-item-detail-reply-schema' + +function reads(schema: z.ZodType, value: unknown): T { + const parsed = schema.safeParse(value) + if (!parsed.success) { + throw new Error(`expected a readable reply: ${parsed.error.message}`) + } + return parsed.data +} + +function refuses(schema: z.ZodType, value: unknown): boolean { + return !schema.safeParse(value).success +} + +const LINEAR_ISSUE = { + id: 'issue-2', + identifier: 'ENG-2', + title: 'A sub-issue', + url: '', + description: 'a description', + state: { name: 'Todo', type: 'unstarted', color: '#000' }, + team: { id: 'team-1', key: 'ENG', name: 'Engineering' }, + labels: [], + priority: 0, + updatedAt: '2020-01-01T00:00:00.000Z', + workspaceId: 'linear-workspace', + subIssues: [] +} + +describe('the GitHub detail pane', () => { + it('reads the recorded reply and keeps every member the sheet reads behind a guard', () => { + const details = reads(githubWorkItemDetailSchema, { + body: 'body', + comments: [], + item: { labels: ['bug'], reviewDecision: 'APPROVED', reviewRequests: [], latestReviews: [] }, + assignees: ['octocat'], + headSha: 'head-sha', + baseSha: 'base-sha', + pullRequestId: 'PR_kwDO', + checks: [], + files: [] + }) + expect(details).toMatchObject({ body: 'body', headSha: 'head-sha', assignees: ['octocat'] }) + expect(details?.item?.reviewDecision).toBe('APPROVED') + }) + + it('answers null for the null the host sends, and names anything that is not the container', () => { + expect(reads(githubWorkItemDetailSchema, null)).toBeNull() + expect(refuses(githubWorkItemDetailSchema, 'nothing here')).toBe(true) + expect(refuses(githubWorkItemDetailSchema, 7)).toBe(true) + }) + + it('keeps an explicit reviewDecision null, which the call site falls back from itself', () => { + const details = reads(githubWorkItemDetailSchema, { item: { reviewDecision: null } }) + expect(details?.item?.reviewDecision).toBeNull() + }) + + it('requires nothing inside, because the sheet reads every member behind ?? or ?.', () => { + expect(reads(githubWorkItemDetailSchema, {})).toEqual({}) + }) +}) + +describe('the GitLab detail pane', () => { + it('reads the recorded reply, approval state and pipeline jobs included', () => { + const details = reads(gitlabWorkItemDetailSchema, { + body: 'body', + comments: [], + item: { labels: ['bug'], mergeable: 'MERGEABLE' }, + assignees: [], + pipelineJobs: [], + reviewers: [], + approvalState: { approvalsRequired: 1, approvalsLeft: 0 } + }) + expect(details?.item?.mergeable).toBe('MERGEABLE') + expect(details?.approvalState).toEqual({ approvalsRequired: 1, approvalsLeft: 0 }) + }) + + it('carries every mergeable arm and degrades one it has not heard of to absent', () => { + for (const mergeable of ['MERGEABLE', 'CONFLICTING', 'UNKNOWN']) { + expect(reads(gitlabWorkItemDetailSchema, { item: { mergeable } })?.item?.mergeable).toBe( + mergeable + ) + } + expect( + reads(gitlabWorkItemDetailSchema, { item: { mergeable: 'BLOCKED' } })?.item?.mergeable + ).toBeUndefined() + }) + + it('keeps an explicit null approval count, which the reviewDecision ladder reads', () => { + const details = reads(gitlabWorkItemDetailSchema, { + approvalState: { approvalsRequired: null, approvalsLeft: null } + }) + expect(details?.approvalState).toEqual({ approvalsRequired: null, approvalsLeft: null }) + }) + + it('drops a pipeline job the summary could not classify', () => { + const jobs = reads(gitlabWorkItemDetailSchema, { + pipelineJobs: [ + { id: 1, name: 'build', stage: 'build', status: 'success', webUrl: null, duration: null }, + { id: 2, stage: 'test', status: 'failed' } + ] + })?.pipelineJobs + expect(jobs).toHaveLength(1) + expect(jobs?.[0]).toMatchObject({ name: 'build', duration: null }) + }) +}) + +describe('one Linear issue', () => { + it('reads the recorded reply whole', () => { + expect(reads(linearIssueSchema, LINEAR_ISSUE)).toMatchObject({ id: 'issue-2', priority: 0 }) + }) + + it('answers null for the null getIssue sends when the workspace cannot see the issue', () => { + expect(reads(linearIssueSchema, null)).toBeNull() + }) + + it('refuses a reply missing a member createLinearTask reads with no guard', () => { + for (const key of ['id', 'identifier', 'title', 'updatedAt', 'url', 'priority', 'labels']) { + const partial: Record = { ...LINEAR_ISSUE } + delete partial[key] + expect(refuses(linearIssueSchema, partial)).toBe(true) + } + expect(refuses(linearIssueSchema, { ...LINEAR_ISSUE, team: { id: 't', key: 'ENG' } })).toBe( + true + ) + expect(refuses(linearIssueSchema, { ...LINEAR_ISSUE, state: { type: 'x', color: '#0' } })).toBe( + true + ) + }) + + it('keeps an explicit estimate null, which is the host own "no estimate"', () => { + expect(reads(linearIssueSchema, { ...LINEAR_ISSUE, estimate: null })?.estimate).toBeNull() + expect(reads(linearIssueSchema, { ...LINEAR_ISSUE, estimate: 3 })?.estimate).toBe(3) + expect(reads(linearIssueSchema, LINEAR_ISSUE)?.estimate).toBeUndefined() + }) + + it('drops a sub-issue row the children list could not open', () => { + const withChildren = { + ...LINEAR_ISSUE, + subIssues: [ + { id: 'c-1', identifier: 'ENG-9', title: 'child', url: 'https://linear.app/x' }, + { id: 'c-2', identifier: 'ENG-10' } + ] + } + expect(reads(linearIssueSchema, withChildren)?.subIssues).toHaveLength(1) + }) + + it('drops a label element that is not text rather than failing the sheet', () => { + expect(reads(linearIssueSchema, { ...LINEAR_ISSUE, labels: ['bug', 7] })?.labels).toEqual([ + 'bug' + ]) + }) +}) + +describe('the lists beside the sheet', () => { + it('reads a comment list, and reads nullish as the empty list the call site already read', () => { + expect( + reads(linearIssueCommentsSchema, [ + { id: 'comment-1', body: 'a comment', createdAt: '2020-01-01T00:00:00.000Z' } + ]) + ).toHaveLength(1) + expect(reads(linearIssueCommentsSchema, null)).toBeNull() + expect(reads(linearIssueCommentsSchema, undefined)).toBeUndefined() + expect(refuses(linearIssueCommentsSchema, 'none')).toBe(true) + }) + + it('reads the label vocabulary and drops an element that is not a chip', () => { + expect(reads(githubRepoLabelsSchema, ['bug', 'chore'])).toEqual(['bug', 'chore']) + expect(reads(githubRepoLabelsSchema, ['bug', { name: 'chore' }])).toEqual(['bug']) + expect(refuses(githubRepoLabelsSchema, { labels: [] })).toBe(true) + }) + + it('reads the recorded assignable users with their null avatar intact', () => { + expect( + reads(githubAssignableUsersSchema, [{ login: 'octocat', name: 'Octo', avatarUrl: null }]) + ).toEqual([{ login: 'octocat', name: 'Octo', avatarUrl: null }]) + }) + + it('requires a workflow state id, name and type, and leaves colour to the call site', () => { + expect( + reads(linearTeamStatesSchema, [{ id: 'state-1', name: 'Todo', type: 'unstarted' }]) + ).toHaveLength(1) + expect(reads(linearTeamStatesSchema, [{ id: 'state-1', name: 'Todo' }])).toEqual([]) + }) + + it('requires a team id, name and key, which both team readers depend on', () => { + expect( + reads(linearTeamsSchema, [ + { id: 'team-1', key: 'ENG', name: 'Engineering', workspaceId: 'linear-workspace' } + ]) + ).toHaveLength(1) + expect(reads(linearTeamsSchema, [{ id: 'team-1', name: 'Engineering' }])).toEqual([]) + expect(refuses(linearTeamsSchema, null)).toBe(true) + }) +}) diff --git a/mobile/src/tasks/task-item-detail-reply-schema.ts b/mobile/src/tasks/task-item-detail-reply-schema.ts new file mode 100644 index 00000000000..e726bf750f8 --- /dev/null +++ b/mobile/src/tasks/task-item-detail-reply-schema.ts @@ -0,0 +1,217 @@ +import { z } from 'zod' +import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' +import { + prCount, + prNullableText, + prStringList, + prText +} from '../session/github-pr-entity-reply-schema' +import { + assignableUserListSchema, + detailCheckListSchema, + detailCommentListSchema, + detailFileListSchema, + reviewSummaryListSchema +} from './task-provider-entity-reply-schema' + +// What one task item's detail sheet reads. Checked against the handlers in +// src/main/runtime/rpc/methods/ — github-repo-work-item-methods.ts:52-87, gitlab.ts:175-178, +// linear.ts:77-104 and :169-172 — and the shared results they return: GitHubWorkItemDetails in +// src/shared/github/work-item-types.ts, GitLabWorkItemDetails in src/shared/gitlab-types.ts, +// LinearIssue in src/shared/linear/issue-types.ts, and `Promise` from +// src/main/github/issue-field-options.ts:15. + +const MERGEABLE_STATE = ['MERGEABLE', 'CONFLICTING', 'UNKNOWN'] as const + +/** + * A GitHub work item's detail pane. + * + * Object or `null`, and nothing inside is required: `if (!details) throw` at + * use-mobile-tasks-item-detail-loading.tsx:85 is the whole identity test, and :91-102 reads every + * member behind `??` or `?.`. What the schema adds is the container — main read `details.body` off + * a string reply and published an empty sheet as if the host had answered, and off `null` it threw + * a property-read TypeError the sheet showed verbatim. + * + * `reviewDecision` keeps an explicit `null`, because the call site's own `?? actionItem.source + * .reviewDecision` is what decides whether the host's null or the row's value wins; collapsing it + * here would take that decision away from the call site. + */ +export const githubWorkItemDetailSchema = z + .looseObject({ + body: prText('body'), + comments: salvagedOptional('comments', detailCommentListSchema), + item: salvagedOptional( + 'item', + z.looseObject({ + labels: prStringList('labels'), + reviewDecision: prNullableText('reviewDecision'), + reviewRequests: salvagedOptional('reviewRequests', assignableUserListSchema), + latestReviews: salvagedOptional('latestReviews', reviewSummaryListSchema) + }) + ), + assignees: prStringList('assignees'), + headSha: prText('headSha'), + baseSha: prText('baseSha'), + pullRequestId: prText('pullRequestId'), + checks: salvagedOptional('checks', detailCheckListSchema), + files: salvagedOptional('files', detailFileListSchema) + }) + .nullable() + +/** + * A GitLab work item's detail pane, read the same way and required the same amount: not at all + * past the container (use-mobile-tasks-item-detail-loading.tsx:136-158). + * + * `mergeable` is a closed arm set that degrades to absent rather than to an arm. The three arms + * are what the row's merge affordance is keyed on, so coercing an arm this build has not heard of + * into one of them would offer or withhold a merge against a state the client cannot place; + * dropping the member leaves the row exactly as the list had it, which is what main did for a + * detail reply that carried no `mergeable` at all. + */ +export const gitlabWorkItemDetailSchema = z + .looseObject({ + body: prText('body'), + comments: salvagedOptional('comments', detailCommentListSchema), + item: salvagedOptional( + 'item', + z.looseObject({ + labels: prStringList('labels'), + mergeable: salvagedOptional('mergeable', z.enum(MERGEABLE_STATE)) + }) + ), + assignees: prStringList('assignees'), + pipelineJobs: salvagedOptional( + 'pipelineJobs', + salvagingArray( + z.looseObject({ + id: prCount('id'), + name: z.string(), + stage: z.string(), + status: z.string(), + webUrl: prNullableText('webUrl'), + duration: salvagedOptional('duration', z.number().finite().nullable()) + }) + ) + ), + reviewers: salvagedOptional('reviewers', z.array(z.unknown())), + approvalState: salvagedOptional( + 'approvalState', + z.looseObject({ + approvalsRequired: z.number().finite().nullable(), + approvalsLeft: z.number().finite().nullable() + }) + ) + }) + .nullable() + +/** + * One Linear issue, or `null` for an issue this workspace cannot see — which is what + * `getIssue` (src/main/linear/linear-issue-lookups.ts:33) answers, and what both call sites + * already report as "not found". + * + * This is the one detail reply with required members, because `createLinearTask` + * (mobile-tasks-item-mapping.ts:291-300) reads six of them with no guard: `id`, `title`, + * `identifier`, `updatedAt`, `team.name` and `state.name`. `url`, `labels` and `priority` join + * them because the shared type declares them non-optional and the row renders them unguarded, and + * the recorded reply at every site carries all nine. + * + * `estimate` keeps an explicit `null`: the host writes `issue.estimate ?? null` + * (src/main/linear/mappers.ts:112), so `null` is the value "no estimate" and absence is a host + * that did not report one. + */ +export const linearIssueSchema = z + .looseObject({ + id: z.string(), + identifier: z.string(), + title: z.string(), + url: z.string(), + updatedAt: z.string(), + priority: z.number().finite(), + labels: salvagingArray(z.string()), + state: z.looseObject({ name: z.string(), type: z.string(), color: z.string() }), + team: z.looseObject({ id: z.string(), name: z.string(), key: z.string() }), + workspaceId: prText('workspaceId'), + workspaceName: prText('workspaceName'), + description: prText('description'), + labelIds: prStringList('labelIds'), + estimate: salvagedOptional('estimate', z.number().finite().nullable()), + assignee: salvagedOptional( + 'assignee', + z.looseObject({ id: prText('id'), displayName: z.string() }) + ), + project: salvagedOptional( + 'project', + z.looseObject({ + id: z.string(), + name: z.string(), + url: prText('url'), + color: prText('color') + }) + ), + subIssues: salvagedOptional( + 'subIssues', + salvagingArray( + z.looseObject({ + id: z.string(), + identifier: z.string(), + title: z.string(), + url: z.string() + }) + ) + ) + }) + .nullable() + +/** + * The comment list beside a Linear issue. + * + * Nullish as well as an array, because the call site reads it as `accepted.value ?? []` + * (use-mobile-tasks-item-detail-loading.tsx:215): a host that answers `null` still means "no + * comments", and rejecting it would turn a reply main rendered into an error the sheet shows. + */ +export const linearIssueCommentsSchema = detailCommentListSchema.nullish() + +/** + * The repo's label vocabulary, for the label picker. + * + * `listLabels` returns `string[]`, and the picker maps it unguarded, so an element that is not a + * string drops rather than rendering `undefined` as a chip. + */ +export const githubRepoLabelsSchema = salvagingArray(z.string()) + +/** The repo's assignable users, keyed by `login` the way every reader of this list is. */ +export const githubAssignableUsersSchema = assignableUserListSchema + +/** + * A Linear team's workflow states, for the status picker. + * + * `id`, `name` and `type` are what `LinearState` declares non-optional and what the picker rows + * and `setLinearStatus` read unguarded; `color` is reached through + * `state.color ?? item.source.state.color` (use-mobile-tasks-github-reply-merge-actions.tsx:211). + */ +export const linearTeamStatesSchema = salvagingArray( + z.looseObject({ + id: z.string(), + name: z.string(), + type: z.string(), + color: prText('color') + }) +) + +/** + * A Linear workspace's teams. One reader for the composer picker and for provider hydration, which + * disagree only about what a refusal means. + * + * `id`, `name` and `key` are `LinearTeam`'s own required members; hydration's + * `reconcileTeamSelection` maps `team.id` with no guard + * (mobile-tasks-reviewer-linear.ts:204), and the composer labels each row by name and key. + */ +export const linearTeamsSchema = salvagingArray( + z.looseObject({ + id: z.string(), + name: z.string(), + key: z.string(), + workspaceId: prText('workspaceId'), + workspaceName: prText('workspaceName') + }) +) diff --git a/mobile/src/tasks/task-item-state-reply-schema.test.ts b/mobile/src/tasks/task-item-state-reply-schema.test.ts new file mode 100644 index 00000000000..d173b803e83 --- /dev/null +++ b/mobile/src/tasks/task-item-state-reply-schema.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'vitest' +import type { z } from 'zod' +import { + githubPullRequestChecksSchema, + githubPullRequestFileContentsSchema, + hostedIssueCreatedSchema, + linearIssueCreatedSchema, + linearIssueUpdatedSchema, + taskItemMutationSchema, + taskMutationConfirmationSchema +} from './task-item-state-reply-schema' + +function reads(schema: z.ZodType, value: unknown): T { + const parsed = schema.safeParse(value) + if (!parsed.success) { + throw new Error(`expected a readable reply: ${parsed.error.message}`) + } + return parsed.data +} + +function refuses(schema: z.ZodType, value: unknown): boolean { + return !schema.safeParse(value).success +} + +describe('creating an item', () => { + it('reads the recorded GitHub and GitLab create replies unchanged', () => { + const created = reads(hostedIssueCreatedSchema, { + ok: true, + number: 11, + url: 'https://github.com/owner/repo/issues/11' + }) + expect(created).toMatchObject({ ok: true, number: 11 }) + }) + + it('reads a number the composer would not have accepted as absent', () => { + expect(reads(hostedIssueCreatedSchema, { ok: true, number: '11' }).number).toBeUndefined() + }) + + it('leaves id and identifier to the Linear call site, which words that refusal itself', () => { + expect(reads(linearIssueCreatedSchema, { ok: true }).id).toBeUndefined() + expect( + reads(linearIssueCreatedSchema, { ok: true, id: 'issue-3', identifier: 'ENG-3' }).identifier + ).toBe('ENG-3') + expect(refuses(linearIssueCreatedSchema, null)).toBe(true) + }) +}) + +describe('the nine writes that share one reader', () => { + it('is the mutation envelope, not a second copy of it', () => { + expect(reads(taskItemMutationSchema, { ok: true })).toMatchObject({ ok: true }) + expect(refuses(taskItemMutationSchema, null)).toBe(true) + }) +}) + +describe('the checks read', () => { + it('reads the recorded reply and refuses a payload that is not a list', () => { + expect( + reads(githubPullRequestChecksSchema, [ + { name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS', url: '' } + ]) + ).toHaveLength(1) + expect(refuses(githubPullRequestChecksSchema, { checks: [] })).toBe(true) + expect(refuses(githubPullRequestChecksSchema, null)).toBe(true) + }) +}) + +describe('the file-contents read', () => { + it('reads the recorded reply through untouched, because nothing in it is required', () => { + const recorded = { oldContent: 'a', newContent: 'b', truncated: false } + expect(reads(githubPullRequestFileContentsSchema, recorded)).toEqual(recorded) + }) + + it('still names a payload that is not the container', () => { + expect(refuses(githubPullRequestFileContentsSchema, null)).toBe(true) + expect(refuses(githubPullRequestFileContentsSchema, 'a\nb')).toBe(true) + }) + + it('reads the host contract when a host sends it', () => { + const host = { original: 'a', modified: 'b', originalIsBinary: false, modifiedIsBinary: false } + expect(reads(githubPullRequestFileContentsSchema, host)).toMatchObject(host) + }) +}) + +describe('the two unread replies', () => { + it('reads anything for the Linear state write, whose body no call site looks at', () => { + expect(refuses(linearIssueUpdatedSchema, null)).toBe(false) + expect(refuses(linearIssueUpdatedSchema, 'accepted')).toBe(false) + }) +}) + +describe('the viewed-state confirmation', () => { + it('keeps a real false, which is the refusal the call site words', () => { + expect(reads(taskMutationConfirmationSchema, true)).toBe(true) + expect(reads(taskMutationConfirmationSchema, false)).toBe(false) + }) + + it('names a non-boolean instead of reading it as "not confirmed"', () => { + expect(refuses(taskMutationConfirmationSchema, 'true')).toBe(true) + expect(refuses(taskMutationConfirmationSchema, { ok: true })).toBe(true) + expect(refuses(taskMutationConfirmationSchema, null)).toBe(true) + }) +}) diff --git a/mobile/src/tasks/task-item-state-reply-schema.ts b/mobile/src/tasks/task-item-state-reply-schema.ts new file mode 100644 index 00000000000..fd47565a7eb --- /dev/null +++ b/mobile/src/tasks/task-item-state-reply-schema.ts @@ -0,0 +1,108 @@ +import { z } from 'zod' +import { prCount, prFlag, prText } from '../session/github-pr-entity-reply-schema' +import { + detailCheckListSchema, + taskMutationEnvelopeSchema +} from './task-provider-entity-reply-schema' + +// What a task item's writes answer with, plus the two PR reads that go with them. Checked against +// the handlers in src/main/runtime/rpc/methods/ — github-issue-methods.ts:16-33, +// github-pull-request-methods.ts:84-92, github-pull-request-update-methods.ts, gitlab.ts, +// linear.ts:58-87 — and the shared results they return: GitHubCreateIssueResult and +// GitHubIssueUpdate's `{ ok } | { ok, error }` in src/shared/issue-mutation-types.ts, +// GitHubCommentResult in src/shared/github/comment-types.ts, and GitHubPRFileContents in +// src/shared/github/pull-request-types.ts. + +/** + * Creating a GitHub or GitLab issue. + * + * Nothing is required beyond the envelope itself: use-mobile-tasks-task-create-actions.tsx:83 + * tests `ok === false`, :88 gates the optimistic row on `typeof number === 'number'` and :98 reads + * `url ?? ''`. What the schema adds is the container — main read `result.ok` off a string reply + * and silently reported success, and off a `null` one it threw a property-read TypeError. + */ +export const hostedIssueCreatedSchema = z.looseObject({ + ok: prFlag('ok'), + error: prText('error'), + number: prCount('number'), + url: prText('url') +}) + +/** + * Creating a Linear issue, from the composer or from the sub-issue field. + * + * Also all-optional, for the same reason: both call sites gate on + * `result.ok === false || !result.id || !result.identifier` + * (use-mobile-tasks-task-create-actions.tsx:140, use-mobile-tasks-linear-item-actions.tsx:138) and + * read `title` and `url` behind `??`. `id` and `identifier` are therefore a refusal the call site + * already words, not a decode failure. + */ +export const linearIssueCreatedSchema = z.looseObject({ + ok: prFlag('ok'), + error: prText('error'), + id: prText('id'), + identifier: prText('identifier'), + title: prText('title'), + url: prText('url') +}) + +/** + * The state and metadata writes: both issue edits, both pull/merge-request edits, both state + * toggles, the reviewer request, the checks rerun and both merges. + * + * One schema for nine methods, because there is one convention and no input on which two of them + * would want different answers: every call site reads `ok === false` and raises `error` or its own + * copy. Kept separate from the session domain's `githubPrMutationStatusSchemas` even where the + * method matches, because that reader answers a `{ structured, ok, error }` verdict its own + * outcome module discriminates, where these call sites read the two members directly. + */ +export const taskItemMutationSchema = taskMutationEnvelopeSchema + +/** + * The checks list behind the item sheet's Checks panel and the project row's. + * + * An array, and each row needs the `name` and `status` the list renders unguarded; a row without + * either drops rather than failing the refresh. Both call sites hand the decoded list straight to + * `buildGitHubCheckSummary`, whose classifier reads the same two members + * (use-mobile-tasks-hosted-comment-review-actions.tsx:256). + */ +export const githubPullRequestChecksSchema = detailCheckListSchema + +/** + * One file's two sides of a pull-request diff. + * + * Every member is optional even though `getPRFileContents` + * (src/main/github/pull-request-file-contents.ts:121) always returns the first four. The corpus is + * why: the recorded `normal` reply at both sites is `{ oldContent, newContent, truncated }`, a + * shape the host cannot produce, so requiring `original` would reject this surface's only success + * control. The call site reads nothing off the payload — it files it under the file path and the + * diff view reads it later — so nothing here is a member this reader can justify requiring. + * What the schema does add is the container: a string or a `null` reply is now named. + */ +export const githubPullRequestFileContentsSchema = z.looseObject({ + original: prText('original'), + modified: prText('modified'), + originalIsBinary: prFlag('originalIsBinary'), + modifiedIsBinary: prFlag('modifiedIsBinary'), + originalTooLarge: prFlag('originalTooLarge'), + modifiedTooLarge: prFlag('modifiedTooLarge') +}) + +/** + * Syncing one file's viewed state. + * + * `z.boolean()`, the same reader the session domain's two boolean mutations use: `!== true` is the + * confirmation rule at both call sites (use-mobile-tasks-project-review-check-actions.tsx:219, + * use-mobile-tasks-github-check-file-actions.tsx:99), so a non-boolean read as "not confirmed" was + * indistinguishable from a host that declined the write. A real `false` still reaches that rule. + */ +export { githubPrMutationConfirmationSchema as taskMutationConfirmationSchema } from '../session/github-pr-mutation-reply-schema' + +/** + * Setting a Linear issue's workflow state. + * + * The reply body is unread: use-mobile-tasks-github-reply-merge-actions.tsx:207 interprets the + * envelope for its acceptance and looks at nothing in it, so there is no member to declare. The + * acceptance still carries a refusal to the callback's `catch`, which is the whole verdict here. + */ +export const linearIssueUpdatedSchema = z.unknown() diff --git a/mobile/src/tasks/task-list-reply-schema.test.ts b/mobile/src/tasks/task-list-reply-schema.test.ts new file mode 100644 index 00000000000..43657348475 --- /dev/null +++ b/mobile/src/tasks/task-list-reply-schema.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from 'vitest' +import type { z } from 'zod' +import { + githubWorkItemCountSchema, + gitlabTodoListSchema, + linearAccountConnectedSchema, + linearAccountStatusSchema, + taskRepoPreferenceWrittenSchema +} from './task-list-reply-schema' + +function reads(schema: z.ZodType, value: unknown): T { + const parsed = schema.safeParse(value) + if (!parsed.success) { + throw new Error(`expected a readable reply: ${parsed.error.message}`) + } + return parsed.data +} + +function refuses(schema: z.ZodType, value: unknown): boolean { + return !schema.safeParse(value).success +} + +describe('Linear account status', () => { + it('reads the recorded reply, workspace row included', () => { + const status = reads(linearAccountStatusSchema, { + connected: true, + workspaces: [{ id: 'linear-workspace', name: 'Workspace' }], + selectedWorkspaceId: 'linear-workspace' + }) + expect(status.connected).toBe(true) + expect(status.workspaces?.[0]).toMatchObject({ id: 'linear-workspace', name: 'Workspace' }) + }) + + it('reads a disconnected host the same way every settings family records it', () => { + expect(reads(linearAccountStatusSchema, { connected: false }).connected).toBe(false) + }) + + it('names the null main read `connected` off, which the screen showed as its load error', () => { + expect(refuses(linearAccountStatusSchema, null)).toBe(true) + expect(refuses(linearAccountStatusSchema, 'connected')).toBe(true) + }) + + it('keeps an explicit selectedWorkspaceId null, which the ?? ladder is what interprets', () => { + const status = reads(linearAccountStatusSchema, { + connected: true, + selectedWorkspaceId: null, + activeWorkspaceId: null + }) + expect(status.selectedWorkspaceId).toBeNull() + expect(status.activeWorkspaceId).toBeNull() + }) + + it('drops only the workspace row with no id, which can be neither selected nor matched', () => { + expect(reads(linearAccountStatusSchema, { workspaces: [{ name: 'W' }] }).workspaces).toEqual([]) + expect( + reads(linearAccountStatusSchema, { workspaces: [{ id: 'w-1' }, { name: 'W' }] }).workspaces + ).toEqual([{ id: 'w-1' }]) + expect(reads(linearAccountStatusSchema, { workspaces: 'none' }).workspaces).toBeUndefined() + }) +}) + +describe('the GitHub item count', () => { + it('reads the recorded number and names anything else', () => { + expect(reads(githubWorkItemCountSchema, 4)).toBe(4) + expect(refuses(githubWorkItemCountSchema, '4')).toBe(true) + expect(refuses(githubWorkItemCountSchema, { count: 4 })).toBe(true) + expect(refuses(githubWorkItemCountSchema, null)).toBe(true) + }) +}) + +describe('the GitLab to-do inbox', () => { + it('reads the recorded reply through untouched, row and all', () => { + const recorded = [ + { + id: 1, + targetType: 'Issue', + target: { id: 'gid://1', iid: 4, title: 'A GitLab todo', webUrl: '' } + } + ] + expect(reads(gitlabTodoListSchema, recorded)).toEqual(recorded) + }) + + it('reads nullish as the empty inbox the call site already read', () => { + expect(reads(gitlabTodoListSchema, null)).toBeNull() + expect(reads(gitlabTodoListSchema, undefined)).toBeUndefined() + }) + + it('names a reply that is neither, which the screen showed as ".map is not a function"', () => { + expect(refuses(gitlabTodoListSchema, { todos: [] })).toBe(true) + expect(refuses(gitlabTodoListSchema, 'none')).toBe(true) + }) +}) + +describe('the two writes on this surface', () => { + it('reads the connect envelope and names a reply that is not one', () => { + expect(reads(linearAccountConnectedSchema, { ok: true }).ok).toBe(true) + expect(reads(linearAccountConnectedSchema, { ok: false, error: 'bad key' }).error).toBe( + 'bad key' + ) + expect(refuses(linearAccountConnectedSchema, null)).toBe(true) + }) + + it('reads anything for the repo preference write, whose body the screen never looks at', () => { + expect(refuses(taskRepoPreferenceWrittenSchema, null)).toBe(false) + expect(refuses(taskRepoPreferenceWrittenSchema, 'written')).toBe(false) + }) +}) diff --git a/mobile/src/tasks/task-list-reply-schema.ts b/mobile/src/tasks/task-list-reply-schema.ts new file mode 100644 index 00000000000..ea66b2ed432 --- /dev/null +++ b/mobile/src/tasks/task-list-reply-schema.ts @@ -0,0 +1,88 @@ +import { z } from 'zod' +import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' +import { prFlag, prText } from '../session/github-pr-entity-reply-schema' +import { taskMutationEnvelopeSchema } from './task-provider-entity-reply-schema' + +// What the Tasks list reads to fill itself for a provider, plus the write that connects a Linear +// account. Checked against the handlers in src/main/runtime/rpc/methods/ — linear.ts:25-43 and +// :101-104, gitlab.ts:66-69, github-repo-work-item-methods.ts, repo.ts — and the shared results +// they return: LinearConnectionStatus in src/shared/linear/workspace-types.ts, GitLabTodo in +// src/shared/gitlab-types.ts:219, and `getStatus()` in src/main/linear/client.ts:164. + +/** + * Linear account status for provider hydration. + * + * The container is required and nothing in it is: use-mobile-tasks-provider-load-actions.tsx:58 + * compares `connected` to `true`, :66 reads `workspaces ?? []`, and :68 walks + * `selectedWorkspaceId ?? activeWorkspaceId ?? workspaces[0]?.id ?? null`. Main read those members + * off `null` and threw a property-read TypeError the Tasks screen showed as its load error. + * + * `selectedWorkspaceId` keeps an explicit `null`, which is a value this host sends + * (src/main/linear/client.ts:180) and which the `??` chain at the call site is what interprets. + * + * The workspace row is mobile's own `LinearWorkspace` (mobile-tasks-view-state-types.ts:63), not + * the host's: the picker reads `workspace.id` as its value and match key and + * `organizationName ?? displayName ?? id` as its label + * (use-mobile-tasks-provider-view-projection.tsx:76-86), and nothing on this screen reads the rest + * of what `getStatus` sends. A row without an `id` drops, because it can neither be selected nor + * matched. + */ +export const linearAccountStatusSchema = z.looseObject({ + connected: prFlag('connected'), + workspaces: salvagedOptional( + 'workspaces', + salvagingArray( + z.looseObject({ + id: z.string(), + organizationName: prText('organizationName'), + displayName: prText('displayName') + }) + ) + ), + selectedWorkspaceId: salvagedOptional('selectedWorkspaceId', z.string().nullable()), + activeWorkspaceId: salvagedOptional('activeWorkspaceId', z.string().nullable()) +}) + +/** + * The GitHub item total for the current filter, asked once per repo and summed. + * + * The payload is the number, so the number is the schema. Main's `typeof count === 'number' ? count + * : 0` fallback is gone from the call site because the reader now answers for it: a reply that is + * not a number reaches the per-repo `catch` that already swallows a failed count as zero, and logs + * which repo and why instead of adding a silent zero to the total. + */ +export const githubWorkItemCountSchema = z.number().finite() + +/** + * The GitLab to-do inbox. + * + * An array, or the nullish the call site already reads as an empty inbox — and nothing about a + * row, which is the one place this domain's reader stays at the container on purpose. The host + * returns `GitLabTodo[]` (src/shared/gitlab-types.ts:219) and `createGitLabTodoTask` reads + * `actionName.replace` with no guard, so every member of that type has a claim to being required. + * The corpus is what stops it: the recorded `normal` reply at this site is + * `[{ id, targetType, target }]`, a shape `listTodos` cannot produce, and narrowing the row would + * refuse this site's only success control. Tightening it needs that scenario corrected first. + * + * What the container alone already buys is the failure the call site names: a reply that is + * neither an array nor nullish was `(response.result ?? []).map is not a function` on the screen, + * and is now one error naming `gitlab.todos`. + */ +export const gitlabTodoListSchema = z.array(z.unknown()).nullish() + +/** + * Connecting a Linear account with a pasted API key, and the repository issue-source write. + * + * The connect reply is the standard envelope: use-mobile-tasks-task-pagination-actions.tsx:56 + * reads `ok === false` and raises `error` or its own copy, and nothing else in the reply. + */ +export const linearAccountConnectedSchema = taskMutationEnvelopeSchema + +/** + * The repository issue-source preference write. + * + * Deliberately unread: use-mobile-tasks-task-create-actions.tsx:199 interprets the envelope for + * its acceptance and then re-reads the repo list rather than patching its cached copy, so there is + * no member to declare and a narrower reader would only invent a failure the screen never had. + */ +export const taskRepoPreferenceWrittenSchema = z.unknown() diff --git a/mobile/src/tasks/task-provider-entity-reply-schema.test.ts b/mobile/src/tasks/task-provider-entity-reply-schema.test.ts new file mode 100644 index 00000000000..8969e29f3e9 --- /dev/null +++ b/mobile/src/tasks/task-provider-entity-reply-schema.test.ts @@ -0,0 +1,177 @@ +import { describe, expect, it } from 'vitest' +import type { z } from 'zod' +import { + assignableUserListSchema, + detailCheckListSchema, + detailCommentListSchema, + detailFileListSchema, + reviewSummaryListSchema, + taskCommentWriteEnvelopeSchema, + taskMutationEnvelopeSchema +} from './task-provider-entity-reply-schema' + +// The entity claims the four tasks schema modules are built on: which member a row is identified +// by, which arm sets are closed, and which members keep an explicit null. + +function reads(schema: z.ZodType, value: unknown): T { + const parsed = schema.safeParse(value) + if (!parsed.success) { + throw new Error(`expected a readable reply: ${parsed.error.message}`) + } + return parsed.data +} + +function refuses(schema: z.ZodType, value: unknown): boolean { + return !schema.safeParse(value).success +} + +const COMMENT = { id: 902, author: 'You', body: 'a comment', createdAt: '2020-01-01T00:00:00.000Z' } + +describe('the mutation envelope', () => { + it('keeps ok tri-state, so absent is the success main read and false is a refusal', () => { + expect(reads(taskMutationEnvelopeSchema, {}).ok).toBeUndefined() + expect(reads(taskMutationEnvelopeSchema, { ok: true }).ok).toBe(true) + expect(reads(taskMutationEnvelopeSchema, { ok: false }).ok).toBe(false) + }) + + it('reads a non-boolean ok as absent, which is the success main read for it', () => { + expect(reads(taskMutationEnvelopeSchema, { ok: 'false' }).ok).toBeUndefined() + }) + + it('requires the container, which is the property read main died on', () => { + expect(refuses(taskMutationEnvelopeSchema, null)).toBe(true) + expect(refuses(taskMutationEnvelopeSchema, 'accepted')).toBe(true) + expect(refuses(taskMutationEnvelopeSchema, 7)).toBe(true) + }) + + it('drops an error that is not text, so the call site shows its own copy', () => { + expect(reads(taskMutationEnvelopeSchema, { ok: false, error: 'boom' }).error).toBe('boom') + expect( + reads(taskMutationEnvelopeSchema, { ok: false, error: { message: 'boom' } }).error + ).toBeUndefined() + }) + + it('passes members it does not declare straight through', () => { + expect(reads(taskMutationEnvelopeSchema, { ok: true, number: 11 })).toMatchObject({ + number: 11 + }) + }) +}) + +describe('a comment row', () => { + it('requires the id it is keyed by and the body it renders', () => { + expect(reads(detailCommentListSchema, [COMMENT])).toHaveLength(1) + expect(reads(detailCommentListSchema, [{ ...COMMENT, id: 'c-1' }])[0]?.id).toBe('c-1') + expect(reads(detailCommentListSchema, [{ ...COMMENT, id: undefined }])).toEqual([]) + expect(reads(detailCommentListSchema, [{ ...COMMENT, body: undefined }])).toEqual([]) + }) + + it('drops the unreadable row rather than the whole list', () => { + expect(reads(detailCommentListSchema, [COMMENT, { id: 1 }])).toHaveLength(1) + }) + + it('refuses a list that is not one', () => { + expect(refuses(detailCommentListSchema, { comments: [] })).toBe(true) + expect(refuses(detailCommentListSchema, 'none')).toBe(true) + }) + + it('carries the reaction vocabulary this app renders and drops the one it does not', () => { + const reactions = [ + { content: 'thumbs_up', count: 1 }, + { content: 'thumbs_down', count: 1 }, + { content: 'laugh', count: 1 }, + { content: 'confused', count: 1 }, + { content: 'heart', count: 1 }, + { content: 'hooray', count: 1 }, + { content: 'rocket', count: 1 }, + { content: 'eyes', count: 1 } + ] + expect(reads(detailCommentListSchema, [{ ...COMMENT, reactions }])[0]?.reactions).toHaveLength( + 8 + ) + expect( + reads(detailCommentListSchema, [{ ...COMMENT, reactions: [{ content: '+1', count: 1 }] }])[0] + ?.reactions + ).toEqual([]) + }) + + it('leaves every guarded member exactly as the host sent it', () => { + const row = reads(detailCommentListSchema, [ + { ...COMMENT, path: 'src/a.ts', line: 12, threadId: 't-1', isResolved: false } + ])[0] + expect(row).toMatchObject({ path: 'src/a.ts', line: 12, threadId: 't-1', isResolved: false }) + }) +}) + +describe('the comment write envelope', () => { + it('drops a comment the timeline could not key, leaving the local echo in place', () => { + expect( + reads(taskCommentWriteEnvelopeSchema, { ok: true, comment: COMMENT }).comment + ).toMatchObject({ id: 902 }) + expect( + reads(taskCommentWriteEnvelopeSchema, { ok: true, comment: { id: 902 } }).comment + ).toBeUndefined() + expect(reads(taskCommentWriteEnvelopeSchema, { ok: true }).comment).toBeUndefined() + }) +}) + +describe('a user row', () => { + it('requires the login every reader trims, and keeps an explicit null beside it', () => { + const rows = reads(assignableUserListSchema, [ + { login: 'octocat', name: 'Octo', avatarUrl: null } + ]) + expect(rows[0]).toMatchObject({ login: 'octocat', name: 'Octo', avatarUrl: null }) + expect(reads(assignableUserListSchema, [{ name: 'Octo' }])).toEqual([]) + }) + + it('keeps a review row own null state rather than defaulting it', () => { + expect(reads(reviewSummaryListSchema, [{ login: 'octocat', state: null }])[0]?.state).toBeNull() + expect(reads(reviewSummaryListSchema, [{ state: 'APPROVED' }])).toEqual([]) + }) +}) + +describe('a check row and a file row', () => { + it('keeps the host casing the tasks surface actually sends', () => { + const checks = reads(detailCheckListSchema, [ + { name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS', url: '' } + ]) + expect(checks[0]).toMatchObject({ name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }) + }) + + it('drops a check with no name or no status', () => { + expect(reads(detailCheckListSchema, [{ status: 'completed' }])).toEqual([]) + expect(reads(detailCheckListSchema, [{ name: 'build' }])).toEqual([]) + }) + + it('requires the path a file row is matched by', () => { + expect(reads(detailFileListSchema, [{ path: 'src/a.ts' }])).toHaveLength(1) + expect(reads(detailFileListSchema, [{ oldPath: 'src/a.ts' }])).toEqual([]) + }) + + it('carries every file status arm and drops one it has not heard of', () => { + for (const status of [ + 'added', + 'modified', + 'removed', + 'renamed', + 'copied', + 'changed', + 'unchanged' + ]) { + expect(reads(detailFileListSchema, [{ path: 'a', status }])[0]?.status).toBe(status) + } + expect(reads(detailFileListSchema, [{ path: 'a', status: 'ADDED' }])[0]?.status).toBeUndefined() + }) + + it('carries every viewed-state arm and drops one it has not heard of', () => { + for (const viewerViewedState of ['DISMISSED', 'VIEWED', 'UNVIEWED']) { + expect( + reads(detailFileListSchema, [{ path: 'a', viewerViewedState }])[0]?.viewerViewedState + ).toBe(viewerViewedState) + } + expect( + reads(detailFileListSchema, [{ path: 'a', viewerViewedState: 'PENDING' }])[0] + ?.viewerViewedState + ).toBeUndefined() + }) +}) diff --git a/mobile/src/tasks/task-provider-entity-reply-schema.ts b/mobile/src/tasks/task-provider-entity-reply-schema.ts new file mode 100644 index 00000000000..c84c070bf5f --- /dev/null +++ b/mobile/src/tasks/task-provider-entity-reply-schema.ts @@ -0,0 +1,162 @@ +import { z } from 'zod' +import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage' +import { prCount, prFlag, prNullableText, prText } from '../session/github-pr-entity-reply-schema' + +// The entities the tasks screen's provider replies are built out of: the mutation envelope every +// GitHub/GitLab/Linear write answers with, a conversation comment, an assignable user, a review +// summary, a check row and a changed file. Checked against src/main/github/issue-create.ts, +// issue-update.ts, issue-comment.ts, client/create/add-pr-review-comment.ts and the shared +// GitHubCommentResult / GitHubCreateIssueResult types the host returns from them. +// +// Two rules run through this file and the four schema modules beside it. +// +// 1. A member is required only where a tasks consumer reads it with no guard. Everything the +// consumer reaches through `?.`, `??` or a `typeof` test stays optional, because main read it +// that way and a reply without it rendered the same fallback it renders now. +// 2. No member is required that the site's own recorded `normal` reply does not carry. The corpus +// is the only evidence of what a host really sends at each site, and requiring a member absent +// from that control would turn a good reply into an incompatible one. Where that rule holds a +// schema looser than the host's own type, the schema says so at the member. +// +// Member helpers come from the session domain's entity module rather than a second copy: they are +// plain salvaged-member combinators over zod-salvage, and one definition is what keeps "absent +// stays absent, malformed reads as absent" identical on both surfaces. + +const DETAIL_REACTION_CONTENT = [ + 'thumbs_up', + 'thumbs_down', + 'laugh', + 'confused', + 'heart', + 'hooray', + 'rocket', + 'eyes' +] as const + +const DETAIL_FILE_STATUS = [ + 'added', + 'modified', + 'removed', + 'renamed', + 'copied', + 'changed', + 'unchanged' +] as const + +const VIEWER_VIEWED_STATE = ['DISMISSED', 'VIEWED', 'UNVIEWED'] as const + +/** + * One conversation comment, as every task sheet holds it. + * + * `id` and `body` are the only required members, and they are the two `DetailComment` declares + * non-optional: the timeline keys rows by id and renders body unguarded. Every other member is + * reached through `?.` or `??` — commentAuthor (mobile-tasks-item-comments.tsx:47) is the shape of + * all of them — so it stays optional and is passed through exactly as the host sent it. The + * reaction arm set is closed because the mobile vocabulary (`thumbs_up`) is not GitHub's (`+1`): + * a row this build cannot name has no glyph to render, so it drops rather than reaching the list. + */ +export const detailCommentSchema = z.looseObject({ + id: z.union([z.string(), z.number().finite()]), + author: prText('author'), + authorAvatarUrl: prText('authorAvatarUrl'), + user: salvagedOptional('user', z.looseObject({ displayName: prText('displayName') })), + isBot: prFlag('isBot'), + body: z.string(), + createdAt: prText('createdAt'), + url: prText('url'), + reactions: salvagedOptional( + 'reactions', + salvagingArray( + z.looseObject({ content: z.enum(DETAIL_REACTION_CONTENT), count: z.number().finite() }) + ) + ), + path: prText('path'), + line: prCount('line'), + startLine: prCount('startLine'), + threadId: prText('threadId'), + isResolved: prFlag('isResolved') +}) + +export const detailCommentListSchema = salvagingArray(detailCommentSchema) + +/** + * A user the item can be assigned to or asked to review. + * + * `login` is the identity: the reviewer merge reads `reviewer.login.trim()` with no guard + * (use-mobile-tasks-hosted-comment-review-actions.tsx:180), so a row without one drops rather than + * taking the whole picker down. `name` and `avatarUrl` keep an explicit `null` — the recorded + * `github.listAssignableUsers` reply sends `avatarUrl: null`, and collapsing it to `''` would + * change what the avatar row renders for a good reply. + */ +export const assignableUserSchema = z.looseObject({ + login: z.string(), + name: prNullableText('name'), + avatarUrl: prNullableText('avatarUrl') +}) + +export const assignableUserListSchema = salvagingArray(assignableUserSchema) + +/** One review, keyed by `login` the same way. Nested `author.login` is not accepted here: this + * surface never read it, and inventing the fallback would widen what the sheet shows. */ +export const reviewSummaryListSchema = salvagingArray( + z.looseObject({ + login: z.string(), + state: prNullableText('state'), + avatarUrl: prNullableText('avatarUrl') + }) +) + +/** + * One check row. `name` labels it and `status` drives its icon, and both are read unguarded by the + * checks list and by summarizeProviderChecks. + * + * `status` is a free string rather than the host's `queued | in_progress | completed` arm set, + * because this surface is fed by two different producers: the recorded `github.prChecks` reply + * sends `COMPLETED` / `SUCCESS` in caps, and `GitHubDetailCheck` declares both as strings. + */ +export const detailCheckListSchema = salvagingArray( + z.looseObject({ + name: z.string(), + status: z.string(), + conclusion: prNullableText('conclusion'), + url: prNullableText('url') + }) +) + +/** One changed file. `path` is what the expansion, the viewed toggle and the comment anchor are + * all keyed on, so a row without one can never match and drops. */ +export const detailFileListSchema = salvagingArray( + z.looseObject({ + path: z.string(), + oldPath: prText('oldPath'), + status: salvagedOptional('status', z.enum(DETAIL_FILE_STATUS)), + additions: prCount('additions'), + deletions: prCount('deletions'), + isBinary: prFlag('isBinary'), + viewerViewedState: salvagedOptional('viewerViewedState', z.enum(VIEWER_VIEWED_STATE)) + }) +) + +/** + * The envelope every task mutation answers with, and the reason there is one of it rather than + * twenty. + * + * `ok === false` is the only failure test any of these call sites makes, and `error` is the only + * text any of them raises. Both stay optional and tri-state: a reply with no `ok` is the success + * main read, and `ok: false` is a refusal the caller reports with the host's own sentence. + * `error` is a string because that is what every call site interpolates into `new Error(...)`; a + * host that answers an object there now reaches the call site's own fallback copy instead of + * rendering `[object Object]`. + */ +export const taskMutationEnvelopeSchema = z.looseObject({ + ok: prFlag('ok'), + error: prText('error') +}) + +/** The mutation envelope a comment write adds its created row to. The row is salvaged, so a + * comment the host could not describe leaves the call site's local echo in place. */ +export const taskCommentWriteEnvelopeSchema = z.looseObject({ + ok: prFlag('ok'), + error: prText('error'), + comment: salvagedOptional('comment', detailCommentSchema) +}) diff --git a/mobile/src/tasks/use-mobile-tasks-github-check-file-actions.tsx b/mobile/src/tasks/use-mobile-tasks-github-check-file-actions.tsx index 29f6b68cee8..1f8851ce048 100644 --- a/mobile/src/tasks/use-mobile-tasks-github-check-file-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-github-check-file-actions.tsx @@ -13,7 +13,6 @@ import type { DetailComment, DetailPayload, GitHubDetailFile, - GitHubPRFileContents, TaskItem } from './mobile-tasks-legacy-foundation' @@ -52,11 +51,7 @@ export function useMobileTasksGithubCheckFileActions(model: HostedCommentReviewA }, { timeoutMs: 60_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubPullRequestChecksRerun.interpret(reply) as { - ok?: boolean - error?: string - } + const result = githubPullRequestChecksRerun.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to rerun checks') } @@ -204,8 +199,7 @@ export function useMobileTasksGithubCheckFileActions(model: HostedCommentReviewA }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const contents = githubPullRequestFileContentsRead.interpret(reply) as GitHubPRFileContents + const contents = githubPullRequestFileContentsRead.interpret(reply) setPrFileContents((current) => ({ ...current, [file.path]: contents })) } catch (err) { setError(err instanceof Error ? err.message : 'Failed to load file contents') @@ -249,12 +243,7 @@ export function useMobileTasksGithubCheckFileActions(model: HostedCommentReviewA }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubReviewCommentWrite.interpret(reply) as { - ok?: boolean - error?: string - comment?: DetailComment - } + const result = githubReviewCommentWrite.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to add review comment') } diff --git a/mobile/src/tasks/use-mobile-tasks-github-reply-merge-actions.tsx b/mobile/src/tasks/use-mobile-tasks-github-reply-merge-actions.tsx index 4b582351f57..e0178bef784 100644 --- a/mobile/src/tasks/use-mobile-tasks-github-reply-merge-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-github-reply-merge-actions.tsx @@ -88,12 +88,7 @@ export function useMobileTasksGithubReplyMergeActions(model: GithubCheckFileActi { timeoutMs: 30_000 } ) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const envelope = replyResult as { - ok?: boolean - error?: string - comment?: DetailComment - } + const envelope = replyResult if (envelope.ok === false) { throw new Error(envelope.error ?? 'Failed to reply') } @@ -171,8 +166,7 @@ export function useMobileTasksGithubReplyMergeActions(model: GithubCheckFileActi { timeoutMs: 60_000 } ) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = merged as { ok?: boolean; error?: string } + const result = merged if (result.ok === false) { throw new Error(result.error ?? 'Failed to merge') } diff --git a/mobile/src/tasks/use-mobile-tasks-gitlab-github-status-actions.tsx b/mobile/src/tasks/use-mobile-tasks-gitlab-github-status-actions.tsx index 92c74bc58ae..9b4864d294d 100644 --- a/mobile/src/tasks/use-mobile-tasks-gitlab-github-status-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-gitlab-github-status-actions.tsx @@ -52,8 +52,7 @@ export function useMobileTasksGitlabGithubStatusActions(model: ProjectFileMergeA projectRef: item.source.projectRef }) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = updated as { ok?: boolean; error?: string } + const result = updated if (result.ok === false) { throw new Error(result.error ?? 'Failed to update GitLab item') } @@ -95,8 +94,7 @@ export function useMobileTasksGitlabGithubStatusActions(model: ProjectFileMergeA }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubIssueUpdate.interpret(reply) as { ok?: boolean; error?: string } + const result = githubIssueUpdate.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to update GitHub issue') } diff --git a/mobile/src/tasks/use-mobile-tasks-hosted-comment-review-actions.tsx b/mobile/src/tasks/use-mobile-tasks-hosted-comment-review-actions.tsx index 67705a97ae3..0ad6fda90ea 100644 --- a/mobile/src/tasks/use-mobile-tasks-hosted-comment-review-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-hosted-comment-review-actions.tsx @@ -8,7 +8,6 @@ import { import { type DetailComment, type GitHubAssignableUser, - type GitHubDetailCheck, type TaskItem, splitReviewerList } from './mobile-tasks-legacy-foundation' @@ -94,12 +93,7 @@ export function useMobileTasksHostedCommentReviewActions(model: HostedMetadataAc { timeoutMs: 30_000 } ) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = written as { - ok?: boolean - error?: string - comment?: DetailComment - } + const result = written if (result.ok === false) { throw new Error(result.error ?? 'Failed to add comment') } @@ -167,8 +161,7 @@ export function useMobileTasksHostedCommentReviewActions(model: HostedMetadataAc }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubReviewerRequest.interpret(reply) as { ok?: boolean; error?: string } + const result = githubReviewerRequest.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to request reviewers') } @@ -247,12 +240,9 @@ export function useMobileTasksHostedCommentReviewActions(model: HostedMetadataAc }, { timeoutMs: 30_000 } ) - const payload = githubPullRequestChecksRead.interpret(reply) - if (!Array.isArray(payload)) { - throw new Error('Invalid checks response') - } - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const checks = payload as GitHubDetailCheck[] + // The reader answers an array of readable rows, so the hand-rolled shape test this call + // site kept is gone: a reply that is not one now names the method it came from. + const checks = githubPullRequestChecksRead.interpret(reply) const checksSummary = buildGitHubCheckSummary(checks) setDetailPayload((current) => current?.provider === 'github' ? { ...current, checks } : current diff --git a/mobile/src/tasks/use-mobile-tasks-hosted-metadata-actions.tsx b/mobile/src/tasks/use-mobile-tasks-hosted-metadata-actions.tsx index 9c77e313306..e2473c8fd11 100644 --- a/mobile/src/tasks/use-mobile-tasks-hosted-metadata-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-hosted-metadata-actions.tsx @@ -50,11 +50,7 @@ export function useMobileTasksHostedMetadataActions(model: GitlabGithubStatusAct }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubPullRequestUpdate.interpret(reply) as { - ok?: boolean - error?: string - } + const result = githubPullRequestUpdate.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to update GitHub pull request') } @@ -146,8 +142,7 @@ export function useMobileTasksHostedMetadataActions(model: GitlabGithubStatusAct { timeoutMs: 30_000 } ) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = updated as { ok?: boolean; error?: string } + const result = updated if (result.ok === false) { throw new Error(result.error ?? 'Failed to update GitLab item') } diff --git a/mobile/src/tasks/use-mobile-tasks-item-detail-loading.tsx b/mobile/src/tasks/use-mobile-tasks-item-detail-loading.tsx index eae64ede6c4..4ffafce06e7 100644 --- a/mobile/src/tasks/use-mobile-tasks-item-detail-loading.tsx +++ b/mobile/src/tasks/use-mobile-tasks-item-detail-loading.tsx @@ -4,16 +4,7 @@ import { buildGitLabCheckSummary, useEffect } from './mobile-tasks-dependencies' -import { - type DetailComment, - type GitHubAssignableUser, - type GitHubDetailCheck, - type GitHubDetailFile, - type GitHubPRReviewSummary, - type LinearIssue, - type TaskItem, - createLinearTask -} from './mobile-tasks-legacy-foundation' +import { type TaskItem, createLinearTask } from './mobile-tasks-legacy-foundation' import { githubItemDetailRead, gitlabItemDetailRead, @@ -57,31 +48,7 @@ export function useMobileTasksItemDetailLoading(model: ItemDetailMetadataEffects }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const details = githubItemDetailRead.interpret(reply) as { - body?: string - comments?: DetailComment[] - item?: { - labels?: string[] - reviewDecision?: string | null - reviewRequests?: GitHubAssignableUser[] - latestReviews?: GitHubPRReviewSummary[] - } - assignees?: string[] - headSha?: string - baseSha?: string - pullRequestId?: string - checks?: GitHubDetailCheck[] - files?: Array<{ - path: string - oldPath?: string - status?: GitHubDetailFile['status'] - additions?: number - deletions?: number - isBinary?: boolean - viewerViewedState?: 'DISMISSED' | 'VIEWED' | 'UNVIEWED' - }> - } | null + const details = githubItemDetailRead.interpret(reply) if (!details) { throw new Error('Details not found') } @@ -116,23 +83,7 @@ export function useMobileTasksItemDetailLoading(model: ItemDetailMetadataEffects }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const details = gitlabItemDetailRead.interpret(reply) as { - body?: string - comments?: DetailComment[] - item?: { labels?: string[]; mergeable?: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN' } - assignees?: string[] - pipelineJobs?: Array<{ - id?: number - name: string - stage: string - status: string - webUrl?: string | null - duration?: number | null - }> - reviewers?: unknown[] - approvalState?: { approvalsRequired: number | null; approvalsLeft: number | null } - } | null + const details = gitlabItemDetailRead.interpret(reply) if (!details) { throw new Error('Details not found') } @@ -208,11 +159,9 @@ export function useMobileTasksItemDetailLoading(model: ItemDetailMetadataEffects { timeoutMs: 30_000 } ) ]) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const issue = linearIssueRead.interpret(issueReply) as LinearIssue | null + const issue = linearIssueRead.interpret(issueReply) const accepted = linearIssueCommentsRead.interpret(commentsReply) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const comments = accepted.accepted ? ((accepted.value as DetailComment[]) ?? []) : [] + const comments = accepted.accepted ? (accepted.value ?? []) : [] if (!issue) { throw new Error('Details not found') } diff --git a/mobile/src/tasks/use-mobile-tasks-item-detail-metadata-effects.tsx b/mobile/src/tasks/use-mobile-tasks-item-detail-metadata-effects.tsx index 4a4af3bf4b2..bf398a2b22f 100644 --- a/mobile/src/tasks/use-mobile-tasks-item-detail-metadata-effects.tsx +++ b/mobile/src/tasks/use-mobile-tasks-item-detail-metadata-effects.tsx @@ -1,6 +1,5 @@ import type { ListAndDetailEffectsModel } from './use-mobile-tasks-list-and-detail-effects' import { useEffect } from './mobile-tasks-dependencies' -import type { GitHubAssignableUser } from './mobile-tasks-legacy-foundation' import { githubAssignableUserListRead, githubRepoLabelListRead @@ -52,8 +51,7 @@ export function useMobileTasksItemDetailMetadataEffects(model: ListAndDetailEffe if (stale) { return } - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - setItemAvailableLabels(githubRepoLabelListRead.interpret(response) as string[]) + setItemAvailableLabels(githubRepoLabelListRead.interpret(response)) }) .catch((err) => { if (!stale) { @@ -80,10 +78,7 @@ export function useMobileTasksItemDetailMetadataEffects(model: ListAndDetailEffe if (stale) { return } - setItemAssignableUsers( - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - githubAssignableUserListRead.interpret(response) as GitHubAssignableUser[] - ) + setItemAssignableUsers(githubAssignableUserListRead.interpret(response)) }) .catch((err) => { if (!stale) { diff --git a/mobile/src/tasks/use-mobile-tasks-linear-item-actions.tsx b/mobile/src/tasks/use-mobile-tasks-linear-item-actions.tsx index ddf93db8ad9..f76eb5c46fa 100644 --- a/mobile/src/tasks/use-mobile-tasks-linear-item-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-linear-item-actions.tsx @@ -2,7 +2,6 @@ import type { GithubReplyMergeActionsModel } from './use-mobile-tasks-github-rep import { useCallback } from './mobile-tasks-dependencies' import { type DetailComment, - type LinearIssue, type LinearIssueChild, type TaskItem, createLinearTask @@ -45,12 +44,7 @@ export function useMobileTasksLinearItemActions(model: GithubReplyMergeActionsMo }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = linearIssueCommentWrite.interpret(reply) as { - ok?: boolean - id?: string - error?: string - } + const result = linearIssueCommentWrite.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to add comment') } @@ -88,8 +82,7 @@ export function useMobileTasksLinearItemActions(model: GithubReplyMergeActionsMo { id: child.id, workspaceId }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const issue = linearIssueRead.interpret(reply) as LinearIssue | null + const issue = linearIssueRead.interpret(reply) if (!issue) { throw new Error('Sub-issue not found') } @@ -126,15 +119,7 @@ export function useMobileTasksLinearItemActions(model: GithubReplyMergeActionsMo }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = linearIssueCreate.interpret(reply) as { - ok?: boolean - id?: string - identifier?: string - title?: string - url?: string - error?: string - } + const result = linearIssueCreate.interpret(reply) if (result.ok === false || !result.id || !result.identifier) { throw new Error(result.error ?? 'Failed to create sub-issue') } diff --git a/mobile/src/tasks/use-mobile-tasks-list-and-detail-effects.tsx b/mobile/src/tasks/use-mobile-tasks-list-and-detail-effects.tsx index 300f2263b35..657685cfb74 100644 --- a/mobile/src/tasks/use-mobile-tasks-list-and-detail-effects.tsx +++ b/mobile/src/tasks/use-mobile-tasks-list-and-detail-effects.tsx @@ -6,12 +6,7 @@ import { useCallback, useEffect } from './mobile-tasks-dependencies' -import { - type LinearState, - type LinearTeam, - getTaskPresetQuery, - scopeGitHubTaskSearch -} from './mobile-tasks-legacy-foundation' +import { getTaskPresetQuery, scopeGitHubTaskSearch } from './mobile-tasks-legacy-foundation' import { linearComposerTeamListRead, linearTeamStateListRead @@ -202,8 +197,7 @@ export function useMobileTasksListAndDetailEffects(model: ProjectLoadingActionsM } const accepted = linearComposerTeamListRead.interpret(response) if (accepted.accepted) { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const teams = accepted.value as LinearTeam[] + const teams = accepted.value setLinearTeams(teams) setCreateTeamId((current) => current ?? teams[0]?.id ?? null) } else { @@ -244,8 +238,7 @@ export function useMobileTasksListAndDetailEffects(model: ProjectLoadingActionsM return } const accepted = linearTeamStateListRead.interpret(statesResponse) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - setLinearStates(accepted.accepted ? (accepted.value as LinearState[]) : []) + setLinearStates(accepted.accepted ? accepted.value : []) }) .catch(() => { if (!stale) { diff --git a/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx b/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx index f5693ecd723..c96ee719515 100644 --- a/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-project-file-merge-actions.tsx @@ -3,7 +3,6 @@ import { useCallback } from './mobile-tasks-dependencies' import { type DetailComment, type GitHubDetailFile, - type GitHubPRFileContents, type GitHubProjectRow, type HostedReviewMergeMethod, type TaskItem, @@ -82,8 +81,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const contents = githubPullRequestFileContentsRead.interpret(reply) as GitHubPRFileContents + const contents = githubPullRequestFileContentsRead.interpret(reply) setPrFileContents((current) => ({ ...current, [file.path]: contents })) } catch (err) { setProjectRowDetailError( @@ -140,12 +138,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubReviewCommentWrite.interpret(reply) as { - ok?: boolean - error?: string - comment?: DetailComment - } + const result = githubReviewCommentWrite.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to add review comment') } @@ -213,8 +206,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA }, { timeoutMs: 60_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubPullRequestMerge.interpret(reply) as { ok?: boolean; error?: string } + const result = githubPullRequestMerge.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to merge pull request') } @@ -273,8 +265,7 @@ export function useMobileTasksProjectFileMergeActions(model: ProjectReviewCheckA updates: { state: nextState } }) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = updated as { ok?: boolean; error?: string } + const result = updated if (result.ok === false) { throw new Error(result.error ?? 'Failed to update GitHub status') } diff --git a/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx b/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx index 1394a62b449..f899ae15678 100644 --- a/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-project-review-check-actions.tsx @@ -2,7 +2,6 @@ import type { ProjectMetadataActionsModel } from './use-mobile-tasks-project-met import { useCallback } from './mobile-tasks-dependencies' import { type GitHubAssignableUser, - type GitHubDetailCheck, type GitHubDetailFile, type GitHubProjectRow, projectRowGitHubRepository, @@ -52,8 +51,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc }, { timeoutMs: 30_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubReviewerRequest.interpret(reply) as { ok?: boolean; error?: string } + const result = githubReviewerRequest.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to request reviewers') } @@ -129,12 +127,9 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc }, { timeoutMs: 30_000 } ) - const payload = githubPullRequestChecksRead.interpret(reply) - if (!Array.isArray(payload)) { - throw new Error('Invalid checks response') - } - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const checks = payload as GitHubDetailCheck[] + // The reader answers an array of readable rows, so the hand-rolled shape test this call + // site kept is gone: a reply that is not one now names the method it came from. + const checks = githubPullRequestChecksRead.interpret(reply) setProjectRowDetail((current) => current?.provider === 'github' ? { ...current, checks } : current ) @@ -173,11 +168,7 @@ export function useMobileTasksProjectReviewCheckActions(model: ProjectMetadataAc }, { timeoutMs: 60_000 } ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = githubPullRequestChecksRerun.interpret(reply) as { - ok?: boolean - error?: string - } + const result = githubPullRequestChecksRerun.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to rerun checks') } diff --git a/mobile/src/tasks/use-mobile-tasks-project-thread-reply-actions.tsx b/mobile/src/tasks/use-mobile-tasks-project-thread-reply-actions.tsx index 584cea4d128..14180083b44 100644 --- a/mobile/src/tasks/use-mobile-tasks-project-thread-reply-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-project-thread-reply-actions.tsx @@ -196,12 +196,7 @@ export function useMobileTasksProjectThreadReplyActions( { timeoutMs: 30_000 } ) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = written as { - ok?: boolean - error?: string - comment?: DetailComment - } + const result = written if (result.ok === false) { throw new Error(result.error ?? 'Failed to reply') } diff --git a/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx b/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx index 30e905f9b73..43051210c31 100644 --- a/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-provider-load-actions.tsx @@ -15,7 +15,6 @@ import { GITHUB_REPO_CONCURRENCY, type GitHubRepoSources, type GitHubWorkItem, - type LinearStatusResponse, type LinearTeam, type RepoSummary, type TaskItem, @@ -53,8 +52,7 @@ export function useMobileTasksProviderLoadActions(model: RuntimeHydrationModel) return } const statusReply = await linearAccountStatusRead.request(client) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const status = linearAccountStatusRead.interpret(statusReply) as LinearStatusResponse + const status = linearAccountStatusRead.interpret(statusReply) setLinearConnected(status.connected === true) if (status.connected !== true) { setLinearWorkspaces([]) @@ -72,8 +70,7 @@ export function useMobileTasksProviderLoadActions(model: RuntimeHydrationModel) const teamsReply = await linearWorkspaceTeamListRead.request(client, { workspaceId: workspaceId ?? undefined }) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const teams = linearWorkspaceTeamListRead.interpret(teamsReply) as LinearTeam[] + const teams = linearWorkspaceTeamListRead.interpret(teamsReply) setLinearTeams(teams) setSelectedLinearTeamIds(reconcileTeamSelection(teams, defaultLinearTeamSelectionRef.current)) }, [client, connState, tasksSupported]) @@ -202,8 +199,10 @@ export function useMobileTasksProviderLoadActions(model: RuntimeHydrationModel) }, { timeoutMs: 30_000 } ) - const count = githubWorkItemCountRead.interpret(reply) - return typeof count === 'number' ? count : 0 + // The reader answers the number, so the `typeof` fallback this call site kept is gone: + // a reply that is not one reaches the catch below, which already counts a failed repo + // as zero and now says which repo and why. + return githubWorkItemCountRead.interpret(reply) } catch (err) { const isExpectedSshSkip = isGitHubWorkItemsSshRemoteRequiredError(err) const logWorkItemCountFailure = isExpectedSshSkip ? console.log : console.warn diff --git a/mobile/src/tasks/use-mobile-tasks-task-create-actions.tsx b/mobile/src/tasks/use-mobile-tasks-task-create-actions.tsx index f6970177e5c..1d80ab31d31 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-create-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-create-actions.tsx @@ -73,13 +73,7 @@ export function useMobileTasksTaskCreateActions(model: LinearItemActionsModel) { body: createBody }) ) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = created as { - ok?: boolean - number?: number - url?: string - error?: string - } + const result = created if (result.ok === false) { throw new Error( result.error ?? `Failed to create ${provider === 'github' ? 'GitHub' : 'GitLab'} issue` @@ -128,15 +122,7 @@ export function useMobileTasksTaskCreateActions(model: LinearItemActionsModel) { description: createBody.trim() || undefined, workspaceId: team.workspaceId }) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = linearIssueCreate.interpret(reply) as { - ok?: boolean - id?: string - identifier?: string - title?: string - url?: string - error?: string - } + const result = linearIssueCreate.interpret(reply) if (result.ok === false || !result.id || !result.identifier) { throw new Error(result.error ?? 'Failed to create Linear issue') } diff --git a/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx b/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx index 12fca82b038..dd8d299f12d 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-list-loading.tsx @@ -145,16 +145,18 @@ export function useMobileTasksTaskListLoading(model: ProviderLoadActionsModel) { const reply = await gitlabTodoListRead.request(requestClient, { repo: `id:${queriedRepos[0]!.id}` }) - // Kept spelled `response.result`: a reply that is neither an array nor nullish - // crashes in `.map` below, and the message the screen shows is this expression's - // source text, which `matrix-tasks.task-list-gitlab-todos-gitlab.todos-1` pins. - const response = { result: gitlabTodoListRead.interpret(reply) } + // The reader answers an array or the nullish this line already read as an empty inbox, + // so the `.map is not a function` the screen used to show for anything else is now one + // error naming `gitlab.todos`. The row stays uninspected: `listTodos` returns + // `GitLabTodo[]`, but the recorded reply at this site is not one, so narrowing the row + // would refuse this family's only success control. + const todos = gitlabTodoListRead.interpret(reply) if (!isCurrent()) { return } setItems( - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - ((response.result as GitLabTodo[]) ?? []) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the reader proved the container; the row is the host's declared GitLabTodo. + ((todos ?? []) as GitLabTodo[]) .map(createGitLabTodoTask) .sort((a, b) => taskTime(b.updatedAt) - taskTime(a.updatedAt)) ) diff --git a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx index b7cc45ef8a0..51ecde687a2 100644 --- a/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-task-pagination-actions.tsx @@ -51,8 +51,7 @@ export function useMobileTasksTaskPaginationActions(model: TaskListLoadingModel) setLinearConnectError('') try { const reply = await linearAccountConnect.request(client, { apiKey }) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. - const result = linearAccountConnect.interpret(reply) as { ok?: boolean; error?: string } + const result = linearAccountConnect.interpret(reply) if (result.ok === false) { throw new Error(result.error ?? 'Failed to connect Linear') } diff --git a/mobile/src/transport/unchecked-rpc-reader-inventory.ts b/mobile/src/transport/unchecked-rpc-reader-inventory.ts index be4dd3977ee..66be0057a86 100644 --- a/mobile/src/transport/unchecked-rpc-reader-inventory.ts +++ b/mobile/src/transport/unchecked-rpc-reader-inventory.ts @@ -71,10 +71,6 @@ export const UNCHECKED_RPC_READERS: readonly UncheckedRpcReaderEntry[] = [ { file: 'src/notifications/mobile-push-registration-operations.ts', readers: 2 }, { file: 'src/notifications/push-dismissal-operations.ts', readers: 1 }, // tasks - { file: 'src/tasks/mobile-task-item-comment-operations.ts', readers: 7 }, - { file: 'src/tasks/mobile-task-item-detail-operations.ts', readers: 8 }, - { file: 'src/tasks/mobile-task-item-state-operations.ts', readers: 17 }, - { file: 'src/tasks/mobile-task-list-operations.ts', readers: 6 }, { file: 'src/tasks/mobile-task-project-board-operations.ts', readers: 17 }, { file: 'src/tasks/mobile-task-runtime-operations.ts', readers: 7 }, { file: 'src/tasks/mobile-task-source-search-operations.ts', readers: 7 },