From bf1b692df934df4baf9bdffd3cae275e5e358db1 Mon Sep 17 00:00:00 2001 From: jiwangyihao Date: Tue, 23 Jun 2026 15:01:49 +0800 Subject: [PATCH] =?UTF-8?q?fix(gitlab-duo):=20direct=5Faccess=20=E9=94=99?= =?UTF-8?q?=E8=AF=AF=E4=BF=9D=E7=95=99=20HTTP=20=E7=8A=B6=E6=80=81?= =?UTF-8?q?=E7=A0=81=E4=BB=A5=E8=A7=A6=E5=8F=91=E5=87=AD=E6=8D=AE=E8=BD=AE?= =?UTF-8?q?=E6=8D=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 机器人指出两处问题，本提交处理： 1) direct_access 带 body message 的错误丢弃了 response.status，导致 streaming auth-retry 路径（extractStatusFromAssistantError -> extractHttpStatusFromError）无法恢复状态、无法刷新/轮换过期 OAuth 或 配额受限的 broker 凭据。现在即便有 body message 也内嵌 HTTP ， 与 create 路径既有约定一致。新增 401 Unauthorized 回归测试断言状态可恢复， 并强化既有 403 配额测试。 2) generate-models 种子注释过度暗示生成期会跑 namespace-scoped 发现。 实际上 gitlab-duo-agent descriptor 故意不带 catalogDiscovery，被 isCatalogDescriptor 过滤排除在生成发现循环之外，因此生成期绝不会拉取 某账号的 aiChatAvailableModels，只播种通用 namespace-free fallback。 修正注释表述，新增两条针对 descriptor 的回归测试：断言 descriptor 无 catalogDiscovery（永不参与生成发现），以及 fallback 模型不带 gitlabDuoWorkflowRootNamespaceId。 --- packages/ai/CHANGELOG.md | 3 ++ .../ai/src/providers/gitlab-duo-workflow.ts | 15 ++++-- .../test/gitlab-duo-workflow-provider.test.ts | 47 ++++++++++++++++++- packages/catalog/scripts/generate-models.ts | 13 +++-- .../gitlab-duo-workflow-discovery.test.ts | 32 +++++++++++++ 5 files changed, 101 insertions(+), 9 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 91c28714b..aa9bf44af 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -65,6 +65,9 @@ - Fixed OpenRouter Anthropic models on the Responses path omitting `cache_control`, so prompt caching engages without forcing Chat Completions. ([#3397](https://github.com/can1357/oh-my-pi/issues/3397)) - Fixed OpenRouter Anthropic Responses follow-up requests replaying prior reasoning items with stale signatures, which caused HTTP 400 `Invalid signature in thinking block` errors after a thinking turn. ([#3399](https://github.com/can1357/oh-my-pi/issues/3399)) - Fixed OpenRouter Anthropic models on the Responses path omitting `cache_control`, so prompt caching engages without forcing Chat Completions. `cacheRetention: "long"` now upgrades the breakpoint to `ttl: "1h"`. ([#3397](https://github.com/can1357/oh-my-pi/issues/3397)) +### Fixed + +- Fixed GitLab Duo Workflow `direct_access` errors dropping the HTTP status when GitLab returned a JSON error body (e.g. a 401 `{"message":"Unauthorized"}` from an expired OAuth token, or a 429 quota body). The thrown error now embeds `HTTP ` alongside the body message so the streaming auth-retry path (`extractStatusFromAssistantError` → `extractHttpStatusFromError`) can recover the status and refresh/rotate the parked broker credential instead of surfacing a hard failure. ## [16.1.16] - 2026-06-23 diff --git a/packages/ai/src/providers/gitlab-duo-workflow.ts b/packages/ai/src/providers/gitlab-duo-workflow.ts index 74cf48f6d..422da36b3 100644 --- a/packages/ai/src/providers/gitlab-duo-workflow.ts +++ b/packages/ai/src/providers/gitlab-duo-workflow.ts @@ -1400,10 +1400,17 @@ async function requestGitLabDuoWorkflowDirectAccess( }); if (!response.ok) { const message = await readGitLabDuoWorkflowResponseErrorMessage(response); - if (message) { - throw new Error(`GitLab Duo Workflow direct_access failed: ${message}`); - } - throw new Error(`GitLab Duo Workflow direct_access failed with HTTP ${response.status}`); + // Always embed the HTTP status, even when the body carries a message: the + // streaming auth-retry/rotation path (`extractStatusFromAssistantError` -> + // `extractHttpStatusFromError`) refreshes/rotates broker credentials only + // when the assistant error exposes `errorStatus` or the message embeds an + // `HTTP ` token. A 401 `{"message":"Unauthorized"}` or a 429 quota + // body would otherwise surface as a hard failure with no recoverable status. + throw new Error( + message + ? `GitLab Duo Workflow direct_access failed with HTTP ${response.status}: ${message}` + : `GitLab Duo Workflow direct_access failed with HTTP ${response.status}`, + ); } const payload = (await response.json()) as GitLabDirectAccessResponse; const token = extractGitLabWorkflowToken(payload); diff --git a/packages/ai/test/gitlab-duo-workflow-provider.test.ts b/packages/ai/test/gitlab-duo-workflow-provider.test.ts index cf90e8bdc..85b17746e 100644 --- a/packages/ai/test/gitlab-duo-workflow-provider.test.ts +++ b/packages/ai/test/gitlab-duo-workflow-provider.test.ts @@ -38,6 +38,7 @@ import type { } from "@oh-my-pi/pi-ai/types"; import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; +import { extractHttpStatusFromError } from "@oh-my-pi/pi-utils"; import { z } from "zod/v4"; const model: Model<"gitlab-duo-agent"> = buildModel({ @@ -1503,7 +1504,51 @@ describe("GitLab Duo Workflow WebSocket state machine", () => { expect(result.errorMessage).toContain("GitLab Duo Workflow direct_access failed"); expect(result.errorMessage).toContain("USAGE_QUOTA_EXCEEDED"); expect(result.errorMessage).toContain("Usage quota exceeded"); - expect(result.errorMessage).not.toBe("GitLab Duo Workflow direct_access failed with HTTP 403"); + // The body message must be preserved AND the HTTP status embedded so the + // streaming auth-retry path can recover it (`extractStatusFromAssistantError` + // -> `extractHttpStatusFromError`) and rotate the parked credential. + expect(result.errorMessage).toContain("HTTP 403"); + expect(extractHttpStatusFromError({ message: result.errorMessage })).toBe(403); + }); + + it("preserves the 401 status for an Unauthorized direct_access body so the credential can rotate", async () => { + const fetchImpl: FetchImpl = async (input: string | URL | Request) => { + const url = String(input); + if (url.includes("/api/graphql")) { + return new Response( + JSON.stringify({ + data: { + aiChatAvailableModels: { + defaultModel: { name: "Claude", ref: "claude_sonnet_4_6_vertex" }, + selectableModels: [], + pinnedModel: null, + }, + }, + }), + { status: 200 }, + ); + } + if (url.includes("/api/v4/ai/duo_workflows/direct_access")) { + // An expired OAuth token: GitLab returns a terse `Unauthorized` body + // with no status digits. Without embedding the HTTP status, the message + // alone ("...failed: Unauthorized") would surface as a hard failure and + // the broker could never refresh/rotate the credential. + return new Response(JSON.stringify({ message: "Unauthorized" }), { status: 401 }); + } + return new Response("{}", { status: 404 }); + }; + + const stream = streamGitLabDuoWorkflow(model, context, { + apiKey: "[REDACTED]", + rootNamespaceId: "gid://gitlab/Group/1", + fetch: fetchImpl, + }); + const result = await stream.result(); + + expect(result.stopReason).toBe("error"); + expect(result.errorMessage).toContain("Unauthorized"); + expect(result.errorMessage).toContain("HTTP 401"); + expect(extractHttpStatusFromError({ message: result.errorMessage })).toBe(401); }); it("auto-discovers a namespace project for the inline flow when none is configured", async () => { diff --git a/packages/catalog/scripts/generate-models.ts b/packages/catalog/scripts/generate-models.ts index 8454778f0..4be82b15d 100644 --- a/packages/catalog/scripts/generate-models.ts +++ b/packages/catalog/scripts/generate-models.ts @@ -500,10 +500,15 @@ async function generateModels() { } // Seed the GitLab Duo Agent fallback model so a fresh install (no credentialed // dynamic discovery/cache yet) still surfaces the provider's default model in the - // built-in catalog. The provider is dynamicModelsAuthoritative, so when live - // `aiChatAvailableModels` discovery succeeds during generation its entries win the - // id-keyed dedup above (catalogProviderModels precede this seed); the seed only - // lands on a credential-less or failed regen. + // built-in catalog. The descriptor deliberately has NO `catalogDiscovery`, so it is + // excluded from the generator's discovery loop (`isCatalogDescriptor` filter above): + // generation never fetches `aiChatAvailableModels` for it. That is intentional — + // Duo discovery is credential- and namespace-scoped, so running it during generation + // would bundle one private account's pinned/selectable models (and its + // `gitlabDuoWorkflowRootNamespaceId`) as authoritative for every fresh install. + // The generic fallback is the only thing bundled; live namespace-scoped models are + // discovered at runtime per credential/workspace. The `authoritativeCatalogProviders` + // guard therefore always passes for this id, kept only to mirror the Sakana seed shape. if (!authoritativeCatalogProviders.has("gitlab-duo-agent")) { allModels.push(buildGitLabDuoWorkflowFallbackModel()); } diff --git a/packages/catalog/test/gitlab-duo-workflow-discovery.test.ts b/packages/catalog/test/gitlab-duo-workflow-discovery.test.ts index dd8996c89..9e0bd34e3 100644 --- a/packages/catalog/test/gitlab-duo-workflow-discovery.test.ts +++ b/packages/catalog/test/gitlab-duo-workflow-discovery.test.ts @@ -3,12 +3,15 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import { + buildGitLabDuoWorkflowFallbackModel, buildGitLabDuoWorkflowModelSpec, discoverGitLabDuoWorkflowNamespace, discoverGitLabDuoWorkflowRuntimeNamespace, fetchGitLabDuoWorkflowModels, } from "@oh-my-pi/pi-catalog/discovery/gitlab-duo-workflow"; import { getSupportedEfforts } from "@oh-my-pi/pi-catalog/model-thinking"; +import { isCatalogDescriptor } from "@oh-my-pi/pi-catalog/provider-models/descriptor-types"; +import { PROVIDER_DESCRIPTORS } from "@oh-my-pi/pi-catalog/provider-models/descriptors"; import { gitLabDuoWorkflowModelManagerOptions } from "@oh-my-pi/pi-catalog/provider-models/special"; import type { FetchImpl } from "@oh-my-pi/pi-catalog/types"; @@ -525,6 +528,35 @@ describe("GitLab Duo Workflow discovery", () => { expect(seed?.reasoning).toBe(false); }); + it("keeps the gitlab-duo-agent descriptor out of catalog generation discovery", () => { + // The descriptor must NOT carry `catalogDiscovery`: that field is the sole gate + // for the generator's discovery loop (`isCatalogDescriptor`). Were it present, + // `generate-models` running on a machine with GitLab credentials would fetch the + // account's namespace-scoped `aiChatAvailableModels` and bundle one private + // namespace's pinned/selectable catalog into models.json as authoritative for + // every fresh install. Only the generic, namespace-free fallback may be bundled; + // live namespace-scoped models are discovered at runtime per credential/workspace. + const descriptor = PROVIDER_DESCRIPTORS.find(entry => entry.providerId === "gitlab-duo-agent"); + expect(descriptor).toBeDefined(); + expect(descriptor?.catalogDiscovery).toBeUndefined(); + expect(descriptor && isCatalogDescriptor(descriptor)).toBe(false); + }); + + it("seeds a namespace-free fallback model carrying no account-scoped namespace id", () => { + // The bundled seed must never leak the generating machine's root namespace. + const seed = buildGitLabDuoWorkflowFallbackModel(); + expect(seed.id).toBe("claude_sonnet_4_6_vertex"); + expect(seed.provider).toBe("gitlab-duo-agent"); + expect(seed).not.toHaveProperty("gitlabDuoWorkflowRootNamespaceId"); + // A credentialed runtime discovery, by contrast, pins the namespace it resolved. + const scoped = buildGitLabDuoWorkflowModelSpec( + { name: "Sonnet", ref: "claude_sonnet_4_6_vertex" }, + undefined, + "root-namespace-123", + ); + expect(scoped.gitlabDuoWorkflowRootNamespaceId).toBe("root-namespace-123"); + }); + it("does not include bearer credentials in namespace discovery errors", async () => { const { fetch } = createMockFetch({ groups: [{ id: "missing" }], models: { missing: null } });