Skip to content

test(catalog): pin the two model-preset convergence calls (#2465) #2606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidge-jun merged 1 commit into from
Aug 25, 2026
Merged

test(catalog): pin the two model-preset convergence calls (#2465) #2606

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions tests/codex-convergence-contract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@ test("a failure cause never carries message text, paths or identifiers (#1784)",
expect(body).not.toContain("failed writing");
});

test("the route inventory contains exactly the specified 7 + 6 + 2 + 2 convergence calls", () => {
test("the route inventory contains exactly the specified 7 + 8 + 2 + 2 convergence calls", () => {
const counts = Object.fromEntries([
["provider-routes.ts", 7],
["model-routes.ts", 6],
["model-routes.ts", 8],
["combo-routes.ts", 2],
["agent-settings-routes.ts", 2],
].map(([file, expected]) => {
Expand All @@ -387,12 +387,35 @@ test("the route inventory contains exactly the specified 7 + 6 + 2 + 2 convergen
}));
expect(counts).toEqual({
"provider-routes.ts": 7,
"model-routes.ts": 6,
"model-routes.ts": 8,
"combo-routes.ts": 2,
"agent-settings-routes.ts": 2,
});
});

/**
* Same discipline as the reload-route assertion below: the count above went 6 -> 8 for the
* model-preset routes (#2465), and a bare count that only ever rises stops being a contract.
* Assert those two calls specifically, so a later bump cannot pass while some OTHER route
* quietly gained one, or while a preset route lost its own convergence.
*
* Both are write paths that change which models ship to the catalog — applying a preset
* narrows it, clearing back to "all" widens it — so each must converge for exactly the same
* reason `PUT /api/selected-models` does.
*/
test("both model-preset write paths converge the Codex catalog", () => {
const source = readFileSync(
join(import.meta.dir, "..", "src", "server", "management", "model-routes.ts"),
"utf8",
);
const handlerStart = source.indexOf('url.pathname === "/api/model-presets" && req.method === "PUT"');
expect(handlerStart).toBeGreaterThan(-1);
const handlerBody = source.slice(handlerStart, source.indexOf("url.pathname ===", handlerStart + 1));
// The "all" branch and the materialize branch each converge; "custom" only moves the marker,
// so it deliberately does not.
expect(handlerBody.match(/await convergeCodexCatalog\(\)/g)?.length).toBe(2);
Comment on lines +413 to +416

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert convergence in each preset branch

This assertion only counts two calls anywhere in the entire PUT handler, so it still passes if the required all or preset branch loses its convergence call while the custom branch gains one, even though the test explicitly intends to pin both write paths and ensure custom does not converge. Split the handler into branch-specific slices and assert one call in all, zero in custom, and one in the materialization path, or invoke each mode with a convergence spy.

Useful? React with 👍 / 👎.

});

/**
* The inventory above is a bare count, so raising it is the obvious way to make this file
* green again — and a count that only ever gets raised stops being a contract. #1541's
Expand Down
Loading