diff --git a/docs/subsystems/forge.md b/docs/subsystems/forge.md index b5156c101f..208c6a5d9f 100644 --- a/docs/subsystems/forge.md +++ b/docs/subsystems/forge.md @@ -71,9 +71,10 @@ interface ForgeProvider { /** * Read the changes of a change request at head commit `headSha` as the host computes them, as one unified * patch with `a/` and `b/` path prefixes. A patch is returned only when it describes `headSha`; once the - * head is another commit, the read resolves to a `stale` failure. + * head is another commit, the read resolves to a `stale` failure. `base`, when given, is the `baseSha` the + * latest status recorded with `headSha`; a provider may start reading from it before it confirms the head. */ - diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise + diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal, base?: string): Promise /** Read the text of `path` at `commit` in the change request's repository. */ fileText(ref: ChangeRequestRef, commit: string, path: string, signal: AbortSignal): Promise /** Build the web page of `path` at `commit` in the change request's repository. */ @@ -199,6 +200,11 @@ interface ChangeRequestSnapshot { readonly headSha: string /** Branch the change request merges into. */ readonly baseBranch: string + /** + * Base commit the host recorded with `headSha`, whose merge base with the head starts the change request's diff; + * omitted when the provider does not report it. + */ + readonly baseSha?: string /** Lines the change request adds. */ readonly additions: number /** Lines the change request deletes. */ @@ -907,7 +913,8 @@ async fetchChecks(ref: ChangeRequestRef, signal: AbortSignal): Promise + diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal, base?: string): Promise /** Read the text of `path` at `commit` in the change request's repository. */ fileText(ref: ChangeRequestRef, commit: string, path: string, signal: AbortSignal): Promise /** Build the web page of `path` at `commit` in the change request's repository. */ @@ -199,6 +200,11 @@ interface ChangeRequestSnapshot { readonly headSha: string /** Branch the change request merges into. */ readonly baseBranch: string + /** + * Base commit the host recorded with `headSha`, whose merge base with the head starts the change request's diff; + * omitted when the provider does not report it. + */ + readonly baseSha?: string /** Lines the change request adds. */ readonly additions: number /** Lines the change request deletes. */ @@ -907,7 +913,8 @@ async fetchChecks(ref: ChangeRequestRef, signal: AbortSignal): Promise --repo //`. Merging calls `PUT repos///pulls//merge` through `gh api` with the method and the expected head SHA, so GitHub refuses once new commits arrived, which is reported as `stale`. Unlike `gh pr merge`, it never enables auto-merge, never joins a merge queue, and never deletes the head branch. A refused action reports GitHub's message, and a diagnostic that mentions authentication becomes `unauthenticated`. -`diff` reads `headRefOid` and `baseRefOid` in one GraphQL query and returns `stale` when the head is not the requested commit, which must be a full 40- or 64-digit hexadecimal commit. Otherwise it reads `repos///compare/...` through `gh api --allow-escape-sequences` with the diff media type, the patch from the merge base of both commits, so neither end follows a moving branch. For every open and merged pull request measured, this patch is byte-identical to `gh pr diff`, which takes no commit argument. Unlike the pull request diff, the compare endpoint returns diffs over 300 files, so only output over `maxOutputBytes` or GitHub's own refusal makes such a diff `too-large`. `--allow-escape-sequences` keeps terminal escape sequences that file contents hold, which `gh` 2.97 and newer otherwise refuse to print; an older `gh` rejects the option with `unknown flag` before sending a request and prints such sequences unchanged, so that rejection repeats the read without it. `fileText` reads `repos///contents/?ref=` through `gh api` with the raw media type and accepts only a full 40- or 64-digit hexadecimal commit. `fileUrl` returns `https://///blob//`. Both paths percent-encode each segment. `commits` reads a pull request's commits through GraphQL, 100 per page with the owner, name, number, and cursor as variables, until `maxCommits`; every page must report the head and commit count of the first one and a cursor no earlier page returned, else the listing is `unavailable`. `compare` reads the patch from the merge base of two full commits to the second through the same compare request as `diff`. A refused read becomes `not-found` for HTTP 404, `unauthenticated` for a diagnostic that mentions authentication, `too-large` for GitHub's HTTP 406 refusal or `too_large` error, which a diff or file over its size limit draws, and `unavailable` otherwise, with GitHub's message. A read whose output exceeds `maxOutputBytes` becomes `too-large`, and one that outlives `timeoutMs` becomes `unavailable`. +`diff` reads `headRefOid` and `baseRefOid` in one GraphQL query and returns `stale` when the head is not the requested commit, which must be a full 40- or 64-digit hexadecimal commit. Otherwise it reads `repos///compare/...` through `gh api --allow-escape-sequences` with the diff media type, the patch from the merge base of both commits, so neither end follows a moving branch. Each status snapshot carries `baseRefOid` as `baseSha`; given it, `diff` starts this comparison while the GraphQL query confirms the head, and a stale head or another base commit cancels the early comparison and waits for it to end before `diff` returns. For every open and merged pull request measured, this patch is byte-identical to `gh pr diff`, which takes no commit argument. Unlike the pull request diff, the compare endpoint returns diffs over 300 files, so only output over `maxOutputBytes` or GitHub's own refusal makes such a diff `too-large`. `--allow-escape-sequences` keeps terminal escape sequences that file contents hold, which `gh` 2.97 and newer otherwise refuse to print; an older `gh` rejects the option with `unknown flag` before sending a request and prints such sequences unchanged, so that rejection repeats the read without it. `fileText` reads `repos///contents/?ref=` through `gh api` with the raw media type and accepts only a full 40- or 64-digit hexadecimal commit. `fileUrl` returns `https://///blob//`. Both paths percent-encode each segment. `commits` reads a pull request's commits through GraphQL, 100 per page with the owner, name, number, and cursor as variables, until `maxCommits`; every page must report the head and commit count of the first one and a cursor no earlier page returned, else the listing is `unavailable`. `compare` reads the patch from the merge base of two full commits to the second through the same compare request as `diff`. A refused read becomes `not-found` for HTTP 404, `unauthenticated` for a diagnostic that mentions authentication, `too-large` for GitHub's HTTP 406 refusal or `too_large` error, which a diff or file over its size limit draws, and `unavailable` otherwise, with GitHub's message. A read whose output exceeds `maxOutputBytes` becomes `too-large`, and one that outlives `timeoutMs` becomes `unavailable`. Review submission checks the current pull request head, then posts a single inline comment or one completed review through the REST API with an explicit `commit_id`. Multiline comments carry their side and inclusive start and end lines. Review decisions are Comment, Approve, and Request changes. JSON request bodies travel on stdin. The provider never retries a mutation: a timeout, unexpected response, or server error after dispatch returns an unknown outcome that the caller must reconcile on GitHub. A commit arriving between the check and the write can make the explicitly addressed review outdated; GitHub does not offer an atomic head precondition on these endpoints. @@ -100,7 +100,7 @@ None. Provider output reaches only host-side watchers. - **Batch deadline** — continuation pages of a pull request with many review threads lengthen its host's polling batch; a batch that outlives `ctx.forge`'s `fetchTimeoutMs` reports `unavailable` for every pull request in it, not only for the one with many threads. - **Rewritten base branch** — the diff starts from the merge base of the head and the base commit GitHub recorded at the pull request's last update. After the base branch is force-pushed, that merge base can differ from the one GitHub's pull request view uses, so the patch can include or omit commits until the pull request is updated. - **No fork discovery** — head lookups exclude pull requests whose head is in another repository. -- **Rate limits** — every poll costs one GraphQL query per host, plus one query per further page of 100 review threads, and a complete check read one more query per further page of 100 check records; every diff read costs one GraphQL query and one REST request. All count against the signed-in account’s rate limit. +- **Rate limits** — every poll costs one GraphQL query per host, plus one query per further page of 100 review threads, and a complete check read one more query per further page of 100 check records; every diff read costs one GraphQL query and one REST request, and a second REST request when the base commit changed after the latest status. All count against the signed-in account’s rate limit. - **No merge queue** — a base branch that requires a merge queue refuses a direct merge, and the provider never queues one. diff --git a/packages/experimental/forge-github/README.zh.md b/packages/experimental/forge-github/README.zh.md index 205a8f0439..0851001833 100644 --- a/packages/experimental/forge-github/README.zh.md +++ b/packages/experimental/forge-github/README.zh.md @@ -46,7 +46,7 @@ kind: "package-reference" pull request 处于打开状态、不是草稿、GitHub 报告其合并状态为 `CLEAN`、`HAS_HOOKS` 或 `UNSTABLE`，且已登录用户具有 write、maintain 或 admin 权限时，`mergeReady` 为真。`mergeMethods` 列出仓库允许的压缩合并、合并提交与变基合并方式，用户默认的合并方式排在最前。标记为可评审运行 `gh pr ready --repo //`。合并经 `gh api` 调用 `PUT repos///pulls//merge`，带上合并方式与预期的 head SHA，因此有新提交到达后 GitHub 会拒绝，报告为 `stale`。与 `gh pr merge` 不同，它从不启用自动合并、从不加入合并队列，也从不删除 head 分支。被拒绝的操作报告 GitHub 的原因，提到身份验证的诊断转为 `unauthenticated`。 -`diff` 在一次 GraphQL 查询中读取 `headRefOid` 与 `baseRefOid`，head 不是请求的提交时返回 `stale`；请求的提交必须是完整的 40 位或 64 位十六进制提交。否则经 `gh api --allow-escape-sequences` 以 diff 媒体类型读取 `repos///compare/...`，即两个提交的合并基准到 head 的补丁，两端都不跟随移动的分支。实测的所有 open 与已合并 pull request 上，该补丁与不接受提交参数的 `gh pr diff` 逐字节一致。与 pull request diff 不同，compare 接口会返回超过 300 个文件的 diff，因此这类 diff 只有在输出超过 `maxOutputBytes` 或 GitHub 自身拒绝时才为 `too-large`。`--allow-escape-sequences` 保留文件内容中的终端转义序列；否则 `gh` 2.97 及以上版本拒绝输出。更早的 `gh` 在发出请求前就以 `unknown flag` 拒绝该选项，且原样输出这类序列，因此遇到这一拒绝时去掉该选项重读一次。`fileText` 经 `gh api` 以 raw 媒体类型读取 `repos///contents/?ref=`，只接受完整的 40 位或 64 位十六进制提交。`fileUrl` 返回 `https://///blob//`。两者的路径都逐段做百分号编码。`commits` 经 GraphQL 读取拉取请求的提交，每页 100 个，owner、name、number 与游标作为变量传入，直到 `maxCommits`；每一页报告的 head 与提交总数必须与第一页相同，游标也不能与之前任何一页重复，否则返回 `unavailable`。`compare` 用与 `diff` 相同的 compare 请求读取两个完整提交的合并基准到第二个提交的补丁。被拒绝的读取在 HTTP 404 时为 `not-found`，诊断提到身份验证时为 `unauthenticated`，GitHub 以 HTTP 406 拒绝或报告 `too_large` 错误时（diff 或文件超过其大小上限即会如此）为 `too-large`，其余为 `unavailable`，并带有 GitHub 的原因。输出超过 `maxOutputBytes` 的读取为 `too-large`，超过 `timeoutMs` 的为 `unavailable`。 +`diff` 在一次 GraphQL 查询中读取 `headRefOid` 与 `baseRefOid`，head 不是请求的提交时返回 `stale`；请求的提交必须是完整的 40 位或 64 位十六进制提交。否则经 `gh api --allow-escape-sequences` 以 diff 媒体类型读取 `repos///compare/...`，即两个提交的合并基准到 head 的补丁，两端都不跟随移动的分支。每个状态快照把 `baseRefOid` 作为 `baseSha` 携带；拿到它时，`diff` 在 GraphQL 查询确认 head 的同时就开始这次比较，head 已过时或基准提交不同时会取消提前开始的比较并等它结束，然后才返回。实测的所有 open 与已合并 pull request 上，该补丁与不接受提交参数的 `gh pr diff` 逐字节一致。与 pull request diff 不同，compare 接口会返回超过 300 个文件的 diff，因此这类 diff 只有在输出超过 `maxOutputBytes` 或 GitHub 自身拒绝时才为 `too-large`。`--allow-escape-sequences` 保留文件内容中的终端转义序列；否则 `gh` 2.97 及以上版本拒绝输出。更早的 `gh` 在发出请求前就以 `unknown flag` 拒绝该选项，且原样输出这类序列，因此遇到这一拒绝时去掉该选项重读一次。`fileText` 经 `gh api` 以 raw 媒体类型读取 `repos///contents/?ref=`，只接受完整的 40 位或 64 位十六进制提交。`fileUrl` 返回 `https://///blob//`。两者的路径都逐段做百分号编码。`commits` 经 GraphQL 读取拉取请求的提交，每页 100 个，owner、name、number 与游标作为变量传入，直到 `maxCommits`；每一页报告的 head 与提交总数必须与第一页相同，游标也不能与之前任何一页重复，否则返回 `unavailable`。`compare` 用与 `diff` 相同的 compare 请求读取两个完整提交的合并基准到第二个提交的补丁。被拒绝的读取在 HTTP 404 时为 `not-found`，诊断提到身份验证时为 `unauthenticated`，GitHub 以 HTTP 406 拒绝或报告 `too_large` 错误时（diff 或文件超过其大小上限即会如此）为 `too-large`，其余为 `unavailable`，并带有 GitHub 的原因。输出超过 `maxOutputBytes` 的读取为 `too-large`，超过 `timeoutMs` 的为 `unavailable`。 提交评审时先检查 PR 当前的 head，再通过 REST API 发布单条行内评论或一次完整评审，并显式指定 `commit_id`。多行评论携带所属侧及包含端点的起止行号。评审结论为“评论”“批准”或“请求更改”。JSON 请求正文通过标准输入传递。provider 不会重试写操作；发送后的超时、异常响应或服务端错误会返回结果不明，由调用方到 GitHub 核对。检查与写入之间到来的新提交可能让已明确定位到旧提交的评审过时；GitHub 的这些接口不提供原子的 head 前置条件。 @@ -100,7 +100,7 @@ pull request 处于打开状态、不是草稿、GitHub 报告其合并状态为 - **批次时限**——评审讨论很多的 pull request 的续页会拉长其主机的轮询批次；批次超过 `ctx.forge` 的 `fetchTimeoutMs` 时，批次内每个 pull request 都报告 `unavailable`，而不只是讨论很多的那个。 - **基准分支被改写**——diff 从 head 与 GitHub 在 pull request 最后一次更新时记录的基准提交的合并基准开始。基准分支被强制推送后，该合并基准可能不同于 GitHub pull request 页面使用的合并基准，因此在 pull request 更新前，补丁可能多出或缺少提交。 - **不发现 fork**——按 head 分支查找时，不包含 head 位于其他仓库的 pull request。 -- **速率限制**——每次轮询为每个主机消耗一次 GraphQL 查询，评审讨论每多一页（100 个）再消耗一次；完整检查读取时检查记录每多一页（100 条）再消耗一次；每次 diff 读取消耗一次 GraphQL 查询与一次 REST 请求。这些都计入已登录账号的速率配额。 +- **速率限制**——每次轮询为每个主机消耗一次 GraphQL 查询，评审讨论每多一页（100 个）再消耗一次；完整检查读取时检查记录每多一页（100 条）再消耗一次；每次 diff 读取消耗一次 GraphQL 查询与一次 REST 请求；最新状态之后基准提交有变化时再多一次 REST 请求。这些都计入已登录账号的速率配额。 - **不支持合并队列**——要求合并队列的基准分支会拒绝直接合并，提供方也不会把变更请求加入队列。 diff --git a/packages/experimental/forge-github/src/graphql.ts b/packages/experimental/forge-github/src/graphql.ts index 75922515eb..92bcc4fbb2 100644 --- a/packages/experimental/forge-github/src/graphql.ts +++ b/packages/experimental/forge-github/src/graphql.ts @@ -32,7 +32,7 @@ const THREAD_FIELDS = 'pageInfo { hasNextPage endCursor } nodes { isResolved }' function pullRequestFields(threads: number): string { return `id number url title state isDraft updatedAt closedAt author { login } - headRefName headRefOid baseRefName additions deletions mergeable mergeStateStatus reviewDecision + headRefName headRefOid baseRefName baseRefOid additions deletions mergeable mergeStateStatus reviewDecision reviewThreads(first: ${threads}) { ${THREAD_FIELDS} } commits(last: 1) { nodes { commit { statusCheckRollup { id state contexts(first: ${PAGE_SIZE}) { ${CHECK_FIELDS} @@ -132,6 +132,7 @@ const pullRequestSchema = z.object({ headRefName: z.string(), headRefOid: z.string(), baseRefName: z.string(), + baseRefOid: z.string().refine(isCommit), additions: z.number().int().nonnegative(), deletions: z.number().int().nonnegative(), mergeable: z.string(), @@ -466,6 +467,7 @@ function toSnapshot( headBranch: node.headRefName, headSha: node.headRefOid, baseBranch: node.baseRefName, + baseSha: node.baseRefOid, additions: node.additions, deletions: node.deletions, mergeability: mergeability(node.mergeable), diff --git a/packages/experimental/forge-github/src/provider.ts b/packages/experimental/forge-github/src/provider.ts index 67e963e3d9..d94a277bb8 100644 --- a/packages/experimental/forge-github/src/provider.ts +++ b/packages/experimental/forge-github/src/provider.ts @@ -159,24 +159,44 @@ export class GitHubProvider implements ForgeProvider { * current head and the base commit GitHub recorded with it; a head other * than `headSha` returns `stale`. Otherwise the compare endpoint reads the * patch from the merge base of that base commit and `headSha`, as the pull - * request's own diff does, with both ends fixed. + * request's own diff does, with both ends fixed. Given the `base` the latest + * status recorded with `headSha`, the comparison from it runs while the + * GraphQL read confirms the head; a stale head or another base commit + * cancels that comparison and waits for it to end before the read returns. */ - async diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise { + async diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal, base?: string): Promise { const { host, owner, name } = checkedRepository(ref) const failure = this.hostFailure(host) if (failure !== undefined) return failure if (!isCommit(headSha)) throw new Error(`forge-github: invalid head commit ${JSON.stringify(headSha)}`) - const output = await this.read([ - 'api', 'graphql', '--hostname', host, - '-f', `query=${PULL_REQUEST_COMMITS_QUERY}`, '-f', `owner=${owner}`, '-f', `name=${name}`, '-F', `number=${ref.number}`, - ], signal) - if (typeof output !== 'string') return output - const commits = mapPullRequestCommitsResponse(parseJson(output)) - if (commits === undefined) return { code: 'unavailable', message: 'unexpected GitHub response to the pull request commit read' } - if (commits.head.toLowerCase() !== headSha.toLowerCase()) { - return { code: 'stale', message: `the pull request head moved from ${headSha} to ${commits.head}` } + if (base !== undefined && !isCommit(base)) throw new Error(`forge-github: invalid base commit ${JSON.stringify(base)}`) + const compare = async (from: string, operation: AbortSignal): Promise => + await readComparison(async (args, inner) => await this.read(args, inner), ref.repository, from, headSha, operation) + const speculation = new AbortController() + const early = base === undefined ? undefined : { base, patch: compare(base, AbortSignal.any([signal, speculation.signal])) } + // The early comparison is observed here, so a cancelled or unused one never rejects unhandled. + const settled = early?.patch.then(() => undefined, () => undefined) + const discard = async (): Promise => { + speculation.abort() + await settled + } + try { + const output = await this.read([ + 'api', 'graphql', '--hostname', host, + '-f', `query=${PULL_REQUEST_COMMITS_QUERY}`, '-f', `owner=${owner}`, '-f', `name=${name}`, '-F', `number=${ref.number}`, + ], signal) + if (typeof output !== 'string') return output + const commits = mapPullRequestCommitsResponse(parseJson(output)) + if (commits === undefined) return { code: 'unavailable', message: 'unexpected GitHub response to the pull request commit read' } + if (commits.head.toLowerCase() !== headSha.toLowerCase()) { + return { code: 'stale', message: `the pull request head moved from ${headSha} to ${commits.head}` } + } + if (early?.base.toLowerCase() === commits.base.toLowerCase()) return await early.patch + await discard() + return await compare(commits.base, signal) + } finally { + await discard() } - return await readComparison(async (args, operation) => await this.read(args, operation), ref.repository, commits.base, headSha, signal) } /** Read one file at a commit through the contents API in its raw form. */ diff --git a/packages/experimental/forge-github/tests/fixtures.ts b/packages/experimental/forge-github/tests/fixtures.ts index 66f2f47652..bc5d1297b0 100644 --- a/packages/experimental/forge-github/tests/fixtures.ts +++ b/packages/experimental/forge-github/tests/fixtures.ts @@ -54,6 +54,7 @@ export function pullRequestNode(number: number, overrides: Record { headBranch: 'topic-1', headSha: 'f'.repeat(40), baseBranch: 'master', + baseSha: 'b'.repeat(40), additions: 128, deletions: 14, mergeability: 'mergeable', diff --git a/packages/experimental/forge-github/tests/provider.spec.ts b/packages/experimental/forge-github/tests/provider.spec.ts index 1993c8cee6..d923d7425e 100644 --- a/packages/experimental/forge-github/tests/provider.spec.ts +++ b/packages/experimental/forge-github/tests/provider.spec.ts @@ -1,5 +1,5 @@ import { changeRequestKey } from '@deepseek-ai/dsh-experimental-forge' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { GhOutputLimitError, GhTimeoutError, GhUnavailableError } from '../src/gh.ts' import type { GhOutput, GhRunner } from '../src/gh.ts' import { PULL_REQUEST_COMMITS_QUERY, THREADS_QUERY } from '../src/graphql.ts' @@ -446,6 +446,55 @@ describe('GitHub provider reads', () => { expect(gh.calls[1]?.at(-1)).toBe(`repos/octo/demo/compare/${base}...${head.toUpperCase()}`) }) + it('starts the comparison from the base the status recorded while it confirms the head', async () => { + const confirmed = Promise.withResolvers() + const gh = fakeGh(args => args.includes(`query=${PULL_REQUEST_COMMITS_QUERY}`) + ? confirmed.promise : output('diff --git a/x b/x\n')) + + const read = provider(gh.run).diff(pull(7), head, signal, base) + await vi.waitFor(() => { expect(gh.calls).toHaveLength(2) }) + expect(gh.calls.map(call => call.at(-1))).toContain(`repos/octo/demo/compare/${base}...${head}`) + confirmed.resolve(output({ data: { repository: { pullRequest: { headRefOid: head, baseRefOid: base.toUpperCase() } } } })) + + await expect(read).resolves.toBe('diff --git a/x b/x\n') + expect(gh.calls).toHaveLength(2) + }) + + it('cancels the early comparison and waits for it when the head moved, the base differs, or the head read fails', async () => { + const moved = 'e'.repeat(40) + for (const [answer, expected, compares] of [ + [output({ data: { repository: { pullRequest: { headRefOid: pushed, baseRefOid: base } } } }), + { code: 'stale', message: `the pull request head moved from ${head} to ${pushed}` }, [base]], + [output({ data: { repository: { pullRequest: { headRefOid: head, baseRefOid: moved } } } }), 'diff --git a/y b/y\n', [base, moved]], + [output('', 1, 'gh: Something went wrong (HTTP 502)'), { code: 'unavailable', message: 'gh: Something went wrong (HTTP 502)' }, [base]], + ] as const) { + const calls: string[][] = [] + const cancelled: string[] = [] + const run: GhRunner = async (args, operation) => { + calls.push([...args]) + if (args.includes(`query=${PULL_REQUEST_COMMITS_QUERY}`)) return answer + const range = String(args.at(-1)) + if (range.includes(`${moved}...`)) return output('diff --git a/y b/y\n') + // The early comparison runs until the provider cancels it. + await new Promise((resolve) => { operation.addEventListener('abort', resolve, { once: true }) }) + cancelled.push(range) + throw operation.reason + } + + await expect(provider(run).diff(pull(7), head, signal, base)).resolves.toEqual(expected) + expect(calls.map(call => String(call.at(-1))).filter(target => target.includes('/compare/')) + .map(target => target.split('/compare/')[1]?.split('...')[0])).toEqual(compares) + expect(cancelled).toEqual([`repos/octo/demo/compare/${base}...${head}`]) + } + }) + + it('refuses a base that is not a full commit object name before starting gh', async () => { + const gh = pinned() + + await expect(provider(gh.run).diff(pull(7), head, signal, 'HEAD~1')).rejects.toThrow('forge-github: invalid base commit "HEAD~1"') + expect(gh.calls).toEqual([]) + }) + it('reports a head that moved past the requested commit as stale without reading the diff', async () => { const gh = pinned({ head: pushed, base }) diff --git a/packages/experimental/forge-session/tests/loader-composition.spec.ts b/packages/experimental/forge-session/tests/loader-composition.spec.ts index 486e4f0635..f403e347a4 100644 --- a/packages/experimental/forge-session/tests/loader-composition.spec.ts +++ b/packages/experimental/forge-session/tests/loader-composition.spec.ts @@ -80,6 +80,7 @@ const node = (number) => ({ headRefName: number === 7 ? 'feat' : 'other', headRefOid: 'a'.repeat(40), baseRefName: 'main', + baseRefOid: 'e'.repeat(40), additions: 1, deletions: 0, mergeable: 'MERGEABLE', diff --git a/packages/experimental/forge/README.i18n.yaml b/packages/experimental/forge/README.i18n.yaml index ce02168f4c..19fb0423c8 100644 --- a/packages/experimental/forge/README.i18n.yaml +++ b/packages/experimental/forge/README.i18n.yaml @@ -15,8 +15,8 @@ en: d152484eb41ac6b4 zh: 2f8a621b991260a1 /deepseek-ai-dsh-experimental-forge/use-this-package: - en: 66ce801db0de565f - zh: 884973fad2ad2540 + en: 4b29b898061bcc4e + zh: fd37c2a8b06e7aaf /deepseek-ai-dsh-experimental-forge/understand-the-implementation: en: e193cb547c47069b zh: faa517522402ea41 diff --git a/packages/experimental/forge/README.md b/packages/experimental/forge/README.md index 8727e8cd2b..b316d57a78 100644 --- a/packages/experimental/forge/README.md +++ b/packages/experimental/forge/README.md @@ -36,7 +36,7 @@ Mount the registry with at least one provider package, such as the GitHub provid `markReady(ref, signal)` marks a draft ready for review, and `merge(ref, { method, headSha }, signal)` merges a change request only while its head is still `headSha`. Both delegate to the provider of the reference's forge, throw when none is registered, and resolve to undefined once the host accepted the change or to a `ForgeActionFailure`: `unauthenticated`, `stale` when the head moved past `headSha`, or `failed` with the host's reason. After either settles, a watched change request is polled at once. Snapshots carry `mergeReady`, whether the host would merge the change request now for the signed-in user, and `mergeMethods`, the methods the repository accepts with that user's usual one first. -`diff(ref, headSha, signal)` reads the change request's unified patch at head commit `headSha` as the host computes it, with `a/` and `b/` path prefixes, and `fileText(ref, commit, path, signal)` reads one file of its repository at a commit. Both delegate to the provider of the reference's forge, throw when none is registered, and resolve to the text or to a `ForgeFailure`, which is `too-large` when the host or the provider refuses an answer over its size limit. A provider returns a patch only when it describes `headSha`; once the head is another commit, `diff` resolves to `stale`. The first stale read at a head commit polls a watched change request at once so its next status names the current head; later stale reads at that commit wait for the regular poll, which bounds reads while the host's answers briefly disagree. Snapshots carry `unresolvedThreads` with `threadsTruncated`, which marks the count as a lower bound when the provider counted only some review threads. `fileUrl(ref, commit, path)` returns the web page of that file at that commit. +`diff(ref, headSha, signal)` reads the change request's unified patch at head commit `headSha` as the host computes it, with `a/` and `b/` path prefixes, and `fileText(ref, commit, path, signal)` reads one file of its repository at a commit. Both delegate to the provider of the reference's forge, throw when none is registered, and resolve to the text or to a `ForgeFailure`, which is `too-large` when the host or the provider refuses an answer over its size limit. A provider returns a patch only when it describes `headSha`; once the head is another commit, `diff` resolves to `stale`. The first stale read at a head commit polls a watched change request at once so its next status names the current head; later stale reads at that commit wait for the regular poll, which bounds reads while the host's answers briefly disagree. When the latest status of a watched change request names `headSha` and carries `baseSha`, the base commit the host recorded with that head, `diff` passes it to the provider, which may start reading from it before it confirms the head. Snapshots carry `unresolvedThreads` with `threadsTruncated`, which marks the count as a lower bound when the provider counted only some review threads. `fileUrl(ref, commit, path)` returns the web page of that file at that commit. Providers may implement `commits` and `compare` together. `commits(ref, signal)` lists a change request's commits oldest first, each with its full object name, first parent, title, author, and author time, at one head commit and with a `truncated` flag; `compare(ref, base, head, signal)` reads the unified patch from commit `base` to commit `head`. The registry returns undefined from `commits` and an `unavailable` failure from `compare` when the provider lacks either method, so a consumer can offer commit views only where both exist. diff --git a/packages/experimental/forge/README.zh.md b/packages/experimental/forge/README.zh.md index 168012126c..e931eec159 100644 --- a/packages/experimental/forge/README.zh.md +++ b/packages/experimental/forge/README.zh.md @@ -36,7 +36,7 @@ kind: "package-reference" `markReady(ref, signal)` 把草稿标记为可评审；`merge(ref, { method, headSha }, signal)` 只在 head 仍是 `headSha` 时合并变更请求。两者都委托给引用所属 forge 的提供方，没有注册提供方时抛错；主机接受时 resolve 为 undefined，否则为 `ForgeActionFailure`：`unauthenticated`、head 已越过 `headSha` 时的 `stale`，或带主机原因的 `failed`。两者结束后，被监听的变更请求会立即重新轮询。快照带有 `mergeReady`（主机现在是否会为已登录用户合并该变更请求）与 `mergeMethods`（仓库接受的合并方式，该用户常用的方式排在最前）。 -`diff(ref, headSha, signal)` 读取托管平台计算出的变更请求在 head 提交 `headSha` 处的统一补丁，路径带 `a/` 与 `b/` 前缀；`fileText(ref, commit, path, signal)` 读取其仓库中某个提交下的一个文件。两者都委托给引用所属 forge 的提供方，没有注册提供方时抛错，resolve 为文本或 `ForgeFailure`；托管平台或提供方因结果超过其大小上限而拒绝时，失败为 `too-large`。提供方只在补丁描述 `headSha` 时返回补丁；head 已是其他提交时，`diff` resolve 为 `stale`。同一 head 提交的第一次 stale 读取会让被 watch 的变更请求立即重新轮询，使下一次状态给出当前 head；之后在该提交上的 stale 读取等待常规轮询，从而在托管平台的应答短暂不一致时限制读取次数。快照携带 `unresolvedThreads` 与 `threadsTruncated`；提供方只统计了部分评审讨论时，后者表示该计数只是下限。`fileUrl(ref, commit, path)` 返回该文件在该提交下的网页地址。 +`diff(ref, headSha, signal)` 读取托管平台计算出的变更请求在 head 提交 `headSha` 处的统一补丁，路径带 `a/` 与 `b/` 前缀；`fileText(ref, commit, path, signal)` 读取其仓库中某个提交下的一个文件。两者都委托给引用所属 forge 的提供方，没有注册提供方时抛错，resolve 为文本或 `ForgeFailure`；托管平台或提供方因结果超过其大小上限而拒绝时，失败为 `too-large`。提供方只在补丁描述 `headSha` 时返回补丁；head 已是其他提交时，`diff` resolve 为 `stale`。同一 head 提交的第一次 stale 读取会让被 watch 的变更请求立即重新轮询，使下一次状态给出当前 head；之后在该提交上的 stale 读取等待常规轮询，从而在托管平台的应答短暂不一致时限制读取次数。被 watch 的变更请求的最新状态给出 `headSha` 并携带 `baseSha`（托管平台随该 head 记录的基准提交）时，`diff` 把它传给提供方，提供方可以在确认 head 之前就从它开始读取。快照携带 `unresolvedThreads` 与 `threadsTruncated`；提供方只统计了部分评审讨论时，后者表示该计数只是下限。`fileUrl(ref, commit, path)` 返回该文件在该提交下的网页地址。 提供方可以同时实现 `commits` 与 `compare`。`commits(ref, signal)` 在一个 head 提交上按从旧到新列出变更请求的提交，每个提交带完整对象名、第一个父提交、标题、作者与作者时间，并带 `truncated` 标志；`compare(ref, base, head, signal)` 读取从提交 `base` 到提交 `head` 的统一补丁。提供方缺少其中任一方法时，注册表的 `commits` 返回 undefined，`compare` 返回 `unavailable` 失败，因此使用方只在两者都存在时提供按提交查看。 diff --git a/packages/experimental/forge/src/index.ts b/packages/experimental/forge/src/index.ts index f52f77e818..4040c1f836 100644 --- a/packages/experimental/forge/src/index.ts +++ b/packages/experimental/forge/src/index.ts @@ -242,7 +242,8 @@ export class ForgeService extends Service { * computes them. The first `stale` failure at a head commit polls a watched * change request again at once, so its next status names the current head; * later stale reads at the same commit wait for the regular poll, which - * bounds the reads while the host's answers disagree. + * bounds the reads while the host's answers disagree. When the latest status of a watched change request names + * `headSha`, its `baseSha` goes to the provider, which may then read the patch while it confirms the head. * @param ref - change request served by a registered provider. * @param headSha - head commit the patch must describe, usually the head of the latest status. * @param signal - cancellation of the read. @@ -250,8 +251,10 @@ export class ForgeService extends Service { * @throws when no provider serves the change request's forge, or with `signal.reason` when it aborts. */ async diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise { + const latest = this.watches.get(changeRequestKey(ref))?.latest + const base = latest !== undefined && 'snapshot' in latest && latest.snapshot.headSha === headSha ? latest.snapshot.baseSha : undefined const result = await this.invoke(this.registration(ref.repository.forge), signal, - (provider, operation) => provider.diff(ref, headSha, operation)) + (provider, operation) => provider.diff(ref, headSha, operation, base)) const watch = this.watches.get(changeRequestKey(ref)) if (typeof result !== 'string' && result.code === 'stale' && watch !== undefined && watch.staleHead !== headSha) { watch.staleHead = headSha diff --git a/packages/experimental/forge/src/types.ts b/packages/experimental/forge/src/types.ts index 18c8b02e6d..b691744ec1 100644 --- a/packages/experimental/forge/src/types.ts +++ b/packages/experimental/forge/src/types.ts @@ -79,6 +79,11 @@ export interface ChangeRequestSnapshot { readonly headSha: string /** Branch the change request merges into. */ readonly baseBranch: string + /** + * Base commit the host recorded with `headSha`, whose merge base with the head starts the change request's diff; + * omitted when the provider does not report it. + */ + readonly baseSha?: string /** Lines the change request adds. */ readonly additions: number /** Lines the change request deletes. */ @@ -244,9 +249,10 @@ export interface ForgeProvider { /** * Read the changes of a change request at head commit `headSha` as the host computes them, as one unified * patch with `a/` and `b/` path prefixes. A patch is returned only when it describes `headSha`; once the - * head is another commit, the read resolves to a `stale` failure. + * head is another commit, the read resolves to a `stale` failure. `base`, when given, is the `baseSha` the + * latest status recorded with `headSha`; a provider may start reading from it before it confirms the head. */ - diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise + diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal, base?: string): Promise /** Read the text of `path` at `commit` in the change request's repository. */ fileText(ref: ChangeRequestRef, commit: string, path: string, signal: AbortSignal): Promise /** Build the web page of `path` at `commit` in the change request's repository. */ diff --git a/packages/experimental/forge/tests/watch.spec.ts b/packages/experimental/forge/tests/watch.spec.ts index 4537d82670..0988bb3ad1 100644 --- a/packages/experimental/forge/tests/watch.spec.ts +++ b/packages/experimental/forge/tests/watch.spec.ts @@ -223,6 +223,22 @@ describe('forge watch', () => { expect(provider.fetches).toHaveLength(3) }) + it('passes the base commit of the latest status to a diff read at its head only', async () => { + const provider = new FakeProvider() + provider.results.set(key, snapshot(ref(1), { baseSha: 'e'.repeat(40) })) + const context = await setup(provider) + collect(context) + await vi.advanceTimersByTimeAsync(0) + const diff = vi.spyOn(provider, 'diff') + const signal = new AbortController().signal + + await context.forge.diff(ref(1), 'a'.repeat(40), signal) + await context.forge.diff(ref(1), 'f'.repeat(40), signal) + await context.forge.diff(ref(2), 'a'.repeat(40), signal) + + expect(diff.mock.calls.map(call => call[3])).toEqual(['e'.repeat(40), undefined, undefined]) + }) + it('polls a watched change request at once after a diff read finds a newer head, and only then', async () => { const provider = new FakeProvider() provider.results.set(key, snapshot(ref(1))) diff --git a/packages/extensions/tool-cordis/src/api-catalog.ts b/packages/extensions/tool-cordis/src/api-catalog.ts index de37043f87..690a1a7476 100644 --- a/packages/extensions/tool-cordis/src/api-catalog.ts +++ b/packages/extensions/tool-cordis/src/api-catalog.ts @@ -1271,7 +1271,7 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ }, { signature: 'async diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise', - description: 'Read the changes of a change request at one head commit as its host computes them. The first `stale` failure at a head commit polls a watched change request again at once, so its next status names the current head; later stale reads at the same commit wait for the regular poll, which bounds the reads while the host\'s answers disagree.', + description: 'Read the changes of a change request at one head commit as its host computes them. The first `stale` failure at a head commit polls a watched change request again at once, so its next status names the current head; later stale reads at the same commit wait for the regular poll, which bounds the reads while the host\'s answers disagree. When the latest status of a watched change request names `headSha`, its `baseSha` goes to the provider, which may then read the patch while it confirms the head.', parameters: [{ name: 'ref', description: 'change request served by a registered provider.' }, { name: 'headSha', description: 'head commit the patch must describe, usually the head of the latest status.' }, { name: 'signal', description: 'cancellation of the read.' }], returns: 'one unified patch with `a/` and `b/` path prefixes, or why the host did not provide it at `headSha`.', throws: ['when no provider serves the change request\'s forge, or with `signal.reason` when it aborts.'], @@ -6027,7 +6027,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'ChangeRequestSnapshot', - declaration: 'export interface ChangeRequestSnapshot {\n readonly ref: ChangeRequestRef;\n readonly url: string;\n readonly title: string;\n readonly state: ChangeRequestState;\n readonly draft: boolean;\n readonly author: string | null;\n readonly headBranch: string;\n readonly headSha: string;\n readonly baseBranch: string;\n readonly additions: number;\n readonly deletions: number;\n readonly mergeability: Mergeability;\n readonly mergeReady: boolean;\n readonly mergeMethods: readonly MergeMethod[];\n readonly reviewDecision: ReviewDecision;\n readonly checks: readonly ChangeRequestCheck[];\n readonly checksRollup: ChecksRollup;\n readonly checksTruncated: boolean;\n readonly unresolvedThreads: number;\n readonly threadsTruncated: boolean;\n readonly updatedAt: string;\n readonly closedAt: string | null;\n}', + declaration: 'export interface ChangeRequestSnapshot {\n readonly ref: ChangeRequestRef;\n readonly url: string;\n readonly title: string;\n readonly state: ChangeRequestState;\n readonly draft: boolean;\n readonly author: string | null;\n readonly headBranch: string;\n readonly headSha: string;\n readonly baseBranch: string;\n readonly baseSha?: string;\n readonly additions: number;\n readonly deletions: number;\n readonly mergeability: Mergeability;\n readonly mergeReady: boolean;\n readonly mergeMethods: readonly MergeMethod[];\n readonly reviewDecision: ReviewDecision;\n readonly checks: readonly ChangeRequestCheck[];\n readonly checksRollup: ChecksRollup;\n readonly checksTruncated: boolean;\n readonly unresolvedThreads: number;\n readonly threadsTruncated: boolean;\n readonly updatedAt: string;\n readonly closedAt: string | null;\n}', }, { name: 'ChangeRequestState', @@ -6731,7 +6731,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'ForgeProvider', - declaration: 'export interface ForgeProvider {\n readonly forge: string;\n submitReview?(ref: ChangeRequestRef, request: ForgeReviewRequest, signal: AbortSignal): Promise;\n repositoryFromRemote(url: string): ForgeRepository | undefined;\n changeRequestFromUrl(url: string): ChangeRequestRef | undefined;\n changeRequestUrl(ref: ChangeRequestRef): string;\n fetch(refs: readonly ChangeRequestRef[], signal: AbortSignal): Promise>;\n fetchChecks?(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n findByHead(repository: ForgeRepository, branch: string, signal: AbortSignal): Promise;\n readiness(host: string, signal: AbortSignal): Promise;\n markReady(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n merge(ref: ChangeRequestRef, request: MergeRequest, signal: AbortSignal): Promise;\n diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal): Promise;\n fileText(ref: ChangeRequestRef, commit: string, path: string, signal: AbortSignal): Promise;\n fileUrl(ref: ChangeRequestRef, commit: string, path: string): string;\n commits?(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n compare?(ref: ChangeRequestRef, base: string, head: string, s /* …truncated — full shape in source */', + declaration: 'export interface ForgeProvider {\n readonly forge: string;\n submitReview?(ref: ChangeRequestRef, request: ForgeReviewRequest, signal: AbortSignal): Promise;\n repositoryFromRemote(url: string): ForgeRepository | undefined;\n changeRequestFromUrl(url: string): ChangeRequestRef | undefined;\n changeRequestUrl(ref: ChangeRequestRef): string;\n fetch(refs: readonly ChangeRequestRef[], signal: AbortSignal): Promise>;\n fetchChecks?(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n findByHead(repository: ForgeRepository, branch: string, signal: AbortSignal): Promise;\n readiness(host: string, signal: AbortSignal): Promise;\n markReady(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n merge(ref: ChangeRequestRef, request: MergeRequest, signal: AbortSignal): Promise;\n diff(ref: ChangeRequestRef, headSha: string, signal: AbortSignal, base?: string): Promise;\n fileText(ref: ChangeRequestRef, commit: string, path: string, signal: AbortSignal): Promise;\n fileUrl(ref: ChangeRequestRef, commit: string, path: string): string;\n commits?(ref: ChangeRequestRef, signal: AbortSignal): Promise;\n compare?(ref: ChangeRequestRef, base: string, /* …truncated — full shape in source */', }, { name: 'ForgeReadiness',