From 9133c02c5bfc9cdb2265e4def52155bb331e7553 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 23:04:41 -0400 Subject: [PATCH] refactor(mobile): checked reply readers for the tasks item and list domain (step 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thirty-eight unchecked reply readers across four tasks files become checked zod readers, so a malformed host reply surfaces as one `RpcIncompatibleReplyError` naming the method instead of a downstream `TypeError`, a rendered `undefined`, or a sheet left ready over garbage. Deliberately a behaviour change on malformed replies only; nothing on the wire moves. mobile-task-item-state-operations.ts 17 mobile-task-item-detail-operations.ts 8 mobile-task-item-comment-operations.ts 7 mobile-task-list-operations.ts 6 Two rules decide every schema, and both are stated in task-provider-entity-reply-schema.ts: 1. A member is required only where a tasks consumer reads it with no guard. Everything reached through `?.`, `??` or a `typeof` test stays optional, because a reply without it rendered the same fallback then and now. 2. No member is required that the site's own recorded `normal` reply lacks. 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. Rule 2 holds two schemas at the container: `github.prFileContents`, whose recorded reply is `{ oldContent, newContent, truncated }` where `getPRFileContents` returns `{ original, modified, ... }`, and `gitlab.todos`, whose recorded row is not a `GitLabTodo` and whose `normal` partition therefore records main crashing in `actionName.replace`. Both still gain their container, which is what names a reply that is not an object or not a list. Correcting those two scenarios is the follow-up that unlocks narrowing the rows. Nine writes share one envelope reader and five comment writes share another: `ok === false` and `error` are one host convention across them, and no input would make two of them want different answers. The acceptance, the name and the recorded family stay per operation. Three readers are reused rather than re-declared — the session domain's boolean confirmation for `setPRFileViewed` and `resolveReviewThread`, and its salvaged-member combinators throughout. Three call-site shape tests the reader now answers for are deleted: both `Array.isArray(payload)` guards on the checks read and the `typeof count === 'number'` fallback on the item count. `GitHubPRFileContents` is widened to optional members, which is what the reader can promise, and `buildGitHubPrFileDiffPreview` takes the widened sides — `splitContentLines` already treated a falsy side as no content, so no runtime behaviour moves. The tasks source-parity hashes are refreshed: hook, statement, declaration and render-token counts are unchanged, the render-token hash does not move at all, and `semantics` is a pure deletion of ten lines. Inventory: 137 unchecked readers over 30 files becomes 99 over 26. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- mobile/src/tasks/github-pr-file-diff.ts | 6 +- .../mobile-task-item-comment-operations.ts | 26 ++- .../mobile-task-item-detail-operations.ts | 36 ++- .../mobile-task-item-state-operations.ts | 61 +++-- .../src/tasks/mobile-task-list-operations.ts | 22 +- .../mobile-tasks-provider-detail-types.ts | 13 +- .../mobile-tasks-refactor-parity.test.ts | 17 +- .../task-item-comment-reply-schema.test.ts | 66 ++++++ .../tasks/task-item-comment-reply-schema.ts | 43 ++++ .../task-item-detail-reply-schema.test.ts | 206 +++++++++++++++++ .../tasks/task-item-detail-reply-schema.ts | 217 ++++++++++++++++++ .../task-item-state-reply-schema.test.ts | 102 ++++++++ .../src/tasks/task-item-state-reply-schema.ts | 108 +++++++++ .../src/tasks/task-list-reply-schema.test.ts | 107 +++++++++ mobile/src/tasks/task-list-reply-schema.ts | 88 +++++++ .../task-provider-entity-reply-schema.test.ts | 177 ++++++++++++++ .../task-provider-entity-reply-schema.ts | 162 +++++++++++++ ...mobile-tasks-github-check-file-actions.tsx | 17 +- ...obile-tasks-github-reply-merge-actions.tsx | 10 +- ...ile-tasks-gitlab-github-status-actions.tsx | 6 +- ...le-tasks-hosted-comment-review-actions.tsx | 20 +- ...e-mobile-tasks-hosted-metadata-actions.tsx | 9 +- .../use-mobile-tasks-item-detail-loading.tsx | 61 +---- ...ile-tasks-item-detail-metadata-effects.tsx | 9 +- .../use-mobile-tasks-linear-item-actions.tsx | 21 +- ...e-mobile-tasks-list-and-detail-effects.tsx | 13 +- ...obile-tasks-project-file-merge-actions.tsx | 17 +- ...ile-tasks-project-review-check-actions.tsx | 19 +- ...ile-tasks-project-thread-reply-actions.tsx | 7 +- ...use-mobile-tasks-provider-load-actions.tsx | 13 +- .../use-mobile-tasks-task-create-actions.tsx | 18 +- .../use-mobile-tasks-task-list-loading.tsx | 14 +- ...e-mobile-tasks-task-pagination-actions.tsx | 3 +- .../unchecked-rpc-reader-inventory.ts | 4 - 34 files changed, 1455 insertions(+), 263 deletions(-) create mode 100644 mobile/src/tasks/task-item-comment-reply-schema.test.ts create mode 100644 mobile/src/tasks/task-item-comment-reply-schema.ts create mode 100644 mobile/src/tasks/task-item-detail-reply-schema.test.ts create mode 100644 mobile/src/tasks/task-item-detail-reply-schema.ts create mode 100644 mobile/src/tasks/task-item-state-reply-schema.test.ts create mode 100644 mobile/src/tasks/task-item-state-reply-schema.ts create mode 100644 mobile/src/tasks/task-list-reply-schema.test.ts create mode 100644 mobile/src/tasks/task-list-reply-schema.ts create mode 100644 mobile/src/tasks/task-provider-entity-reply-schema.test.ts create mode 100644 mobile/src/tasks/task-provider-entity-reply-schema.ts 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 },