fix(forge-github): enforce configured hosts before queries
This commit is contained in:
1 parent
cba0128268
commit
fbcfd80e21
5 files changed
+37
-5
No files matched your search
@@ -30,8 +30,8 @@
|
||||
en: 26b946af4b742e17
|
||||
zh: 00b34f0721ac72f6
|
||||
/deepseek-ai-dsh-experimental-forge-github/known-limitations-and-deferred-work:
|
||||
en: 441928a7cc59da3b
|
||||
zh: 96496453ce349cad
|
||||
en: eb7d8000ca55baca
|
||||
zh: 27ba101ffa91175b
|
||||
/deepseek-ai-dsh-experimental-forge-github/known-limitations-and-deferred-work/dev-note:
|
||||
en: e67f633dc6fe7773
|
||||
zh: 3bb0c20c93f16f98
|
||||
@@ -84,7 +84,7 @@ None. Provider output reaches only host-side watchers.
|
||||
|
||||
<a id="known-limitations-and-deferred-work"></a>
|
||||
|
||||
- **GitHub CLI on the host** — `gh` must be installed and signed in for each configured host; tokens in environment variables are not forwarded. Hosts missing from `hosts` are never read.
|
||||
- **GitHub CLI on the host** — `gh` must be installed and signed in for each configured host; tokens in environment variables are not forwarded. Hosts missing from `hosts` are never read, including references retained across configuration changes. A status fetch returns `unavailable` for those references; a head lookup rejects before starting `gh`.
|
||||
- **First 100 checks and review threads** — `checksTruncated` marks a head commit with more checks; unresolved threads after the first 100 are not counted.
|
||||
- **No fork discovery** — head lookups exclude pull requests whose head is in another repository.
|
||||
- **Rate limits** — every poll costs one GraphQL query per host against the signed-in account's rate limit.
|
||||
|
||||
@@ -84,7 +84,7 @@ kind: "package-reference"
|
||||
|
||||
<a id="known-limitations-and-deferred-work"></a>
|
||||
|
||||
- **宿主需要 GitHub CLI**——必须安装 `gh` 并为每个已配置主机登录;环境变量中的令牌不会被转发。不在 `hosts` 中的主机永远不会被读取。
|
||||
- **宿主需要 GitHub CLI**——必须安装 `gh` 并为每个已配置主机登录;环境变量中的令牌不会被转发。不在 `hosts` 中的主机永远不会被读取,包括配置变更后保留的引用。状态读取为这些引用返回 `unavailable`;head 查找会在启动 `gh` 前拒绝。
|
||||
- **前 100 个检查与评审讨论**——head 提交的检查超过上限时以 `checksTruncated` 标记;前 100 个之后的未解决讨论不计入。
|
||||
- **不发现 fork**——按 head 分支查找时,不包含 head 位于其他仓库的 pull request。
|
||||
- **速率限制**——每次轮询都会以已登录账号的速率配额为每个主机消耗一次 GraphQL 查询。
|
||||
|
||||
@@ -45,12 +45,17 @@ export class GitHubProvider implements ForgeProvider {
|
||||
signal: AbortSignal,
|
||||
): Promise<ReadonlyMap<ChangeRequestKey, ChangeRequestSnapshot | ForgeFailure>> {
|
||||
const byHost = new Map<string, ChangeRequestRef[]>()
|
||||
const results = new Map<ChangeRequestKey, ChangeRequestSnapshot | ForgeFailure>()
|
||||
for (const ref of refs) {
|
||||
const failure = this.hostFailure(ref.repository.host)
|
||||
if (failure !== undefined) {
|
||||
results.set(changeRequestKey(ref), failure)
|
||||
continue
|
||||
}
|
||||
const group = byHost.get(ref.repository.host)
|
||||
if (group === undefined) byHost.set(ref.repository.host, [ref])
|
||||
else group.push(ref)
|
||||
}
|
||||
const results = new Map<ChangeRequestKey, ChangeRequestSnapshot | ForgeFailure>()
|
||||
for (const [host, group] of byHost) {
|
||||
for (const [key, result] of await this.fetchHost(host, group, signal)) results.set(key, result)
|
||||
}
|
||||
@@ -58,6 +63,8 @@ export class GitHubProvider implements ForgeProvider {
|
||||
}
|
||||
|
||||
async findByHead(repository: ForgeRepository, branch: string, signal: AbortSignal): Promise<readonly ChangeRequestRef[]> {
|
||||
const failure = this.hostFailure(repository.host)
|
||||
if (failure !== undefined) throw new Error(failure.message)
|
||||
const output = await this.gh([
|
||||
'api', 'graphql', '--hostname', repository.host,
|
||||
'-f', `query=${HEAD_QUERY}`, '-f', `owner=${repository.owner}`, '-f', `name=${repository.name}`, '-f', `branch=${branch}`,
|
||||
@@ -67,6 +74,12 @@ export class GitHubProvider implements ForgeProvider {
|
||||
return mapHeadResponse(body, repository, branch)
|
||||
}
|
||||
|
||||
private hostFailure(host: string): ForgeFailure | undefined {
|
||||
return this.hosts.has(host.toLowerCase()) ? undefined : {
|
||||
code: 'unavailable', message: `forge-github: host ${JSON.stringify(host)} is not configured`,
|
||||
}
|
||||
}
|
||||
|
||||
private async fetchHost(
|
||||
host: string,
|
||||
refs: readonly ChangeRequestRef[],
|
||||
|
||||
@@ -45,6 +45,25 @@ describe('GitHub provider addressing', () => {
|
||||
})
|
||||
|
||||
describe('GitHub provider fetch', () => {
|
||||
it('rejects unconfigured hosts without querying them and still fetches allowed hosts', async () => {
|
||||
const gh = fakeGh(() => output({ data: { r0: { p0: pullRequestNode(1) } } }))
|
||||
const denied = pull(2, { ...enterprise, host: 'unconfigured.example' })
|
||||
const results = await provider(gh.run).fetch([denied, pull(1)], signal)
|
||||
expect(gh.calls.map(call => call[3])).toEqual(['github.com'])
|
||||
expect(results.get(changeRequestKey(denied))).toEqual({ code: 'unavailable', message: 'forge-github: host "unconfigured.example" is not configured' })
|
||||
expect(results.get(changeRequestKey(pull(1)))).toMatchObject({ title: 'Change 1' })
|
||||
})
|
||||
|
||||
it('rejects references retained after their host was removed from configuration', async () => {
|
||||
const gh = fakeGh(() => output(''))
|
||||
const retained = provider(gh.run).changeRequestFromUrl('https://ghe.corp.example/team/svc/pull/2')!
|
||||
const replacement = new GitHubProvider(gh.run, ['github.com'])
|
||||
const results = await replacement.fetch([retained], signal)
|
||||
await expect(replacement.findByHead(retained.repository, 'topic', signal)).rejects.toThrow('is not configured')
|
||||
expect(results.get(changeRequestKey(retained))).toMatchObject({ code: 'unavailable' })
|
||||
expect(gh.calls).toEqual([])
|
||||
})
|
||||
|
||||
it('runs one GraphQL request per host and merges the results', async () => {
|
||||
const gh = fakeGh((args) => {
|
||||
const host = args[3]
|
||||
|
||||
Reference in new issue
Block a user