fix(tools): repair integral floats in native u64 tool fields

devlog/_plan/260822_backlog_disposition_program/011_wp1_execution_record.md

@@ -0,0 +1,103 @@
+# 011 — WP1 execution record: four merged, one held
+
+Work-phase 1 closed with **four of five** PRs on `origin/dev` and one held on
+reproduced test evidence.
+
+## Merged
+
+| PR | Author | Merge commit | Review verdict |
+|----|--------|--------------|----------------|
+| #2309 kiro parallel permission | `Ingwannu` | `b96af222b` | PASS |
+| #2339 google signature order | `luvs01` | `f26c7b5d2` | PASS |
+| #2335 tool-choice linear time | `luvs01` | `25324f839` | GO-WITH-FIXES (blockers=0) |
+| #2313 reasoning replay scoping | `olddonkey` | `f96d9efd2` | PASS |
+| (wp0 roadmap, PR #2369) | — | `5921c20df` | docs-only |
+
+Each was reviewed by an independent read-only lane on `openrouter/stealth-ox-alpha`
+at high reasoning effort. Two lanes produced **falsifiable** regression evidence rather
+than diff-reading:
+
+- **#2339**: reverting only the `src/adapters/google.ts` hunk makes exactly one test
+  fail — `streaming signatures only attach to function calls that follow them in the
+  same frame`, expected `[undefined, SIGNATURE]`, received `[SIGNATURE, ...]`.
+- **#2313**: five separate mutations of the fix each turn the suite red, including
+  dropping the serving identity from the memo key (the exact cross-conversation
+  leakage shape), which fails 2 tests. A baseline diff proves all 11 new integration
+  tests are genuinely new coverage, none tautological.
+- **#2335**: the perf test was adversarially falsified before being trusted — the old
+  path produces ~65k proxy catalog reads at n=256 versus 512 for the new one, so the
+  `size * 2` bound genuinely discriminates O(n²) from O(n).
+
+## Held: #2359
+
+The review lane returned **FAIL**, and the main agent reproduced it independently on
+the merged tree:
+
+```
+$ bun test tests/provider-live-models.test.ts
+(fail) opencode-free live discovery exposes big-pickle plus -free ids
+  [ "big-pickle", - "deepseek-v4-flash-free", "hy3-free", ... ]
+  at tests/provider-live-models.test.ts:163
+ 7 pass, 1 fail
+```
+
+A live probe of `GET https://opencode.ai/zen/v1/models` shows
+`deepseek-v4-flash-free` is **still advertised**, so excluding it hides a model the
+gateway is currently serving. This is the same error class the author already
+self-corrected once inside this PR: `d587a4b4` added `opencode-go/grok-4.6` to the
+exclusion set and `e5c83067` retracted it after finding grok-4.6 live.
+
+Evidence posted to the PR (comment `5379495549`) with the failing assertion, the live
+probe, the author's own precedent, and two non-blocking follow-ups. The two
+`opencode-go` exclusions in the same PR are correct and land as soon as the Zen entry
+is resolved.
+
+## Correction to `001`: `dev` IS protected
+
+`001` recorded "dev protection = 404 not protected" from
+`GET /repos/.../branches/dev/protection`. That endpoint reports only **classic branch
+protection**. The push was rejected:
+
+```
+remote: - Changes must be made through a pull request.
+ ! [remote rejected]     dev -> dev (push declined due to repository rule violations)
+```
+
+`GET /repos/lidge-jun/opencodex/rulesets` shows four **active rulesets**, and
+`GET /repos/.../rules/branches/dev` shows `deletion`, `non_fast_forward`, and
+`pull_request` rules from ruleset `20763889` ("Protect dev"). Every later work-phase
+lands through a pull request with admin merge — which is also better practice, since it
+closes each PR and credits its author. The repository's security configuration was not
+weakened to force a direct push.
+
+## Incident: a reset dropped an unpushed commit
+
+While preparing the merge I ran `git reset --hard origin/dev` on the shared checkout,
+which discarded the unpushed wp0 devlog commit `eb3c97476`. Detected immediately with
+`git merge-base --is-ancestor` (returned LOST), confirmed the object still existed via
+`git cat-file -t`, and restored all 12 documents by cherry-pick (`d2f0aab86`, finally
+`d374bb893` on the PR branch). No work was lost.
+
+The lesson is recorded rather than quietly fixed: a `--hard` reset on a shared checkout
+carrying unpushed work is exactly the destructive-command class that deserves a
+reachability check *before* it runs, not after.
+
+## Verification
+
+Local, focused only — full suites are not run on this machine:
+
+```
+bun x tsc --noEmit                      TSC=0
+bun test <9 suites covering every changed file>
+                                        339 pass / 0 fail / 1578 expect()
+```
+
+Suites: `kiro-adapter`, `google-signature-history-roundtrip`,
+`tool-choice-performance`, `types-barrel-identity`, `reasoning-replay-identity`,
+`request-log-conversation`, `responses-opaque-blob-recovery`,
+`provider-live-models`, `codex-catalog`.
+
+The full suite runs on remote host `lidge` against the merged head `5921c20df` as a
+trailing job, observed rather than blocking; its result and the exact-head CI check are
+recorded at the end of the program, not per phase.
+

src/lib/tool-argument-integers.ts

@@ -59,6 +59,24 @@ function declaresString(schema: SchemaNode): boolean {
   return Array.isArray(type) && type.includes("string");
 }
 
+// Issue #2316: Codex advertises `multi_agent_v1__wait_agent`'s `timeout_ms` as a JSON
+// Schema `number`, but its Rust runtime deserializes the field as `u64` and rejects
+// `120000.0` with "invalid type: floating point `120000.0`, expected u64" before the
+// tool ever runs. The schema lookup is not the problem — the error text comes from
+// Codex's own deserializer, so the call reached it — the problem is that `number`
+// alone never authorizes the integral-float repair below.
+//
+// This allowlist is deliberately one field wide. It names only what has a live
+// reproduction against a real `u64`, because the repair is only unambiguous for a
+// field that cannot legitimately hold a fraction. Generic names (`start`, `end`,
+// `priority`, `port`) would silently rewrite a third-party tool's fractional value,
+// so a new entry needs its own evidence, not a plausible-sounding name.
+//
+// Cursor's sibling `yield_time_ms` (src/adapters/cursor/tool-definitions.ts) is also
+// declared `number`; it is NOT included here because no rejection has been captured
+// against it. It gets its own change when it gets its own reproduction.
+const U64_NUMBER_FIELDS = new Set(["timeout_ms"]);
+
 /** True when the node accepts a JSON number (`integer` or `number`), so a numeric
  * value is already schema-valid and must not be rewritten into a string. */
 function declaresNumeric(schema: SchemaNode): boolean {
@@ -118,15 +136,29 @@ interface CoerceResult {
   changed: boolean;
 }
 
-function coerceValue(value: unknown, schema: SchemaNode | undefined, root: SchemaNode, depth: number): CoerceResult {
+function coerceValue(
+  value: unknown,
+  schema: SchemaNode | undefined,
+  root: SchemaNode,
+  depth: number,
+  propertyName?: string,
+): CoerceResult {
   // A hostile or deeply nested schema must not blow the stack.
   if (depth > 64) return { value, changed: false };
   const resolved = schema ? resolveRef(schema, root, new Set()) : undefined;
 
   if (typeof value === "number") {
     if (!resolved) return { value, changed: false };
     const branches = compositionBranches(resolved);
-    const integerDeclared = declaresInteger(resolved) || branches.some(declaresInteger);
+    // #2316: a known Codex-native u64 field counts as integer-declared even when the
+    // advertised schema says `number`, but only when the field really is numeric —
+    // an allowlisted name over a string-typed field is a disagreement, not a repair.
+    const nativeU64Declared = propertyName !== undefined
+      && U64_NUMBER_FIELDS.has(propertyName)
+      && (declaresNumeric(resolved) || branches.some(declaresNumeric));
+    const integerDeclared = declaresInteger(resolved)
+      || branches.some(declaresInteger)
+      || nativeU64Declared;
     if (!integerDeclared && safelyIntegral(value)) {
       // Issue #1938: a bare integer in a string-only field has exactly one faithful
       // string reading. A field that also accepts a numeric type keeps the number.
@@ -148,6 +180,8 @@ function coerceValue(value: unknown, schema: SchemaNode | undefined, root: Schem
     const itemSchema = resolved ? asSchema(resolved.items) : undefined;
     let changed = false;
     const next = value.map(entry => {
+      // Array items have no property name of their own; passing the array's key would
+      // let `timeout_ms: [1.5]` inherit the allowlist. Items are judged by schema only.
       const result = coerceValue(entry, itemSchema, root, depth + 1);
       if (result.changed) changed = true;
       return result.value;
@@ -164,7 +198,7 @@ function coerceValue(value: unknown, schema: SchemaNode | undefined, root: Schem
   const next: Record<string, unknown> = {};
   for (const [key, entry] of Object.entries(object)) {
     const childSchema = asSchema(properties?.[key]) ?? additional;
-    const result = coerceValue(entry, childSchema, root, depth + 1);
+    const result = coerceValue(entry, childSchema, root, depth + 1, key);
     if (result.changed) changed = true;
     next[key] = result.value;
   }

tests/tool-argument-integers.test.ts

@@ -259,3 +259,107 @@ describe("bare-integer-for-string tool argument repair (#1938)", () => {
   });
 });
 
+
+/**
+ * The multi_agent wait shape from the #2316 report. Codex advertises `timeout_ms` as a
+ * JSON Schema `number` while its Rust runtime deserializes it as `u64`, so an integral
+ * float that is perfectly valid JSON is rejected before the tool runs.
+ */
+const MULTI_AGENT_WAIT_SCHEMA = {
+  type: "object",
+  properties: {
+    targets: { type: "array", items: { type: "string" } },
+    timeout_ms: { type: "number" },
+    temperature: { type: "number" },
+  },
+};
+
+describe("native u64 fields advertised as number (#2316)", () => {
+  test("repairs the exact wait_agent call from the report", () => {
+    expect(coerceIntegerToolArguments('{"targets":["a"],"timeout_ms":120000.0}', MULTI_AGENT_WAIT_SCHEMA))
+      .toBe('{"targets":["a"],"timeout_ms":120000}');
+    expect(coerceIntegerToolArguments('{"timeout_ms":60000.0}', MULTI_AGENT_WAIT_SCHEMA))
+      .toBe('{"timeout_ms":60000}');
+  });
+
+  test("a fractional timeout is a real disagreement and still fails upstream", () => {
+    const raw = '{"timeout_ms":1.5}';
+    expect(coerceIntegerToolArguments(raw, MULTI_AGENT_WAIT_SCHEMA)).toBe(raw);
+  });
+
+  test("an ordinary number field beside it is still never touched", () => {
+    const raw = '{"temperature":1.0}';
+    expect(coerceIntegerToolArguments(raw, MULTI_AGENT_WAIT_SCHEMA)).toBe(raw);
+  });
+
+  test("an already-integral payload keeps its original bytes", () => {
+    const clean = '{"targets":["a"],"timeout_ms":120000}';
+    expect(coerceIntegerToolArguments(clean, MULTI_AGENT_WAIT_SCHEMA)).toBe(clean);
+  });
+
+  test("the allowlist reaches a nested object, not just the top level", () => {
+    const nested = {
+      type: "object",
+      properties: { opts: { type: "object", properties: { timeout_ms: { type: "number" } } } },
+    };
+    expect(coerceIntegerToolArguments('{"opts":{"timeout_ms":120000.0}}', nested))
+      .toBe('{"opts":{"timeout_ms":120000}}');
+  });
+
+  test("an array named like the allowlist does not inherit it", () => {
+    // Array items have no property name of their own, so the element is judged by its
+    // own schema. A fractional element stays fractional rather than being rewritten.
+    const arrayed = {
+      type: "object",
+      properties: { timeout_ms: { type: "array", items: { type: "number" } } },
+    };
+    const raw = '{"timeout_ms":[1.5]}';
+    expect(coerceIntegerToolArguments(raw, arrayed)).toBe(raw);
+  });
+
+  test("an allowlisted name over a string field is a disagreement, not a repair", () => {
+    // `number` is what authorizes the repair. A string-typed timeout_ms carrying a bare
+    // integer falls to the #1938 rule instead, which stringifies it.
+    const stringy = { type: "object", properties: { timeout_ms: { type: "string" } } };
+    expect(coerceIntegerToolArguments('{"timeout_ms":120000}', stringy))
+      .toBe('{"timeout_ms":"120000"}');
+  });
+
+  test("a field NOT on the allowlist keeps its float even when integral", () => {
+    // Deliberate scope proof: only names with a captured u64 rejection are repaired.
+    const others = {
+      type: "object",
+      properties: { yield_time_ms: { type: "number" }, priority: { type: "number" } },
+    };
+    const raw = '{"yield_time_ms":60000.0,"priority":2.0}';
+    expect(coerceIntegerToolArguments(raw, others)).toBe(raw);
+  });
+
+  test("the namespaced wait_agent call is repaired through the real bridge", async () => {
+    const schemas = new Map<string, Record<string, unknown>>([
+      ["multi_agent_v1__wait_agent", MULTI_AGENT_WAIT_SCHEMA],
+    ]);
+    const frames = await collectSse(bridgeToResponsesSSE(
+      replay([
+        { type: "tool_call_start", id: "call_1", name: "multi_agent_v1__wait_agent" },
+        { type: "tool_call_delta", arguments: '{"timeout_ms":120000.0}' },
+        { type: "tool_call_end", id: "call_1" },
+        { type: "done" },
+      ]),
+      "grok-4.6", undefined, undefined, undefined, undefined, 2_000, { toolParameterSchemas: schemas },
+    ));
+    const done = frames.find(f => f.event === "response.function_call_arguments.done");
+    expect(done?.data.arguments).toBe('{"timeout_ms":120000}');
+
+    // The non-streaming path is the same contract; Codex parses the completed item.
+    const body = buildResponseJSON([
+      { type: "tool_call_start", id: "call_1", name: "multi_agent_v1__wait_agent" },
+      { type: "tool_call_delta", arguments: '{"timeout_ms":120000.0}' },
+      { type: "tool_call_end", id: "call_1" },
+      { type: "done" },
+    ], "grok-4.6", { toolParameterSchemas: schemas }) as Record<string, unknown>;
+    const call = (body.output as Record<string, unknown>[]).find(i => i.type === "function_call");
+    expect(call?.arguments).toBe('{"timeout_ms":120000}');
+  });
+});
+