diff --git a/.github/scripts/closed-pr-branch-cleanup.cjs b/.github/scripts/closed-pr-branch-cleanup.cjs index aaca0f6498..57142f19fe 100644 --- a/.github/scripts/closed-pr-branch-cleanup.cjs +++ b/.github/scripts/closed-pr-branch-cleanup.cjs @@ -23,6 +23,20 @@ function normalizeBranchName(value) { return String(value || "").trim(); } +/** + * A commit id, lowercased for comparison. + * + * The REST and GraphQL APIs are not consistent about case, and a full 40-character + * sha compared case-sensitively against an abbreviated or upper-case one silently + * reads as "different" - which here would mean "keep", so the failure direction is + * safe, but it would make the guard useless rather than protective. Anything that + * is not a plausible hex object id becomes null, i.e. unknown. + */ +function normalizeOid(value) { + const text = String(value || "").trim().toLowerCase(); + return /^[0-9a-f]{7,64}$/.test(text) ? text : null; +} + function isProtectedBranch(name) { return PROTECTED_BRANCHES.includes(normalizeBranchName(name)); } @@ -45,6 +59,8 @@ const KEEP_REASONS = Object.freeze({ CROSS_REPOSITORY: "cross-repository-head", MISSING_CLOSED_AT: "missing-closed-at", WITHIN_GRACE: "within-grace-period", + MOVED_SINCE_CLOSE: "branch-moved-since-close", + UNKNOWN_HEAD_SHA: "unknown-head-sha", }); /** @@ -63,12 +79,23 @@ const KEEP_REASONS = Object.freeze({ * contributor's repository and this token has no business there. * - A grace period after `closed_at` leaves room to reopen a PR that was * closed by mistake. + * - The branch must still POINT AT a commit one of those closed pull requests + * had as its head. Matching by NAME alone deletes reused work: `codex/`-style + * names get picked up again all the time, and a branch recreated for new work + * inherits the closed history of every PR that ever used that name. The tip + * moved, so the branch is not the closed PR's branch any more - it only shares + * its label. + * - A branch whose current tip cannot be determined is kept. An unknown tip is + * not evidence of an abandoned branch, and this job's mistakes are not + * recoverable. * * @param {object} input * @param {Array} input.pullRequests Pull requests with - * `headRefName`, `baseRefName`, `state`, `merged`, `closedAt`, and - * `isCrossRepository`. - * @param {Array} input.branches Branch names that currently exist. + * `headRefName`, `headRefOid`, `baseRefName`, `state`, `merged`, `closedAt`, + * and `isCrossRepository`. + * @param {Array} input.branches Branches + * that currently exist. A bare string carries no tip, which is treated as an + * unknown tip and kept. * @param {number} [input.now] Current time in milliseconds. * @param {number} [input.graceDays] Days to wait after `closedAt`. * @returns {{ deletions: Array<{branch: string, pullRequests: number[]}>, @@ -80,7 +107,17 @@ function planClosedPrBranchDeletions({ now = Date.now(), graceDays = DEFAULT_GRACE_DAYS, }) { - const existing = new Set(branches.map(normalizeBranchName).filter(Boolean)); + // Accepts both shapes so an older caller passing bare names still works - it + // just gets the conservative answer, because a name without a tip cannot be + // proven safe to delete. + /** @type {Map} */ + const existing = new Map(); + for (const entry of branches) { + const name = normalizeBranchName(typeof entry === "string" ? entry : entry && entry.name); + if (!name) continue; + const oid = typeof entry === "string" ? null : normalizeOid(entry && entry.oid); + existing.set(name, oid); + } const graceMs = Math.max(0, Number(graceDays) || 0) * 24 * 60 * 60 * 1000; /** @type {Map} */ @@ -104,7 +141,7 @@ function planClosedPrBranchDeletions({ const deletions = []; const keeps = []; - for (const branch of [...existing].sort()) { + for (const branch of [...existing.keys()].sort()) { if (isProtectedBranch(branch)) { keeps.push({ branch, reason: KEEP_REASONS.PROTECTED }); continue; @@ -141,6 +178,33 @@ function planClosedPrBranchDeletions({ continue; } + // The tip check, last because it is the most expensive claim to satisfy and + // the cheaper rules above have already excluded most branches. + // + // A closed PR's head branch is only THIS branch if the branch still points at + // a commit that PR had as its head. Without this, a name reused for new work + // is deleted on the strength of an unrelated PR that happened to share the + // label months earlier - and a deleted branch whose commits were never pushed + // anywhere else is gone. + const currentOid = existing.get(branch) || null; + if (!currentOid) { + keeps.push({ branch, reason: KEEP_REASONS.UNKNOWN_HEAD_SHA }); + continue; + } + const closedOids = new Set( + related.map((pr) => normalizeOid(pr && pr.headRefOid)).filter(Boolean), + ); + // An empty set means the API gave us no head SHA for any of them, which is the + // unknown case again rather than a licence to delete. + if (closedOids.size === 0) { + keeps.push({ branch, reason: KEEP_REASONS.UNKNOWN_HEAD_SHA }); + continue; + } + if (!closedOids.has(currentOid)) { + keeps.push({ branch, reason: KEEP_REASONS.MOVED_SINCE_CLOSE }); + continue; + } + deletions.push({ branch, pullRequests: related diff --git a/.github/workflows/cleanup-closed-pr-branches.yml b/.github/workflows/cleanup-closed-pr-branches.yml index c260bc03a8..4b0c1229a9 100644 --- a/.github/workflows/cleanup-closed-pr-branches.yml +++ b/.github/workflows/cleanup-closed-pr-branches.yml @@ -70,6 +70,10 @@ jobs: merged: Boolean(pr.merged_at), closedAt: pr.closed_at, headRefName: pr.head && pr.head.ref, + // The tip this PR actually pointed at. Without it the planner cannot + // tell a genuinely abandoned branch from a name someone reused, and + // keeps the branch instead of deleting it. + headRefOid: pr.head && pr.head.sha, baseRefName: pr.base && pr.base.ref, // A fork head lives in the contributor's repository. Comparing // repo ids (not names) keeps a same-name fork from looking local. @@ -82,7 +86,10 @@ jobs: repo, per_page: 100, }); - const branches = rawBranches.map((branch) => branch.name); + const branches = rawBranches.map((branch) => ({ + name: branch.name, + oid: branch.commit && branch.commit.sha, + })); const protectedByGitHub = new Set( rawBranches.filter((branch) => branch.protected).map((branch) => branch.name), ); diff --git a/docs-site/public/pr-screenshots/2692-git-attribution-row.png b/docs-site/public/pr-screenshots/2692-git-attribution-row.png new file mode 100644 index 0000000000..464f4df1e2 Binary files /dev/null and b/docs-site/public/pr-screenshots/2692-git-attribution-row.png differ diff --git a/gui/src/components/codex-set/PromptLayerRow.tsx b/gui/src/components/codex-set/PromptLayerRow.tsx index eb2fd6c68d..4dd54a8bd0 100644 --- a/gui/src/components/codex-set/PromptLayerRow.tsx +++ b/gui/src/components/codex-set/PromptLayerRow.tsx @@ -1,7 +1,7 @@ import { useT } from "../../i18n/shared"; import { navigateHash } from "../../hash-routing"; import type { LayerDescriptorDto, ToggleStateDto } from "../../pages/codex-set-prompt"; -import { LAYER_LABEL_KEYS } from "./prompt-layer-copy"; +import { LAYER_CONDITION_KEYS, LAYER_LABEL_KEYS } from "./prompt-layer-copy"; /** * One row of the prompt-layer list. @@ -47,6 +47,10 @@ export default function PromptLayerRow({ // An id this build has no copy for is shown verbatim rather than blank: a newer // runtime listing a layer we do not know about is information, not an error. const label = labelKey ? t(labelKey) : descriptor.id; + // A layer that only appears under a condition has one; the rest do not (the map is + // deliberately partial). Read here as well as in the dialog so a conditional row + // never claims to be unconditional. + const conditionKey = LAYER_CONDITION_KEYS[descriptor.id]; const checked = toggle?.defaultedUserValue ?? descriptor.default ?? true; return ( @@ -123,8 +127,19 @@ export default function PromptLayerRow({ "Always on" is false for a transition notice: it is not on, it fires. Reusing the locked label would tell the user this text is in every prompt when it appears only at a change. + + It is equally false for a layer with a CONDITION. `plugins` is emitted + when a plugin is selected or advertises a capability, and + `git-attribution` follows the account's attribution policy - neither is + unconditionally present, and the dialog has always said so while the row + said "Always on". The row now prefers the condition when one exists, so + the two surfaces cannot disagree about the same layer. */} - {transitionOnly ? t("codexSet.row.onChange") : t("codexSet.row.alwaysOn")} + {transitionOnly + ? t("codexSet.row.onChange") + : conditionKey + ? t(conditionKey) + : t("codexSet.row.alwaysOn")} )} diff --git a/gui/src/components/codex-set/prompt-layer-copy.ts b/gui/src/components/codex-set/prompt-layer-copy.ts index e0136770fd..dec1998745 100644 --- a/gui/src/components/codex-set/prompt-layer-copy.ts +++ b/gui/src/components/codex-set/prompt-layer-copy.ts @@ -24,7 +24,8 @@ export type LayerId = | "plugins" | "tools" | "skills" - | "multi-agent-mode"; + | "multi-agent-mode" + | "git-attribution"; /** * Layer id -> i18n key, written out rather than built by string concatenation. @@ -49,6 +50,7 @@ export const LAYER_LABEL_KEYS: Record = { tools: "codexSet.layer.tools", skills: "codexSet.layer.skills", "multi-agent-mode": "codexSet.layer.multi-agent-mode", + "git-attribution": "codexSet.layer.git-attribution", }; export const LAYER_ABOUT_KEYS: Record = { @@ -67,6 +69,7 @@ export const LAYER_ABOUT_KEYS: Record = { tools: "codexSet.about.tools", skills: "codexSet.about.skills", "multi-agent-mode": "codexSet.about.multi-agent-mode", + "git-attribution": "codexSet.about.git-attribution", }; /** @@ -80,6 +83,10 @@ export const LAYER_CONDITION_KEYS: Partial> = { realtime: "codexSet.condition.realtime", "agents-md": "codexSet.condition.agents-md", plugins: "codexSet.condition.plugins", + // Mandatory for this row, not optional: without it the renderer falls through to + // "always on", and that is false - the account can turn attribution off, in which + // case Codex sends the opposite instruction rather than sending nothing. + "git-attribution": "codexSet.condition.git-attribution", }; export const CLASS_LABEL_KEYS: Record = { diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index e3984972dd..8a2852c436 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -959,6 +959,7 @@ export const de: Record = { "codexSet.layer.plugins": "Plugins", "codexSet.layer.tools": "Tools", "codexSet.layer.multi-agent-mode": "Multi-Agenten-Modus", + "codexSet.layer.git-attribution": "Commit-Attribution", "codexSet.about.base-instructions": "Codex-eigene Anweisungen. Sie werden mit der Anfrage selbst gesendet und können nicht deaktiviert werden.", "codexSet.about.model-switch": "Wird hinzugefügt, wenn das Modell während einer Unterhaltung gewechselt wird.", "codexSet.about.personality": "Vorgaben für Ton und Ausdruck, gesteuert durch ein Feature-Flag.", @@ -974,10 +975,12 @@ export const de: Record = { "codexSet.about.tools": "Verzögert geladene Tool-Beschreibungen, gesteuert durch ein Feature-Flag.", "codexSet.about.skills": "Liste der verfügbaren Skills.", "codexSet.about.multi-agent-mode": "Anweisungen für Subagenten, gesteuert durch ein Feature-Flag.", + "codexSet.about.git-attribution": "Lässt das Modell einen Co-authored-by: Codex-Trailer in Commits schreiben, die es erstellt, und eine Zeile Generated with Codex. in Pull Requests, die es öffnet. Codex liest das aus deinem Konto, deshalb ist es weder hier noch unter [features] einstellbar. Ist es im Konto aus, sendet Codex die umgekehrte Anweisung statt gar keine.", "codexSet.condition.model-switch": "Wird nur nach einem Modellwechsel während der Sitzung ausgegeben.", "codexSet.condition.realtime": "Wird nur in einer Echtzeitsitzung ausgegeben.", "codexSet.condition.agents-md": "Wird ausgegeben, wenn für das Arbeitsverzeichnis eine Projektdokumentation gefunden wird.", "codexSet.condition.plugins": "Wird ausgegeben, wenn ein Plugin ausgewählt ist oder ein Plugin eine Funktion bereitstellt.", + "codexSet.condition.git-attribution": "Wird durch die Attributionsrichtlinie deines Kontos bestimmt.", "nav.api": "API", "nav.integrations": "Integrationen", "nav.openMenu": "Menü öffnen", diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 6624ea15da..5daf8085be 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -1453,6 +1453,7 @@ export const en = { "codexSet.layer.plugins": "Plugins", "codexSet.layer.tools": "Tools", "codexSet.layer.multi-agent-mode": "Multi-agent mode", + "codexSet.layer.git-attribution": "Commit attribution", "codexSet.about.base-instructions": "Codex's own instructions. They travel with the request itself and cannot be turned off.", "codexSet.about.model-switch": "Added when the session changes model mid-conversation.", "codexSet.about.personality": "Tone and voice guidance, governed by a feature flag.", @@ -1468,10 +1469,12 @@ export const en = { "codexSet.about.tools": "Deferred tool descriptions, governed by a feature flag.", "codexSet.about.skills": "The list of available skills.", "codexSet.about.multi-agent-mode": "Subagent instructions, governed by a feature flag.", + "codexSet.about.git-attribution": "Tells the model to add a Co-authored-by: Codex trailer to commits it writes, and a Generated with Codex. line to pull requests it opens. Codex resolves this from your account, so there is no setting for it here or under [features]. When your account has it off, Codex sends the opposite instruction rather than sending nothing.", "codexSet.condition.model-switch": "Emitted only after a mid-session model change.", "codexSet.condition.realtime": "Emitted only in a realtime session.", "codexSet.condition.agents-md": "Emitted when a project doc is found for the working directory.", "codexSet.condition.plugins": "Emitted when a plugin is selected or any plugin advertises a capability.", + "codexSet.condition.git-attribution": "Set by your account's attribution policy.", "nav.api": "API", "nav.integrations": "Integrations", "nav.openMenu": "Open menu", diff --git a/gui/src/i18n/fr.ts b/gui/src/i18n/fr.ts index 12678eaee3..e3682aafcc 100644 --- a/gui/src/i18n/fr.ts +++ b/gui/src/i18n/fr.ts @@ -1426,6 +1426,7 @@ export const fr: Record = { "codexSet.layer.plugins": "Plugins", "codexSet.layer.tools": "Outils", "codexSet.layer.multi-agent-mode": "Mode multi-agents", + "codexSet.layer.git-attribution": "Attribution des commits", "codexSet.about.base-instructions": "Instructions propres à Codex. Elles accompagnent la requête elle-même et ne peuvent pas être désactivées.", "codexSet.about.model-switch": "Ajouté lorsque le modèle change en cours de conversation.", "codexSet.about.personality": "Consignes de ton et de style, régies par un indicateur de fonctionnalité.", @@ -1441,10 +1442,12 @@ export const fr: Record = { "codexSet.about.tools": "Descriptions différées des outils, régies par un indicateur de fonctionnalité.", "codexSet.about.skills": "Liste des compétences disponibles.", "codexSet.about.multi-agent-mode": "Instructions pour les sous-agents, régies par un indicateur de fonctionnalité.", + "codexSet.about.git-attribution": "Demande au modèle d’ajouter un trailer Co-authored-by: Codex aux commits qu’il écrit, et une ligne Generated with Codex. aux pull requests qu’il ouvre. Codex lit ce réglage depuis votre compte : il n’est modifiable ni ici ni dans [features]. Si votre compte le désactive, Codex envoie l’instruction inverse au lieu de ne rien envoyer.", "codexSet.condition.model-switch": "Émis uniquement après un changement de modèle en cours de session.", "codexSet.condition.realtime": "Émis uniquement dans une session en temps réel.", "codexSet.condition.agents-md": "Émis lorsqu’un document de projet est trouvé pour le répertoire de travail.", "codexSet.condition.plugins": "Émis lorsqu’un plugin est sélectionné ou qu’un plugin déclare une capacité.", + "codexSet.condition.git-attribution": "Défini par la politique d’attribution de votre compte.", "nav.api": "API", "nav.integrations": "Intégrations", "nav.openMenu": "Ouvrir le menu", diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 0d78de4c8f..4adb6f2941 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -1386,6 +1386,7 @@ export const ja: Record = { "codexSet.layer.plugins": "プラグイン", "codexSet.layer.tools": "ツール", "codexSet.layer.multi-agent-mode": "マルチエージェントモード", + "codexSet.layer.git-attribution": "コミットの帰属表示", "codexSet.about.base-instructions": "Codex 自体の指示です。リクエストに含まれ、無効にはできません。", "codexSet.about.model-switch": "会話の途中でセッションのモデルが変わると追加されます。", "codexSet.about.personality": "機能フラグで制御されるトーンと語調のガイダンスです。", @@ -1401,10 +1402,12 @@ export const ja: Record = { "codexSet.about.tools": "機能フラグで制御される遅延読み込みツールの説明です。", "codexSet.about.skills": "利用可能なスキルの一覧です。", "codexSet.about.multi-agent-mode": "機能フラグで制御されるサブエージェント向けの指示です。", + "codexSet.about.git-attribution": "モデルが書いたコミットに Co-authored-by: Codex トレーラーを、開いたプルリクエストに Generated with Codex. の行を追加させます。Codex がアカウントから取得するため、ここでも [features] でも変更できません。アカウント側で無効にすると、何も送らないのではなく逆の指示を送ります。", "codexSet.condition.model-switch": "セッション中にモデルが変わった後のみ含まれます。", "codexSet.condition.realtime": "リアルタイムセッションのみ含まれます。", "codexSet.condition.agents-md": "作業ディレクトリ用のプロジェクト文書が見つかると含まれます。", "codexSet.condition.plugins": "プラグインが選択されているか、プラグインが機能を提供すると含まれます。", + "codexSet.condition.git-attribution": "アカウントの帰属表示ポリシーで決まります。", "nav.api": "API", "nav.integrations": "連携", "nav.openMenu": "メニューを開く", diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index a4fd1c92db..af59d75dfd 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -985,6 +985,7 @@ export const ko: Record = { "codexSet.layer.plugins": "플러그인", "codexSet.layer.tools": "도구", "codexSet.layer.multi-agent-mode": "멀티 에이전트 모드", + "codexSet.layer.git-attribution": "커밋 어트리뷰션", "codexSet.about.base-instructions": "Codex 자체 지침입니다. 요청에 포함되며 끌 수 없습니다.", "codexSet.about.model-switch": "대화 도중 세션 모델이 바뀌면 추가됩니다.", "codexSet.about.personality": "기능 플래그로 제어되는 어조와 말투 지침입니다.", @@ -1000,10 +1001,12 @@ export const ko: Record = { "codexSet.about.tools": "기능 플래그로 제어되는 지연 로드 도구 설명입니다.", "codexSet.about.skills": "사용 가능한 스킬 목록입니다.", "codexSet.about.multi-agent-mode": "기능 플래그로 제어되는 서브에이전트 지침입니다.", + "codexSet.about.git-attribution": "모델이 작성한 커밋에 Co-authored-by: Codex 트레일러를, 새로 여는 풀 리퀘스트에 Generated with Codex. 한 줄을 붙이게 합니다. Codex가 계정에서 이 값을 가져오기 때문에 여기서도 [features]에서도 바꿀 수 없습니다. 계정에서 꺼두면 아무것도 보내지 않는 대신 반대 지시를 보냅니다.", "codexSet.condition.model-switch": "세션 도중 모델이 바뀐 뒤에만 포함됩니다.", "codexSet.condition.realtime": "실시간 세션에만 포함됩니다.", "codexSet.condition.agents-md": "작업 디렉터리에서 프로젝트 문서를 찾으면 포함됩니다.", "codexSet.condition.plugins": "플러그인을 선택했거나 플러그인이 기능을 제공하면 포함됩니다.", + "codexSet.condition.git-attribution": "계정의 어트리뷰션 정책이 결정합니다.", "nav.openMenu": "메뉴 열기", "nav.closeMenu": "메뉴 닫기", "integrations.subtitle": "클라이언트를 opencodex에 연결하고 자격 증명과 설정 복원을 관리합니다.", diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index 61eddd3aec..8868552928 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -1437,6 +1437,7 @@ export const ru: Record = { "codexSet.layer.plugins": "Плагины", "codexSet.layer.tools": "Инструменты", "codexSet.layer.multi-agent-mode": "Мультиагентный режим", + "codexSet.layer.git-attribution": "Атрибуция коммитов", "codexSet.about.base-instructions": "Собственные инструкции Codex. Они передаются вместе с запросом, и отключить их нельзя.", "codexSet.about.model-switch": "Добавляется при смене модели во время диалога.", "codexSet.about.personality": "Указания по тону и стилю, управляемые флагом функции.", @@ -1452,10 +1453,12 @@ export const ru: Record = { "codexSet.about.tools": "Описания отложенных инструментов, управляемые флагом функции.", "codexSet.about.skills": "Список доступных навыков.", "codexSet.about.multi-agent-mode": "Инструкции для подагентов, управляемые флагом функции.", + "codexSet.about.git-attribution": "Просит модель добавлять трейлер Co-authored-by: Codex в коммиты, которые она пишет, и строку Generated with Codex. в пул-реквесты, которые она открывает. Codex берёт это из вашей учётной записи, поэтому настройки нет ни здесь, ни в [features]. Если в учётной записи атрибуция отключена, Codex отправляет обратную инструкцию, а не молчит.", "codexSet.condition.model-switch": "Добавляется только после смены модели во время сеанса.", "codexSet.condition.realtime": "Добавляется только в сеансе реального времени.", "codexSet.condition.agents-md": "Добавляется, если для рабочего каталога найден документ проекта.", "codexSet.condition.plugins": "Добавляется, если выбран плагин или какой-либо плагин объявляет возможность.", + "codexSet.condition.git-attribution": "Определяется политикой атрибуции вашей учётной записи.", "nav.api": "API", "nav.integrations": "Интеграции", "nav.openMenu": "Открыть меню", diff --git a/gui/src/i18n/tr.ts b/gui/src/i18n/tr.ts index d1bb12bf00..e289716c8c 100644 --- a/gui/src/i18n/tr.ts +++ b/gui/src/i18n/tr.ts @@ -1444,6 +1444,7 @@ export const tr: Record = { "codexSet.layer.plugins": "Eklentiler", "codexSet.layer.tools": "Araçlar", "codexSet.layer.multi-agent-mode": "Çoklu ajan modu", + "codexSet.layer.git-attribution": "Commit atıfları", "codexSet.about.base-instructions": "Codex'in kendi talimatlarıdır. İstekle birlikte gönderilir ve kapatılamaz.", "codexSet.about.model-switch": "Oturum sırasında model değiştiğinde eklenir.", "codexSet.about.personality": "Bir özellik bayrağının yönettiği ton ve anlatım yönlendirmesi.", @@ -1459,10 +1460,12 @@ export const tr: Record = { "codexSet.about.tools": "Bir özellik bayrağının yönettiği ertelenmiş araç açıklamaları.", "codexSet.about.skills": "Kullanılabilir becerilerin listesi.", "codexSet.about.multi-agent-mode": "Bir özellik bayrağının yönettiği alt ajan talimatları.", + "codexSet.about.git-attribution": "Modelin yazdığı commit’lere Co-authored-by: Codex trailer’ını, açtığı pull request’lere de Generated with Codex. satırını eklemesini söyler. Codex bunu hesabınızdan okur; ne burada ne de [features] altında değiştirilebilir. Hesabınız kapattığında Codex hiçbir şey göndermek yerine tersi yönde talimat gönderir.", "codexSet.condition.model-switch": "Yalnızca oturum sırasında model değiştikten sonra eklenir.", "codexSet.condition.realtime": "Yalnızca gerçek zamanlı oturumlarda eklenir.", "codexSet.condition.agents-md": "Çalışma dizini için bir proje belgesi bulunduğunda eklenir.", "codexSet.condition.plugins": "Bir eklenti seçildiğinde veya herhangi bir eklenti bir yetenek bildirdiğinde eklenir.", + "codexSet.condition.git-attribution": "Hesabınızın atıf politikası belirler.", "nav.api": "API", "nav.integrations": "Entegrasyonlar", "nav.openMenu": "Menüyü aç", diff --git a/gui/src/i18n/zh-TW.ts b/gui/src/i18n/zh-TW.ts index 27118c4eb1..ed80ea0c83 100644 --- a/gui/src/i18n/zh-TW.ts +++ b/gui/src/i18n/zh-TW.ts @@ -1219,6 +1219,7 @@ export const zhTW: Record = { "codexSet.layer.plugins": "外掛程式", "codexSet.layer.tools": "工具", "codexSet.layer.multi-agent-mode": "多代理模式", + "codexSet.layer.git-attribution": "提交署名", "codexSet.about.base-instructions": "Codex 自身的指令。它們會隨請求一同傳送,無法關閉。", "codexSet.about.model-switch": "工作階段中途切換模型時新增。", "codexSet.about.personality": "語氣和表達風格指引,由功能開關控制。", @@ -1234,10 +1235,12 @@ export const zhTW: Record = { "codexSet.about.tools": "延後載入的工具說明,由功能開關控制。", "codexSet.about.skills": "可用技能清單。", "codexSet.about.multi-agent-mode": "子代理指令,由功能開關控制。", + "codexSet.about.git-attribution": "讓模型在它寫的提交加上 Co-authored-by: Codex 尾註,並在它開的拉取請求加上 Generated with Codex. 這一行。Codex 從你的帳號讀取此項,所以這裡和 [features] 都改不了。帳號關閉時,Codex 會送出相反的指令,而不是什麼都不送。", "codexSet.condition.model-switch": "僅在工作階段中途切換模型後插入。", "codexSet.condition.realtime": "僅在即時工作階段中插入。", "codexSet.condition.agents-md": "找到適用於目前工作目錄的專案文件時插入。", "codexSet.condition.plugins": "選取外掛程式或任一外掛程式宣告功能時插入。", + "codexSet.condition.git-attribution": "由你帳號的署名政策決定。", "nav.api": "API", "nav.openMenu": "開啟選單", "nav.closeMenu": "關閉選單", diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index 98567b595d..fa3e98d23a 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -978,6 +978,7 @@ export const zh: Record = { "codexSet.layer.plugins": "插件", "codexSet.layer.tools": "工具", "codexSet.layer.multi-agent-mode": "多代理模式", + "codexSet.layer.git-attribution": "提交署名", "codexSet.about.base-instructions": "Codex 自身的指令。它们随请求一同发送,无法关闭。", "codexSet.about.model-switch": "会话中途切换模型时添加。", "codexSet.about.personality": "语气和表达风格指引,由功能开关控制。", @@ -993,10 +994,12 @@ export const zh: Record = { "codexSet.about.tools": "延迟加载的工具说明,由功能开关控制。", "codexSet.about.skills": "可用技能列表。", "codexSet.about.multi-agent-mode": "子代理指令,由功能开关控制。", + "codexSet.about.git-attribution": "让模型在它写的提交里加上 Co-authored-by: Codex 尾注,并在它开的拉取请求里加上 Generated with Codex. 这一行。Codex 从你的账号读取此项,所以这里和 [features] 都改不了。账号关闭时,Codex 会发送相反的指令,而不是什么都不发。", "codexSet.condition.model-switch": "仅在会话中途切换模型后注入。", "codexSet.condition.realtime": "仅在实时会话中注入。", "codexSet.condition.agents-md": "找到适用于当前工作目录的项目文档时注入。", "codexSet.condition.plugins": "选中插件或任一插件声明功能时注入。", + "codexSet.condition.git-attribution": "由你账号的署名策略决定。", "nav.openMenu": "打开菜单", "nav.closeMenu": "关闭菜单", "integrations.subtitle": "将客户端连接到 opencodex,管理凭据并恢复客户端配置。", diff --git a/gui/src/pages/codex-set-prompt.tsx b/gui/src/pages/codex-set-prompt.tsx index d282d66fc0..4082b0f247 100644 --- a/gui/src/pages/codex-set-prompt.tsx +++ b/gui/src/pages/codex-set-prompt.tsx @@ -319,9 +319,15 @@ export default function CodexSetPrompt({ apiBase }: { apiBase: string }) { }; // Assembly order, so the list reads the way the prompt is actually built. // Every class renders; the row decides what each one gets. + // + // A null order means the position is registration-order dependent rather than fixed + // in world_state.rs - an extension-contributed section. Those sort AFTER every known + // position instead of collapsing to 0, which would have put them at the very top and + // claimed they are assembled first. The stable fallback keeps two such layers in + // inventory order relative to each other rather than swapping unpredictably. const rows = [...(snapshot?.inventory ?? [])] .filter(d => d.class !== "extension-unknown") - .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); + .sort((a, b) => (a.order ?? Number.MAX_SAFE_INTEGER) - (b.order ?? Number.MAX_SAFE_INTEGER)); /** * Two kinds of layer, split by what they ARE rather than by a scope flag. diff --git a/gui/tests/codex-set-prompt-layers.test.tsx b/gui/tests/codex-set-prompt-layers.test.tsx index b110a19c26..db1cb9ecb3 100644 --- a/gui/tests/codex-set-prompt-layers.test.tsx +++ b/gui/tests/codex-set-prompt-layers.test.tsx @@ -359,3 +359,82 @@ test("the layer-text rule outranks .api-code so long bodies wrap", () => { expect(rule![1]).toContain("overflow-wrap: anywhere"); expect(rule![1]).toContain("overflow-x: hidden"); }); + +/** + * The layer `ext/git-attribution` contributes. + * + * Three things have to be true at once, and each would be wrong on its own: it appears + * at all, it has NO switch, and it does not claim to be "always on". The last is the + * one a reader is most likely to get wrong - the account can turn attribution off, and + * when it does Codex sends the opposite instruction rather than sending nothing, so + * neither "always on" nor "sometimes absent" describes it. + */ +test("git-attribution renders as a conditional row with no switch", async () => { + stubRoutes(() => json(snapshot())); + const { container, root } = await mount(); + + const el = row(container, "git-attribution"); + expect(el).not.toBeNull(); + // No switch anywhere in the row: not a disabled one either, which would claim a + // capability Codex does not expose. + expect(el!.querySelector("[role=\"switch\"]")).toBeNull(); + // The locked note, not the feature-gated one - there is no [features] key to link to. + expect(el!.querySelector(".codex-set-prompt__note--locked")).not.toBeNull(); + // No [features] link INSIDE the note. Querying `.link-btn` across the whole row would + // always find one: the row's own name is a link-btn button that opens the dialog. + expect(el!.querySelector(".codex-set-prompt__note .link-btn")).toBeNull(); + // No key chip: the descriptor carries key: null because enablement is account-derived. + expect(el!.querySelector(".codex-set-prompt__key")).toBeNull(); + + // A registration-order layer sorts after every fixed position rather than to the top, + // and shows the neutral marker instead of inventing a number. + // + // Scoped to the STATE list: the transition notices render in their own list below, so + // "last in the document" would be a claim about the split rather than about ordering. + const stateList = container.querySelectorAll(".codex-set-prompt__rows")[0]!; + const stateIds = [...stateList.querySelectorAll("[data-layer-id]")].map(n => n.getAttribute("data-layer-id")); + expect(stateIds[stateIds.length - 1]).toBe("git-attribution"); + // And specifically NOT first, which is where a null order collapsing to 0 would put it. + expect(stateIds[0]).toBe("base-instructions"); + expect(el!.querySelector(".codex-set-prompt__pos")!.textContent).toBe("\u00b7"); + + // The dialog states the real condition. + await act(async () => { + (el!.querySelector("button") as HTMLButtonElement).click(); + }); + const dialog = document.querySelector("dialog.modal-overlay")!; + expect(dialog.textContent ?? "").toContain("attribution policy"); + await act(async () => { root.unmount(); }); +}); + +/** + * A conditional row must not claim to be unconditional. + * + * Caught by rendering the real page in a browser rather than by a unit test: the DOM + * showed `git-attribution` and `plugins` both labelled "Always on" while their dialogs + * described a condition. The condition map existed and only the dialog read it, so the + * two surfaces disagreed about the same layer. + * + * Table-driven over every layer that HAS a condition, so the next one added is covered + * without a new test - and the negative half proves the assertion is not vacuous. + */ +test("a row with a condition shows it instead of \"Always on\"", async () => { + stubRoutes(() => json(snapshot())); + const { container, root } = await mount(); + + // Conditions live on runtime-conditional layers; the transition notices are excluded + // because "it fires on a change" is their own distinct wording. + const conditional = ["plugins", "agents-md", "git-attribution"]; + for (const id of conditional) { + const note = row(container, id)!.querySelector(".codex-set-prompt__note--locked")!; + expect(note.textContent, id).not.toBe("Always on"); + expect((note.textContent ?? "").length, id).toBeGreaterThan(0); + } + + // base-instructions genuinely IS always on: no condition, no off-switch anywhere. If + // this drifted the test above would pass for the wrong reason. + const base = row(container, "base-instructions")!.querySelector(".codex-set-prompt__note--locked")!; + expect(base.textContent).toBe("Always on"); + + await act(async () => { root.unmount(); }); +}); diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index cc8dcaad89..417f4ede4e 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1041,6 +1041,48 @@ function unionRequired(target: unknown, sibling: unknown): unknown { */ const MOONSHOT_DATA_VALUED_KEYWORDS = new Set(["enum", "const", "default", "examples"]); +/** + * Numeric assertions whose intersection is a bound, and which direction tightens. + * + * `$ref` under 2020-12 is an in-place applicator: the node and its target BOTH apply, so + * the emitted schema must be their INTERSECTION. The previous code overwrote the target + * with the node and called that "the narrower reading", which holds only when the node + * happens to be narrower. A node declaring `minLength: 1` beside a target declaring + * `minLength: 5` shipped `minLength: 1` - a contract weaker than either side asked for, + * emitted silently, which is the same failure mode the `required` composition fixed for + * set-valued keywords. + * + * "max" means the surviving value is the larger of the two (lower bounds), "min" the + * smaller (upper bounds). A keyword absent from this table keeps the overwrite: for + * `type`, `format`, `description` and friends there is no ordering to intersect along, + * and the node is the more specific statement. + */ +const MOONSHOT_BOUND_KEYWORDS: Record = { + minLength: "max", + minItems: "max", + minProperties: "max", + minimum: "max", + exclusiveMinimum: "max", + maxLength: "min", + maxItems: "min", + maxProperties: "min", + maximum: "min", + exclusiveMaximum: "min", +}; + +/** + * Intersect one numeric bound. Either side being absent or non-finite yields the other, + * because an unstated bound constrains nothing - returning `undefined` there would drop + * a constraint the remaining side genuinely made. + */ +function intersectBound(target: unknown, sibling: unknown, direction: "max" | "min"): unknown { + const a = typeof target === "number" && Number.isFinite(target) ? target : null; + const b = typeof sibling === "number" && Number.isFinite(sibling) ? sibling : null; + if (a === null) return b === null ? sibling : sibling; + if (b === null) return target; + return direction === "max" ? Math.max(a, b) : Math.min(a, b); +} + /** * Compose two `properties` maps. A property named in BOTH the referenced target and the * node is the same conjunction problem `required` had: letting the sibling win discards @@ -1130,6 +1172,14 @@ function normalizeMoonshotSchemaNode( merged[key] = composeProperties(merged[key] as Record, normalized); continue; } + // Numeric bounds intersect rather than overwrite: both the node and its target + // apply, so the surviving bound is the stricter of the two in whichever direction + // that keyword tightens. + const boundDirection = MOONSHOT_BOUND_KEYWORDS[key]; + if (boundDirection && key in merged) { + merged[key] = intersectBound(merged[key], normalized, boundDirection); + continue; + } merged[key] = normalized; } return merged; diff --git a/src/codex/prompt-layers.ts b/src/codex/prompt-layers.ts index 301cb60b95..f33cc96426 100644 --- a/src/codex/prompt-layers.ts +++ b/src/codex/prompt-layers.ts @@ -100,6 +100,24 @@ export const LAYER_INVENTORY: readonly LayerDescriptor[] = Object.freeze([ { id: "tools", class: "feature-gated", key: "features.deferred_tool_world_state", default: false, order: 12 }, { id: "skills", class: "config-toggle", key: "skills.include_instructions", default: true, order: 13 }, { id: "multi-agent-mode", class: "feature-gated", key: "features.multi_agent_v2.enabled", default: false, order: 14 }, + /** + * Commit and pull-request attribution, contributed by `ext/git-attribution` rather + * than by a world_state.rs section — which is why it is absent from the order list + * above and carries `order: null`: it registers through + * `extensions.context_contributors()` (`core/src/session/world_state.rs:64-66`), + * whose position is registration-order dependent. + * + * `runtime-conditional`, NOT feature-gated. `ext/git-attribution/src/lib.rs:33-80` + * resolves enablement from the AUTH SERVER via `resolve_attribution_policy`, caches + * it on the thread store, and falls back to disabled when the lookup fails. + * `features/src/lib.rs:277` records the old config flag as removed, so there is no + * key for this GUI to write and nothing in [features] to point a user at. + * + * Both states emit text: enabled sends the `Co-authored-by: Codex` trailer plus the + * `Generated with Codex.` PR marker, disabled sends an explicit countermand. So the + * row's condition line must name the policy rather than claiming "always on". + */ + { id: "git-attribution", class: "runtime-conditional", key: null, default: null, order: null }, ] as const); /** diff --git a/src/codex/prompt-text-probe.ts b/src/codex/prompt-text-probe.ts index 99535a1b6b..6db8d1e49e 100644 --- a/src/codex/prompt-text-probe.ts +++ b/src/codex/prompt-text-probe.ts @@ -60,6 +60,13 @@ const UNMAPPED_LAYER_IDS = [ "personality", "realtime", "collaboration", + // The Rust source names a marker pair, but a world-state section is + // DIFF-rendered: it emits nothing on a turn where its state has not changed. Live + // `codex debug prompt-input` (codex-cli 0.145.0, 32978 bytes) showed no such block and + // no attribution text. Listing the id here reports "not exposed" honestly instead of + // claiming a tag this extractor has never actually matched - the same mistake the + // header above records for permissions. + "git-attribution", ] as const; export interface LayerText { diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 40feec0bfd..d15814d830 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -340,9 +340,31 @@ const ANTHROPIC_MODEL_CONTEXT_WINDOWS: Record = { "claude-sonnet // The non-Z.AI providers below are speculative on purpose: they carry 5.2 today and are // expected to pick 5.3 up on their usual lag. Providers whose live /v1/models discovery is // enabled self-correct on the next successful fetch; static ones need a follow-up refresh. -const ZAI_GLM_53_MODELS = ["glm-5.3", "glm-5.3[1m]"]; +// Every 5.3 family member, so the effort ladder, the default effort and the output +// cap are derived in ONE place. `glm-5.3-flash` was seeded into the model list and +// the context map by hand and left out of this constant, which meant it advertised +// a 1M context with a null effort ladder, no default effort and no output cap while +// its siblings carried three tiers, a `max` default and 131072 tokens. A member +// added to the list but not to the family is a model whose metadata silently +// disappears. +const ZAI_GLM_53_MODELS = ["glm-5.3", "glm-5.3[1m]", "glm-5.3-flash"]; const ZAI_GLM_52_MODELS = ["glm-5.2", "glm-5.2[1m]"]; const ZAI_GLM_5X_MODELS = [...ZAI_GLM_53_MODELS, ...ZAI_GLM_52_MODELS]; +/** + * The 5.x rows whose images the PROXY has to describe, which is NOT the same set as + * the 5.x rows themselves. + * + * `glm-5.3-flash` is a native VLM (docs.z.ai/guides/vlm/glm-5.3-flash), so listing it + * in `noVisionModels` sent an image through the vision sidecar and handed the model a + * text description of a picture it could have read itself - no error, worse answer, + * extra call. The correction commit fixed the Alibaba entries and left the eight + * providers that reach this constant behind. + * + * Kept separate from ZAI_GLM_5X_MODELS rather than filtered at each use site: that + * constant also drives `modelSupportsReasoningSummaries` and + * `preserveReasoningContentModels`, where flash DOES belong. + */ +const ZAI_GLM_5X_SIDECAR_VISION_MODELS = ZAI_GLM_5X_MODELS.filter(id => id !== "glm-5.3-flash"); const ZAI_GLM_52_REASONING_EFFORTS = ["low", "medium", "high", "xhigh", "max"]; /** * GLM-5.3 does NOT share 5.2's five-tier ladder. docs.z.ai/devpack/latest-model folds every @@ -471,7 +493,9 @@ const OPENCODE_GO_THINKING_TOGGLE_MODELS = [ * images through the proxy's vision sidecar (src/codex/catalog/provider-fetch.ts), a claim nobody * has verified for BigModel-hosted GLM. */ -const ZHIPU_BIGMODEL_TEXT_MODELS = ["glm-4.6", "glm-4.7", "glm-4.7-flash", "glm-5", "glm-5.1", "glm-5.2", "glm-5.3", "glm-5.3-flash"]; +// `glm-5.3-flash` is deliberately absent: it is a native VLM +// (docs.z.ai/guides/vlm/glm-5.3-flash), unlike glm-5.3 itself. +const ZHIPU_BIGMODEL_TEXT_MODELS = ["glm-4.6", "glm-4.7", "glm-4.7-flash", "glm-5", "glm-5.1", "glm-5.2", "glm-5.3"]; const ZHIPU_BIGMODEL_MODELS = [...ZHIPU_BIGMODEL_TEXT_MODELS, "glm-4.6v"]; const ZHIPU_BIGMODEL_INPUT_MODALITIES: Record = { ...Object.fromEntries(ZHIPU_BIGMODEL_TEXT_MODELS.map(id => [id, ["text"]])), @@ -694,6 +718,9 @@ const VOLCENGINE_AGENT_PLAN_MODELS = [ const VOLCENGINE_PLAN_INPUT_MODALITIES: Record = { "kimi-k2.6": ["text", "image"], "minimax-m3": ["text", "image"], + // Native VLM (docs.z.ai/guides/vlm/glm-5.3-flash), so it is declared here and left + // out of the text-only list below. + "glm-5.3-flash": ["text", "image"], }; // Every other Plan model is text-only. Declaring this explicitly keeps the vision // sidecar from advertising image input for models that cannot accept it — the same @@ -704,7 +731,6 @@ const VOLCENGINE_PLAN_TEXT_ONLY_MODELS = [ "deepseek-v4-pro", "deepseek-v4-flash", "glm-5.3", - "glm-5.3-flash", "glm-5.2", "doubao-seed-2.0-pro", ]; @@ -812,6 +838,7 @@ const NVIDIA_NIM_VISION_MODELS = [ "minimaxai/minimax-m3", "moonshotai/kimi-k2.6", "moonshotai/kimi-k2.5", "stepfun-ai/step-3.7-flash", "thinkingmachines/inkling", "mistralai/mistral-medium-3.5-128b", + "z-ai/glm-5.3-flash", ]; /** * The catalog advertises image input only for `noVisionModels` members, so a natively @@ -845,7 +872,11 @@ const NVIDIA_NIM_NO_VISION_MODELS = [ "nvidia/nemotron-3-ultra-550b-a55b", "nvidia/nemotron-mini-4b-instruct", "nvidia/nvidia-nemotron-nano-9b-v2", "openai/gpt-oss-120b", "openai/gpt-oss-20b", - "poolside/laguna-xs-2.1", "z-ai/glm-5.3", "z-ai/glm-5.3-flash", "z-ai/glm-5.2", + // z-ai/glm-5.3-flash belongs in NVIDIA_NIM_VISION_MODELS, not here: Z.AI documents + // it under docs.z.ai/guides/vlm/. The header above says an id must be classified + // deliberately rather than assumed from its name, and inheriting glm-5.3's + // text-only verdict because of the shared prefix is exactly that mistake. + "poolside/laguna-xs-2.1", "z-ai/glm-5.3", "z-ai/glm-5.2", ]; const KIMI_CODING_MODEL_CONTEXT_WINDOWS: Record = Object.fromEntries( KIMI_CODING_MODELS.map(id => [id, id === "k3[1m]" ? KIMI_K3_1M_CONTEXT_WINDOW : KIMI_K3_STANDARD_CONTEXT_WINDOW]), @@ -976,7 +1007,11 @@ const UMANS_GLM_REASONING_EFFORTS = ["high", "xhigh", "max"]; // 260814: Z.AI folds GLM-5.3 efforts into low/high/max, so `low` is a real tier here and // `xhigh` is not distinct from `max` (docs.z.ai/devpack/latest-model). const UMANS_GLM_53_REASONING_EFFORTS = ["low", "high", "max"]; -const UMANS_TEXT_ONLY_MODELS = ["umans-glm-5.3", "umans-glm-5.3-flash", "umans-glm-5.2", "umans-glm-5.1"]; +// `umans-glm-5.3-flash` is NOT here: Z.AI documents glm-5.3-flash under +// docs.z.ai/guides/vlm/, so it takes images natively and does not need the proxy's +// vision sidecar. The seeding pass classified it from the family name and a later +// pass corrected only some of the providers; this is one it missed. +const UMANS_TEXT_ONLY_MODELS = ["umans-glm-5.3", "umans-glm-5.2", "umans-glm-5.1"]; const UMANS_MODEL_CONTEXT_WINDOWS: Record = { "umans-coder": 262_144, "umans-kimi-k2.7": 262_144, @@ -1030,6 +1065,11 @@ const CLINE_PASS_IMAGE_MODELS = new Set([ "cline-pass/mimo-v2.5", "cline-pass/minimax-m3", "cline-pass/qwen3.7-plus", + // Native VLM (docs.z.ai/guides/vlm/), so its images do not go through the proxy's + // sidecar. Adding it here moves it out of CLINE_PASS_TEXT_ONLY_MODELS and flips its + // declared modalities to ["text", "image"] in one edit, because both are derived + // from this set. + "cline-pass/glm-5.3-flash", ]); const CLINE_PASS_MODALITY_KNOWN_MODELS = CLINE_PASS_MODELS.filter(id => id !== "cline-pass/qwen3.8-max"); const CLINE_PASS_TEXT_ONLY_MODELS = CLINE_PASS_MODALITY_KNOWN_MODELS.filter(id => !CLINE_PASS_IMAGE_MODELS.has(id)); @@ -2199,7 +2239,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ modelContextWindows: { "glm-5.3": 1_000_000, "glm-5.3[1m]": 1_000_000, "glm-5.3-flash": 1_000_000, "glm-5.2": 1_000_000, "glm-5.2[1m]": 1_000_000 }, // Z.AI's OpenAI path returns 400 code 1211 for bracketed model ids. modelSuffixBracketStrip: true, - noVisionModels: ZAI_GLM_5X_MODELS, + noVisionModels: ZAI_GLM_5X_SIDECAR_VISION_MODELS, modelReasoningEfforts: ZAI_GLM_5X_REASONING_EFFORTS, modelDefaultReasoningEfforts: Object.fromEntries(ZAI_GLM_53_MODELS.map(id => [id, "max"])), modelMaxOutputTokens: Object.fromEntries(ZAI_GLM_53_MODELS.map(id => [id, 131_072])), @@ -2280,7 +2320,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ jawcodeBundle: "zai", modelContextWindows: { "glm-5.3": 1_000_000, "glm-5.3[1m]": 1_000_000, "glm-5.3-flash": 1_000_000, "glm-5.2": 1_000_000, "glm-5.2[1m]": 1_000_000 }, modelSuffixBracketStrip: true, - noVisionModels: ZAI_GLM_5X_MODELS, + noVisionModels: ZAI_GLM_5X_SIDECAR_VISION_MODELS, modelReasoningEfforts: ZAI_GLM_5X_REASONING_EFFORTS, modelSupportsReasoningSummaries: Object.fromEntries(ZAI_GLM_5X_MODELS.map(id => [id, true])), preserveReasoningContentModels: ZAI_GLM_5X_MODELS, @@ -2513,7 +2553,9 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ models: ["glm-5.3", "glm-5.3-flash", "glm-5.2", "deepseek-v4-pro", "qwen3-coder:480b", "gpt-oss:120b", "kimi-k2.6", "minimax-m3", "qwen3.5:397b", "gemma4:31b"], defaultModel: "glm-5.3", noVisionModels: [ - "glm-5.3", "glm-5.3-flash", "glm-5.2", "glm-5.1", "glm-5", "glm-4.7", + // glm-5.3-flash is absent on purpose: native VLM + // (docs.z.ai/guides/vlm/glm-5.3-flash), so its images skip the sidecar. + "glm-5.3", "glm-5.2", "glm-5.1", "glm-5", "glm-4.7", "minimax-m2.7", "minimax-m2.5", "minimax-m2.1", "nemotron-3-ultra", "nemotron-3-super", "deepseek-v4-pro", "deepseek-v4-flash", diff --git a/tests/cline-pass-provider.test.ts b/tests/cline-pass-provider.test.ts index 94072bc60a..c5e80cbecf 100644 --- a/tests/cline-pass-provider.test.ts +++ b/tests/cline-pass-provider.test.ts @@ -72,7 +72,6 @@ describe("ClinePass provider", () => { expect(entry?.modelMaxInputTokens).toBeUndefined(); expect(entry?.noVisionModels).toEqual([ "cline-pass/glm-5.3", - "cline-pass/glm-5.3-flash", "cline-pass/glm-5.2", "cline-pass/deepseek-v4-pro", "cline-pass/deepseek-v4-flash", diff --git a/tests/closed-pr-branch-cleanup.test.ts b/tests/closed-pr-branch-cleanup.test.ts new file mode 100644 index 0000000000..b18fdd7016 --- /dev/null +++ b/tests/closed-pr-branch-cleanup.test.ts @@ -0,0 +1,170 @@ +/** + * Deletion planning for .github/scripts/closed-pr-branch-cleanup.cjs. + * + * This job deletes branches, so every test here is a safety test. It had no + * coverage at all, which is how a name-only match reached main: the planner + * selected any branch whose same-NAME historical pull requests were all closed, + * without checking that the branch still pointed at one of their head commits. + * A `codex/`-style name reused for new work inherited the closed history of + * every PR that had ever carried that label. + */ +import { describe, expect, test } from "bun:test"; + +/** + * Dynamic import rather than `require`: the repo's other CommonJS-helper tests reach for + * `await import(...)` (ci-workflows.test.ts:5030), and a `no-require-imports` suppression + * here would be a new lint suppression for a problem that has a supported spelling. + */ +interface CleanupModule { + DEFAULT_GRACE_DAYS: number; + KEEP_REASONS: Record; + PROTECTED_BRANCHES: string[]; + isProtectedBranch: (name: string) => boolean; + planClosedPrBranchDeletions: (input: { + pullRequests?: unknown[]; + branches?: unknown[]; + now?: number; + graceDays?: number; + }) => { deletions: { branch: string; pullRequests: number[] }[]; keeps: { branch: string; reason: string }[] }; +} + +const cleanup = await import("../.github/scripts/closed-pr-branch-cleanup.cjs") as unknown as CleanupModule & { default?: CleanupModule }; +// A .cjs module reached through ESM interop may arrive under `default`; taking whichever +// carries the planner keeps the test honest about what it is calling. +const api: CleanupModule = typeof cleanup.planClosedPrBranchDeletions === "function" + ? cleanup + : cleanup.default!; +const { KEEP_REASONS, planClosedPrBranchDeletions } = api; + +const NOW = Date.parse("2026-08-27T00:00:00Z"); +const LONG_AGO = new Date(NOW - 90 * 24 * 60 * 60 * 1000).toISOString(); +const OLD_TIP = "a".repeat(40); +const NEW_TIP = "b".repeat(40); + +function closedPr(over: Record = {}) { + return { + number: 42, + state: "CLOSED", + merged: false, + closedAt: LONG_AGO, + headRefName: "codex/some-work", + headRefOid: OLD_TIP, + baseRefName: "dev", + isCrossRepository: false, + ...over, + }; +} + +function plan(pullRequests: unknown[], branches: unknown[]) { + return planClosedPrBranchDeletions({ pullRequests, branches, now: NOW, graceDays: 14 }); +} + +function keepReason(result: ReturnType, branch: string): string | undefined { + return result.keeps.find(k => k.branch === branch)?.reason; +} + +describe("closed-PR branch cleanup planning", () => { + test("an abandoned branch still at the closed PR tip is deleted", () => { + // The case the job exists for. If this stops passing the job has become + // a no-op, which is a different failure from deleting live work but still + // a failure. + const result = plan( + [closedPr()], + [{ name: "codex/some-work", oid: OLD_TIP }], + ); + expect(result.deletions).toEqual([{ branch: "codex/some-work", pullRequests: [42] }]); + }); + + test("BUG-R4: a branch reused for new work is kept, not deleted", () => { + // Same NAME, different tip. Before the SHA guard this returned a deletion + // for a branch carrying commits that had never been in any pull request. + const result = plan( + [closedPr()], + [{ name: "codex/some-work", oid: NEW_TIP }], + ); + expect(result.deletions).toEqual([]); + expect(keepReason(result, "codex/some-work")).toBe(KEEP_REASONS.MOVED_SINCE_CLOSE); + }); + + test("a tip matching ANY of several closed PRs is enough", () => { + // Reopening and reclosing a branch, or two PRs from the same head, must not + // make the branch undeletable forever - matching one closed head is the bar. + const result = plan( + [ + closedPr({ number: 7, headRefOid: OLD_TIP }), + closedPr({ number: 9, headRefOid: NEW_TIP }), + ], + [{ name: "codex/some-work", oid: NEW_TIP }], + ); + expect(result.deletions).toEqual([{ branch: "codex/some-work", pullRequests: [7, 9] }]); + }); + + test("an unknown current tip is kept", () => { + // A bare string carries no tip. An older caller passing names gets the + // conservative answer rather than the old destructive one. + const result = plan([closedPr()], ["codex/some-work"]); + expect(result.deletions).toEqual([]); + expect(keepReason(result, "codex/some-work")).toBe(KEEP_REASONS.UNKNOWN_HEAD_SHA); + }); + + test("an unknown closed head SHA is kept", () => { + const result = plan( + [closedPr({ headRefOid: null })], + [{ name: "codex/some-work", oid: OLD_TIP }], + ); + expect(result.deletions).toEqual([]); + expect(keepReason(result, "codex/some-work")).toBe(KEEP_REASONS.UNKNOWN_HEAD_SHA); + }); + + test("SHA comparison ignores case", () => { + // The REST and GraphQL APIs disagree about case. A case-sensitive compare + // would keep every branch and quietly turn the job into a no-op. + const result = plan( + [closedPr({ headRefOid: OLD_TIP.toUpperCase() })], + [{ name: "codex/some-work", oid: OLD_TIP }], + ); + expect(result.deletions).toHaveLength(1); + }); + + test("the existing safety rules still hold ahead of the tip check", () => { + // Each of these must win BEFORE the SHA comparison, so a matching tip cannot + // override them. Asserted through the keep reason, not just the empty + // deletion list: the reason is what proves which rule fired. + const at = (name: string, oid: string | null = OLD_TIP) => [{ name, oid }]; + + const merged = plan([closedPr({ merged: true })], at("codex/some-work")); + expect(keepReason(merged, "codex/some-work")).toBe(KEEP_REASONS.MERGED); + + const open = plan([closedPr({ state: "OPEN" })], at("codex/some-work")); + expect(keepReason(open, "codex/some-work")).toBe(KEEP_REASONS.OPEN); + + const fork = plan([closedPr({ isCrossRepository: true })], at("codex/some-work")); + expect(keepReason(fork, "codex/some-work")).toBe(KEEP_REASONS.CROSS_REPOSITORY); + + const stacked = plan( + [ + closedPr(), + closedPr({ number: 43, state: "OPEN", headRefName: "codex/child", baseRefName: "codex/some-work" }), + ], + at("codex/some-work"), + ); + expect(keepReason(stacked, "codex/some-work")).toBe(KEEP_REASONS.BASE_OF_OPEN); + + const recent = plan( + [closedPr({ closedAt: new Date(NOW - 60 * 60 * 1000).toISOString() })], + at("codex/some-work"), + ); + expect(keepReason(recent, "codex/some-work")).toBe(KEEP_REASONS.WITHIN_GRACE); + + const protectedBranch = plan([closedPr({ headRefName: "dev" })], at("dev")); + expect(keepReason(protectedBranch, "dev")).toBe(KEEP_REASONS.PROTECTED); + }); + + test("a branch no pull request ever used is out of scope entirely", () => { + // Neither deleted nor reported as a keep: this job only speaks about + // branches it can attribute to a pull request. + const result = plan([closedPr()], [{ name: "codex/never-a-pr", oid: NEW_TIP }]); + expect(result.deletions).toEqual([]); + expect(keepReason(result, "codex/never-a-pr")).toBeUndefined(); + }); +}); diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index e507e8778f..a2f5b52fdc 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -4747,6 +4747,10 @@ describe("Codex catalog routed normalization", () => { "glm-5.2[1m]": true, "glm-5.3": true, "glm-5.3[1m]": true, + // glm-5.3-flash joined ZAI_GLM_53_MODELS, which is what modelSupportsReasoningSummaries + // is derived from. It belongs in the family for reasoning metadata even though it is + // excluded from the vision-sidecar list - the two answer different questions. + "glm-5.3-flash": true, }); }); diff --git a/tests/codex-prompt-layers.test.ts b/tests/codex-prompt-layers.test.ts index 63b06c14bf..686f75adbb 100644 --- a/tests/codex-prompt-layers.test.ts +++ b/tests/codex-prompt-layers.test.ts @@ -53,6 +53,25 @@ describe("inventory", () => { expect(base?.class).toBe("base"); expect(isToggleId("base-instructions")).toBe(false); }); + + test("git-attribution is runtime-conditional with no key and no fixed order", () => { + // The layer ext/git-attribution contributes. Its shape is the whole assertion: + // `runtime-conditional` because lib.rs:33-80 resolves enablement from the auth + // server rather than a config key (features/src/lib.rs:277 records the old flag as + // removed), and `order: null` because it registers through + // extensions.context_contributors(), whose position is registration-order dependent. + // + // Refusal at the route is NOT re-asserted here: codex-prompt-route.test.ts case 5 + // already drives the real endpoint table-driven over every non-config-toggle + // descriptor, so a second guard would duplicate coverage rather than add it. + const layer = LAYER_INVENTORY.find(d => d.id === "git-attribution"); + expect(layer).toBeDefined(); + expect(layer?.class).toBe("runtime-conditional"); + expect(layer?.key).toBeNull(); + expect(layer?.order).toBeNull(); + expect(layer?.default).toBeNull(); + expect(isToggleId("git-attribution")).toBe(false); + }); }); describe("normalization", () => { diff --git a/tests/moonshot-tool-schema.test.ts b/tests/moonshot-tool-schema.test.ts index 80ceb5e5b9..ae37643172 100644 --- a/tests/moonshot-tool-schema.test.ts +++ b/tests/moonshot-tool-schema.test.ts @@ -226,6 +226,66 @@ describe("Moonshot tool schema normalization (issue #2673)", () => { expect(value.enum).toEqual(["x", "y"]); }); + // BUG-R6: "the node narrows the target" was asserted, never enforced. + // + // The test above uses a node whose minLength is TIGHTER than the target's, so a plain + // overwrite and a real narrowing are indistinguishable there. When the node is LOOSER, + // the two diverge and the overwrite ships the weaker contract - the opposite of what + // the comment claims and of what `$ref` means under 2020-12, where the node and its + // target both apply. + test("a looser sibling assertion does not relax the target", async () => { + const parameters = await emittedParameters("https://api.moonshot.ai/v1", { + name: "loosening_tool", + parameters: { + type: "object", + $defs: { + Tight: { + type: "string", + minLength: 5, + maxLength: 10, + minimum: 10, + maximum: 100, + }, + }, + properties: { + value: { + $ref: "#/$defs/Tight", + // Every one of these is weaker than the target's. + minLength: 1, + maxLength: 99, + minimum: 0, + maximum: 1_000, + }, + }, + }, + }); + + const value = (parameters?.properties as Record>).value!; + // The intersection, per keyword direction: lower bounds take the max, upper bounds + // take the min. Both sides apply, so the surviving constraint is the stricter one. + expect(value.minLength).toBe(5); + expect(value.minimum).toBe(10); + expect(value.maxLength).toBe(10); + expect(value.maximum).toBe(100); + }); + + test("a tighter sibling assertion still wins", async () => { + // The other direction, so the fix cannot be "always prefer the target" - that would + // discard a genuine narrowing, which is the mirror-image bug. + const parameters = await emittedParameters("https://api.moonshot.ai/v1", { + name: "tightening_tool", + parameters: { + type: "object", + $defs: { Loose: { type: "string", minLength: 1, maxLength: 100 } }, + properties: { value: { $ref: "#/$defs/Loose", minLength: 5, maxLength: 10 } }, + }, + }); + + const value = (parameters?.properties as Record>).value!; + expect(value.minLength).toBe(5); + expect(value.maxLength).toBe(10); + }); + test("a deeply nested ref-free schema is bounded instead of exhausting the stack", async () => { // The second blocker: the expansion budget counts $ref inlines only, so a schema with // no refs at all walked unbounded. This nests far past any real tool. diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index 1f155d76df..53c7fc2076 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -369,8 +369,42 @@ describe("provider registry parity", () => { .filter(entry => entry.modelSuffixBracketStrip) .map(entry => entry.id); expect(zai?.modelContextWindows).toEqual({ "glm-5.3": 1_000_000, "glm-5.3[1m]": 1_000_000, "glm-5.3-flash": 1_000_000, "glm-5.2": 1_000_000, "glm-5.2[1m]": 1_000_000 }); - expect(zai?.modelDefaultReasoningEfforts).toEqual({ "glm-5.3": "max", "glm-5.3[1m]": "max" }); - expect(zai?.modelMaxOutputTokens).toEqual({ "glm-5.3": 131_072, "glm-5.3[1m]": 131_072 }); + // BUG-R5: glm-5.3-flash is a native VLM (docs.z.ai/guides/vlm/glm-5.3-flash), so it + // must never sit in noVisionModels - that list routes a model's images through the + // proxy's vision sidecar, which hands the model a text description of a picture it + // can read itself. The seeding pass classified it from the family name; the + // correction pass fixed the Alibaba entries and missed eight other providers. + // + // Asserted across the WHOLE registry rather than per provider, because the defect + // was not one entry being wrong - it was a set of entries drifting apart, and only + // a global assertion catches the next provider to seed it. + for (const entry of PROVIDER_REGISTRY) { + const flashIds = (entry.models ?? []).filter(id => String(id).includes("glm-5.3-flash")); + for (const id of flashIds) { + expect(entry.noVisionModels ?? []).not.toContain(id); + // An explicit modality declaration must include image. Absent is allowed: an + // unclassified model falls through to native passthrough, which is correct here. + const declared = entry.modelInputModalities?.[id]; + if (declared) expect(declared).toContain("image"); + } + } + // The sibling it is most often confused with stays text-only, so the assertion above + // cannot pass by making every GLM row a VLM. + expect(zai?.noVisionModels ?? []).toContain("glm-5.3"); + // `glm-5.3-flash` belongs in all three maps. It was seeded into the model list + // and the context map alone, so it advertised a 1M window with no effort ladder, + // no default effort and no output cap - and this assertion pinned that gap in + // place rather than catching it, because it was written from the incomplete + // state instead of from the family definition. + expect(zai?.modelDefaultReasoningEfforts).toEqual({ "glm-5.3": "max", "glm-5.3[1m]": "max", "glm-5.3-flash": "max" }); + expect(zai?.modelMaxOutputTokens).toEqual({ "glm-5.3": 131_072, "glm-5.3[1m]": 131_072, "glm-5.3-flash": 131_072 }); + // Every 5.3 row carries the same three-tier ladder. Asserted per member rather + // than as one object literal so adding a member cannot quietly skip it. + for (const id of ["glm-5.3", "glm-5.3[1m]", "glm-5.3-flash"]) { + expect(zai?.modelReasoningEfforts?.[id]).toEqual(["low", "high", "max"]); + expect(zai?.modelDefaultReasoningEfforts?.[id]).toBe("max"); + expect(zai?.modelMaxOutputTokens?.[id]).toBe(131_072); + } expect(providerConfigSeed(zai!).modelSuffixBracketStrip).toBe(true); expect(providerConfigSeed(zai!).modelDefaultReasoningEfforts?.["glm-5.3"]).toBe("max"); expect(deriveKeyLoginMap().zai.modelMaxOutputTokens?.["glm-5.3[1m]"]).toBe(131_072);