diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 71f14c14a9..ae2c27bce7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,11 +1,11 @@ # Default reviewers -* @lidge-jun @Ingwannu @Wibias +* @lidge-jun @Ingwannu # High-impact runtime behavior -/src/adapters/ @lidge-jun @Ingwannu @Wibias -/src/providers/ @lidge-jun @Ingwannu @Wibias -/src/codex/ @lidge-jun @Ingwannu @Wibias -/src/server/ @lidge-jun @Ingwannu @Wibias +/src/adapters/ @lidge-jun @Ingwannu +/src/providers/ @lidge-jun @Ingwannu +/src/codex/ @lidge-jun @Ingwannu +/src/server/ @lidge-jun @Ingwannu # Repository automation and release security /.github/ @lidge-jun @Ingwannu diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 3214adfdf1..43377c093d 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -9,7 +9,6 @@ review and merge policy. | --- | --- | --- | | [@lidge-jun](https://github.com/lidge-jun) | Project owner | Project direction, releases, repository administration, and final governance decisions | | [@Ingwannu](https://github.com/Ingwannu) | Maintainer | Issue and pull-request triage, `dev` integration, security review, and repository maintenance | -| [@Wibias](https://github.com/Wibias) | Maintainer | Issue and pull-request triage, `dev` integration, and provider/CI maintenance | The table describes project responsibilities. Actual repository permissions remain controlled through GitHub repository settings. @@ -17,6 +16,16 @@ through GitHub repository settings. `dev` is the only integration line. The former `dev2-go` carry duty is retired; see [The retired `dev2-go` line](#the-retired-dev2-go-line). +## Former maintainers + +| GitHub account | Project role | Period | +| --- | --- | --- | +| [@Wibias](https://github.com/Wibias) | Maintainer | 2026-07-27 – 2026-08-19 | + +Former maintainers keep contributor standing and are welcome to open issues and pull requests like +anyone else. Authorship credit in git history, release notes, and code comments is not rewritten +when a maintainer steps down. + ## Review and merge policy - Pull requests target `dev`. It is the only integration line, and promotion to @@ -98,6 +107,24 @@ Adding or removing a maintainer requires: ### Change log +- 2026-08-19 — [@Wibias](https://github.com/Wibias) stepped down as a maintainer + and is now a contributor. This follows his own decision to stop developing + opencodex; it is not a disciplinary action, and it was made with the owner's + agreement (requirement 1). Requirement 2 does not apply to a maintainer's own + resignation, which needs no second maintainer to ratify it. Requirement 3 is + met by this file and `.github/CODEOWNERS`, where the default-reviewer line + and the four runtime paths that listed him (`/src/adapters/`, + `/src/providers/`, `/src/codex/`, `/src/server/`) drop back to the two + remaining maintainers. Repository permission was reduced to read access at + the same time, so the roster and the GitHub settings agree again. + + Nothing he authored is being unwound. His commits, the pull requests he + merged, the release-note attributions, and the code comments citing his + reviews stay exactly as they are, and the trust-lane gate derived from his + work in `.github/scripts/pr-sponsored-surface.cjs` keeps its attribution. + Returning to the maintainer table later would go through the same three + requirements that govern every addition. + - 2026-07-27 — [@Wibias](https://github.com/Wibias) added as a maintainer. Requirement 1 (agreement from the project owner) is met: the owner requested the addition. **Requirement 2 (review by another current maintainer) was @@ -105,10 +132,10 @@ Adding or removing a maintainer requires: carried the addition (`a2693c02`, `dc3a4ade`, `02bbd47a`) landed on `dev` as direct owner pushes with no associated pull request, so no second maintainer reviewed them. Requirement 3 is met by this file and `.github/CODEOWNERS`. - The addition is in effect regardless: @Wibias holds write access on the - repository and has been merging pull requests since 2026-07-26. This entry - records the gap rather than papering over it — a later maintainer change - should go through a reviewed pull request. + The addition took effect regardless: @Wibias held write access on the + repository and merged pull requests from 2026-07-26 until he stepped down on + 2026-08-19. This entry records the gap rather than papering over it — a later + maintainer change should go through a reviewed pull request. Scope covers issue and pull-request triage, `dev` integration, and provider/CI maintenance. (This entry originally also described carrying diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 8d2906e536..7cb2bbb0a7 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -2438,13 +2438,13 @@ async function handleResponsesInner( // call it cannot execute, and the turn showed a bare `aborted` with the file untouched. // Forward auth is the canonical ChatGPT backend speaking Codex's own protocol rather than a // routed provider, so it keeps passing through unguarded, as it does for the rewrites above. - // The guard needs a catalog to compare against, so it stands down when the request carries - // none. That is not the same claim as "no tools means no tool may be called": a passthrough - // request can legitimately omit `tools` entirely and still receive a tool call the client - // understands — `tests/github-copilot-stream-contract.test.ts` sends `{model, input, stream}` - // with no tools and Copilot answers with a `custom_tool_call` for `apply_patch`. Policing an - // empty catalog truncates that turn. An unreadable body lands here too, since it yields no - // names either. + // The guard needs a catalog to compare against, so it stands down when the request omits one. + // An explicit empty catalog is still authoritative: it declares that no client tools may be + // called. A passthrough request can legitimately omit `tools` entirely and still receive a call + // the client understands — `tests/github-copilot-stream-contract.test.ts` sends + // `{model, input, stream}` with no tools and Copilot answers with a `custom_tool_call` for + // `apply_patch`. Policing an absent catalog truncates that turn. An unreadable body lands there + // too because the proxy cannot establish the caller's declared authorization boundary. const outboundRequestBody = (() => { try { const body = JSON.parse(request.body) as unknown; @@ -2460,7 +2460,9 @@ async function handleResponsesInner( // the client's name back. Widening only ever makes the guard fire less; a name declared in // neither place — #1700's `apply_patch` — is still refused. for (const name of toolBridgeMaps.declaredToolNames) declaredWireToolNames.add(name); - const undeclaredToolGuardActive = declaredWireToolNames.size > 0 + const hasExplicitWireToolCatalog = outboundRequestBody !== undefined + && Array.isArray((outboundRequestBody as Record).tools); + const undeclaredToolGuardActive = (declaredWireToolNames.size > 0 || hasExplicitWireToolCatalog) && route.provider.authMode !== "forward"; // A refused turn must not seed `previous_response_id` replay. The inspection branch reads the // untouched upstream stream, so it can still observe a `response.completed` the client never diff --git a/tests/responses-undeclared-tool-guard.test.ts b/tests/responses-undeclared-tool-guard.test.ts index 3cf8a08e17..01c7542b2a 100644 --- a/tests/responses-undeclared-tool-guard.test.ts +++ b/tests/responses-undeclared-tool-guard.test.ts @@ -462,7 +462,7 @@ describe("a refused turn does not become continuation state", () => { }); }); -describe("a request that declares no tools has no catalog to police", () => { +describe("empty and absent tool catalogs", () => { const config = { port: 0, defaultProvider: "fixture", @@ -520,25 +520,39 @@ describe("a request that declares no tools has no catalog to police", () => { // A passthrough request may omit `tools` entirely and still receive a tool call the client // understands — the Copilot contract in tests/github-copilot-stream-contract.test.ts does - // exactly that with `apply_patch`. Refusing on an empty catalog truncates those turns, so the - // guard needs at least one declared name before it has an opinion. - for (const [label, tools] of [["no tools field", undefined], ["tools: []", []]] as const) { - test(`non-streaming, ${label} — relayed, not refused`, async () => { - const response = await post(false, tools, jsonUpstream); - - expect(response.status).toBe(200); - const body = await response.json() as { output: Array> }; - expect(body.output[0]).toMatchObject({ name: "apply_patch" }); - }); + // exactly that with `apply_patch`. With no catalog the proxy has no authorization boundary to + // enforce, so the call remains untouched on both transports. + test("non-streaming, no tools field — relayed, not refused", async () => { + const response = await post(false, undefined, jsonUpstream); - test(`streaming, ${label} — relayed, not refused`, async () => { - const response = await post(true, tools, sseUpstream); - const body = await response.text(); + expect(response.status).toBe(200); + const body = await response.json() as { output: Array> }; + expect(body.output[0]).toMatchObject({ name: "apply_patch" }); + }); - expect(body).not.toContain(UNDECLARED_TOOL_CALL_ERROR_CODE); - expect(body).toContain("response.completed"); - }); - } + test("streaming, no tools field — relayed, not refused", async () => { + const response = await post(true, undefined, sseUpstream); + const body = await response.text(); + + expect(body).not.toContain(UNDECLARED_TOOL_CALL_ERROR_CODE); + expect(body).toContain("response.completed"); + }); + + test("non-streaming, tools: [] — refuses an upstream client tool call", async () => { + const response = await post(false, [], jsonUpstream); + + expect(response.status).toBe(502); + const body = await response.json() as { error: { message: string } }; + expect(body.error.message).toContain('undeclared client tool "apply_patch"'); + }); + + test("streaming, tools: [] — refuses an upstream client tool call", async () => { + const response = await post(true, [], sseUpstream); + const body = await response.text(); + + expect(body).toContain(UNDECLARED_TOOL_CALL_ERROR_CODE); + expect(body).not.toContain("response.completed"); + }); }); describe("undeclaredToolCallNameInResponse", () => {