fix: L3 cherry-pick lane of the 260827 bug-PR merge round (#2639 #2647)

devlog/_plan/260827_bug_pr_merge_round/021_status_vs_created_at_asymmetry.md

@@ -0,0 +1,60 @@
+# The status/created_at split is narrower than it first looked
+
+The L3 pick for #2639 rests on a distinction: `status` is safe to backfill,
+`created_at` is not, because `created_at` breaks the byte-exact combo passthrough
+assertion. The reviewer auditing the cherry-pick asked the obvious follow-up — does
+`status` violate the same contract? — and the honest answer is **yes, it can**.
+
+## Evidence
+
+`tests/server-combo-failover-e2e.test.ts` builds its backup body with
+`responsesSuccess()` (line 216), which already sets `status: "completed"` on both the
+response and the message item. So the byte-exact assertion never exercises the
+`status` backfill — it passes because the field is already present.
+
+Remove that one field from the fixture and the same assertion fails on this branch:
+
+```
+$ cd /tmp/ocx-statusprobe   # fixture with the item's status: "completed" deleted
+$ bun test ./tests/server-combo-failover-e2e.test.ts -t 'exact backup response'
+(fail) ... returns the exact backup response
+  + "status": "completed",     # injected into a body relayed verbatim
+```
+
+And it passes with dev's version of the backfill file restored:
+
+```
+$ git checkout 2feffbdc3 -- src/server/responses/responses-field-backfill.ts
+$ bun test ./tests/server-combo-failover-e2e.test.ts -t 'exact backup response'
+ 1 pass, 0 fail
+```
+
+So the difference between the two halves is NOT that one mutates passthrough bodies
+and the other does not. Both do. `src/server/responses/core.ts:4145` runs
+`backfillResponsesFieldsJson` on the bounded-JSON answer, and line 3937 installs the
+SSE rewrite, on the passthrough path as well as the translated one.
+
+## What the difference actually is
+
+`created_at` fires on EVERY response body that lacks the field, and a relay that omits
+`created_at` is common. `status` fires only on a `message` item that lacks `status`,
+which is rarer and is a genuine spec violation upstream — `OutputMessage.status` is
+required, while a missing `created_at` is a Response-level omission the same decoders
+complain about. The blast radius differs by roughly an order of magnitude, and the
+existing test suite happens to sit on the safe side of the `status` case and the
+unsafe side of the `created_at` case.
+
+That is a defensible reason to take one and hold the other, but it is a difference of
+DEGREE, not of kind, and 002 overstated it as a clean line. Corrected here.
+
+## Consequence
+
+The open question in 002 — whether the backfill should be scoped to the translated
+path so a verbatim relay stays verbatim — now applies to `status` too, not only to
+`created_at`. Whoever resolves it should resolve both together. That is a follow-up
+for its own PABCD cycle, not something to bolt onto this cherry-pick: it changes
+behavior for every Responses provider, and the right answer probably involves the
+passthrough path opting out of field backfill entirely.
+
+Recorded rather than fixed here because this lane's contract is "take the correct
+part of a partially-right PR", and the `status` backfill IS what #2639 got right.

devlog/_plan/260827_bug_pr_merge_round/022_l3_audit.md

@@ -0,0 +1,53 @@
+# L3 audit — reviewer findings and dispositions
+
+The independent reviewer audited `codex/l3-cherry-picks-260827` (PR #2721) and
+returned VERDICT: FAIL. Nine findings; four required code changes. All are fixed.
+
+| # | Finding | Disposition |
+|---|---|---|
+| 1 | `status` backfill correctly scoped to `type === "message"`, uses `"status" in item` so a falsy value is never overwritten | confirmed, no change |
+| 2 | `created_at` exclusion verified causally (73/74 before, 74/74 after) | confirmed |
+| 3 | The status/created_at split survives on FIXTURE CONTENTS, not on a principled difference | recorded in 021 |
+| 4 | **`response.queued` produced `completed`** — an unstarted message marked finished | fixed `6877f646f` |
+| 5 | **The regenerated fixture resurrected `stealth/ox-alpha`**, removed in `328931265` | fixed `45c3d31eb` |
+| 6 | **The self-correction justification is false** | fixed `ad8ab4f70` |
+| 7 | Dropping the PR's test file was right (it reintroduced both ox-alpha ids) | confirmed |
+| 8 | Split is clean; one stray ` *` comment line | fixed `8af9ff2bf` |
+| 9 | No status coverage lost by deleting the created_at tests; 148/148 at head | confirmed |
+
+## Finding 6 is the one worth remembering
+
+I wrote a provenance comment saying it was acceptable to record the reporter's
+unverified ladders BECAUSE `refreshCommandCodeReasoningEfforts()` would re-read the
+public profile and replace a wrong row after the first upstream rejection. I had read
+that function and it does exactly what I described — in the code.
+
+The reviewer ran it against the live site instead:
+
+```
+gpt-5.6-luna                          -> UNDEFINED
+google/gemini-3.7-flash               -> UNDEFINED
+deepseek/deepseek-v4-flash-vision-exp -> UNDEFINED
+```
+
+Confirmed independently: all three URLs return 200, and
+`grep -c -i 'reasoning efforts'` on the fetched HTML is 0. The pages ship the ladder
+inside a serialized React payload whose `reasoningEfforts` array is EMPTY in the
+delivered bytes. `parsedProfileEfforts` needs prose of the form
+"Reasoning efforts ... are supported;", finds none, returns undefined — so the row is
+never replaced. The same measurement returns 0 for `deepseek-v4-pro`, `GLM-5.3` and
+`muse-spark-1.2`, so the mechanism is dead for EVERY row in the table, not just the
+three added here.
+
+The lesson is narrow and worth stating: a safety net that exists in the code is not a
+safety net that functions. I cited a mechanism as the reason to accept unverified
+data without testing that the mechanism fires. The reviewer tested it.
+
+## Carried forward
+
+The dead parser is a real pre-existing defect: `parsedProfileEfforts` should read the
+embedded `reasoningEfforts` payload rather than prose. It is NOT fixed here — it
+affects every row, it is not what #2647 reported, and bolting it onto a cherry-pick
+lane would be exactly the scope creep this round is structured to avoid. It belongs
+in its own cycle, and the source comment now says so plainly so the next person does
+not re-derive the false justification.

src/providers/command-code-efforts.ts

@@ -9,6 +9,45 @@ const COMMAND_CODE_MODEL_EFFORTS = {
     efforts: ["high", "max"],
     profileUrl: "https://commandcode.ai/models/deepseek-v4-flash",
   },
+  /*
+   * Three live routes that reached the catalog without an effort ladder (#2647).
+   * Without a row here the model advertises no efforts at all, so a client that
+   * sends one gets it stripped or rejected rather than honored.
+   *
+   * PROVENANCE, stated plainly: these three ladders are the reporter's
+   * (darwintree, #2647), recorded as reported and NOT independently verified.
+   * All three profileUrls return HTTP 200, but commandcode.ai renders these
+   * pages client-side and ships the ladder inside a serialized React payload
+   * whose `reasoningEfforts` array is EMPTY in the delivered HTML. There is no
+   * fetchable statement of these ladders to check them against.
+   *
+   * Do not assume the refresh path launders this. It does not:
+   * `parsedProfileEfforts` below matches prose of the form
+   * "Reasoning efforts ... are supported;", and `grep -c -i 'reasoning efforts'`
+   * against the live pages returns 0 — for these three AND for the older rows
+   * (deepseek-v4-pro, GLM-5.3, muse-spark-1.2 all measured 0 on 2026-08-27).
+   * So `refreshCommandCodeReasoningEfforts` returns undefined and the caller
+   * keeps whatever is written here, indefinitely. The self-correction mechanism
+   * is currently dead for EVERY row in this table, which is a pre-existing
+   * defect worth its own fix (teach the parser to read the embedded payload),
+   * not something these three rows introduced.
+   *
+   * The practical consequence: a wrong ladder here stays wrong until a human
+   * changes it. It degrades safely — an effort the upstream rejects surfaces as
+   * an error rather than silent corruption — but it does not self-heal.
+   */
+  "deepseek/deepseek-v4-flash-vision-exp": {
+    efforts: ["high", "max"],
+    profileUrl: "https://commandcode.ai/models/deepseek-v4-flash-vision-exp",
+  },
+  "gpt-5.6-luna": {
+    efforts: ["low", "medium", "high", "xhigh", "max"],
+    profileUrl: "https://commandcode.ai/models/gpt-5-6-luna",
+  },
+  "google/gemini-3.7-flash": {
+    efforts: ["low", "medium", "high"],
+    profileUrl: "https://commandcode.ai/models/gemini-3-7-flash",
+  },
   // Keys must match the EXACT upstream /provider/v1/models ids (GLM ships as
   // `zai-org/GLM-5.3`, not `zai-org/glm-5.3`). The table doubles as the router's
   // known-ids decode source (via `knownModelIdsForProvider`), so a case mismatch

src/server/responses/responses-field-backfill.ts

@@ -91,6 +91,29 @@ function nextSyntheticItemSlot(): ItemIdSlot {
   return { kind: "fallback", ordinal: syntheticItemOrdinal };
 }
 
+/**
+ * Backfill `status` on a message output item if missing.
+ *
+ * The Responses API spec defines `status` as a required field on
+ * `OutputMessage`. Some upstream relays omit it, which causes strict
+ * deserializers (e.g. grok-build's serde types) to fail with
+ * `missing field 'status'`. Only message items carry this field in the
+ * Responses schema; reasoning, function_call, and other item types do not.
+ *
+ * The value is inferred from the event context: `output_item.added` and
+ * `response.created` / `response.in_progress` mean the message is still
+ * being generated (`in_progress`); `output_item.done` and
+ * `response.completed` / `response.incomplete` mean the message is
+ * finalized (`completed` / `incomplete` respectively).
+ *
+ * Returns the same object reference if no change is needed.
+ */
+function backfillItemStatus(item: Record<string, unknown>, inferredStatus: string): Record<string, unknown> {
+  if (item.type !== "message") return item;
+  if ("status" in item) return item;
+  return { ...item, status: inferredStatus };
+}
+
 /**
  * Backfill annotations: [] on an output_text content part if missing.
  * Returns the same object reference if no change is needed.
@@ -122,10 +145,10 @@ function backfillContentArray(content: unknown): unknown {
 
 /**
  * Walk an output item and backfill output_text parts in its content.
- * Also backfills a missing required id on the item itself.
+ * Also backfills a missing required id and status on the item itself.
  * Returns the same object reference if nothing changed.
  */
-function backfillOutputItem(item: unknown, slot: ItemIdSlot): unknown {
+function backfillOutputItem(item: unknown, slot: ItemIdSlot, inferredStatus: string): unknown {
   if (!isPlainObject(item)) return item;
   // The compact wire family is the `/v1/responses/compact` format, not a Responses output item.
   // Those items have no `id` in that contract, so synthesizing one changes a response body the
@@ -136,34 +159,92 @@ function backfillOutputItem(item: unknown, slot: ItemIdSlot): unknown {
   const content = item.content;
   const repaired = backfillContentArray(content);
   const withId = backfillItemId(item, slot);
-  if (repaired === content && withId === item) return item;
-  return { ...withId, ...(repaired === content ? {} : { content: repaired }) };
+  const withStatus = backfillItemStatus(withId, inferredStatus);
+  if (repaired === content && withStatus === item) return item;
+  return { ...withStatus, ...(repaired === content ? {} : { content: repaired }) };
 }
 
 /**
  * Walk a response object's output[] and backfill output_text parts.
+ *
+ * `inferredItemStatus` is the status to backfill on message items that lack
+ * one — derived from the event type so `output_item.added` / `response.created`
+ * gets `in_progress` while `output_item.done` / `response.completed` gets
+ * `completed`.
+ *
+ * Deliberately does NOT backfill `created_at`. #2639 proposed it for the same
+ * strict-decoder reason as `status`, but the proxy also relays some upstream
+ * responses verbatim, and `tests/server-combo-failover-e2e.test.ts` asserts a
+ * combo backup response is returned byte-exact. Injecting a field the upstream
+ * never sent breaks that contract. Both cannot hold for the same body, so the
+ * `created_at` half needs its own decision about which contract yields; it is
+ * not a detail to slip in beside `status`.
+ *
  * Returns the same object reference if nothing changed.
  */
-function backfillResponseOutput(response: unknown): unknown {
+function backfillResponseOutput(response: unknown, inferredItemStatus: string): unknown {
   if (!isPlainObject(response)) return response;
   const output = response.output;
   if (!Array.isArray(output)) return response;
   let changed = false;
   const repaired = output.map((item, idx) => {
     if (!isPlainObject(item)) return item;
-    const next = backfillOutputItem(item, { kind: "index", index: idx });
+    const next = backfillOutputItem(item, { kind: "index", index: idx }, inferredItemStatus);
     if (next !== item) changed = true;
     return next;
   });
   return changed ? { ...response, output: repaired } : response;
 }
 
+/**
+ * Infer the status to backfill on a message item from the event type.
+ *
+ * `output_item.added` means the item is still being generated (`in_progress`);
+ * `output_item.done` means it is finalized (`completed`). Response-level events
+ * infer from the response's own status field — which is authoritative when present.
+ * If the response status is also absent, the event type itself determines the phase:
+ * `response.created` / `response.in_progress` → `in_progress`,
+ * `response.completed` → `completed`, `response.incomplete` → `incomplete`.
+ */
+function inferredStatusForEventType(eventType: string): string {
+  if (eventType === "response.output_item.added") return "in_progress";
+  if (eventType === "response.output_item.done") return "completed";
+  if (eventType === "response.created" || eventType === "response.in_progress") return "in_progress";
+  // `queued` is a real Responses lifecycle status: the response exists but has not
+  // started generating. Without this row it falls through to the `completed`
+  // default below, which would mark an unstarted message as finished.
+  if (eventType === "response.queued") return "in_progress";
+  if (eventType === "response.incomplete" || eventType === "response.failed") return "incomplete";
+  return "completed";
+}
+
+/**
+ * Map a response-level lifecycle status to a valid OutputMessage status.
+ *
+ * `OutputMessage.status` accepts only `in_progress`, `completed`, or
+ * `incomplete`. Response-level statuses like `failed` or `cancelled` have no
+ * direct message-level equivalent, but `incomplete` is the correct semantic
+ * mapping: the message did not finish generating. Writing `completed` would
+ * assert something the upstream never claimed — a client branching on
+ * `status === "completed"` would treat a truncated message as whole.
+ */
+function messageStatusFromResponseStatus(status: string): string | null {
+  if (status === "in_progress" || status === "completed" || status === "incomplete") return status;
+  // A queued response has not begun generating, so its message items are
+  // in_progress — never completed. Returning null here would fall back to the
+  // event-type inference, whose default is `completed`.
+  if (status === "queued") return "in_progress";
+  if (status === "failed" || status === "cancelled") return "incomplete";
+  return null;
+}
+
 /**
  * Statelessly rewrite one SSE event: backfill annotations
  * on any output_text content part found in the event payload.
  */
 function rewriteEvent(event: Record<string, unknown>): Record<string, unknown> {
   const type = typeof event.type === "string" ? event.type : "";
+  const inferredItemStatus = inferredStatusForEventType(type);
   let next = event;
   let changed = false;
 
@@ -177,8 +258,8 @@ function rewriteEvent(event: Record<string, unknown>): Record<string, unknown> {
     // is not recoverable in that case, but a unique id is what strict decoders require, and a
     // well-formed stream still gets the stable index-derived id.
     const item = typeof rawIndex === "number" && Number.isInteger(rawIndex) && rawIndex >= 0
-      ? backfillOutputItem(event.item, { kind: "index", index: rawIndex })
-      : backfillOutputItem(event.item, nextSyntheticItemSlot());
+      ? backfillOutputItem(event.item, { kind: "index", index: rawIndex }, inferredItemStatus)
+      : backfillOutputItem(event.item, nextSyntheticItemSlot(), inferredItemStatus);
     if (item !== event.item) {
       next = { ...next, item };
       changed = true;
@@ -198,7 +279,13 @@ function rewriteEvent(event: Record<string, unknown>): Record<string, unknown> {
   // response.created / in_progress / completed / incomplete / failed:
   // response.output[].content[] -> output_text parts
   if (isPlainObject(event.response)) {
-    const response = backfillResponseOutput(event.response);
+    // For response-level events, prefer the response's own status when it is a valid
+    // OutputMessage status. Response lifecycle statuses like "failed" or "cancelled"
+    // have no message-level equivalent — fall back to the event-type inference instead.
+    const responseStatus = typeof event.response.status === "string"
+      ? messageStatusFromResponseStatus(event.response.status) ?? inferredItemStatus
+      : inferredItemStatus;
+    const response = backfillResponseOutput(event.response, responseStatus);
     if (response !== event.response) {
       next = { ...next, response };
       changed = true;
@@ -210,9 +297,9 @@ function rewriteEvent(event: Record<string, unknown>): Record<string, unknown> {
 
 /**
  * Create a stateless SSE block rewrite that backfills annotations and
- * on output_text content parts. Unconditional: the field is a required
- * canonical Responses field, so adding it when absent is safe for all
- * clients.
+ * message status on output_text content parts and message items.
+ * Unconditional: both are required canonical Responses fields, so adding
+ * them when absent is safe for all clients.
  */
 export function createResponsesFieldBackfillBlockRewrite(): SseBlockRewrite {
   const rewrite: SseBlockRewrite = (block: string): readonly string[] => {
@@ -245,7 +332,12 @@ export function backfillResponsesFieldsJson(payload: string): string {
     return payload;
   }
   if (!isPlainObject(response)) return payload;
-  const repaired = backfillResponseOutput(response);
+  // For a non-streaming response, derive the item status from the response's own
+  // status field when it is a valid OutputMessage status; fall back to "completed".
+  const inferredItemStatus = typeof response.status === "string"
+    ? messageStatusFromResponseStatus(response.status) ?? "completed"
+    : "completed";
+  const repaired = backfillResponseOutput(response, inferredItemStatus);
   if (repaired === response) return payload;
   return JSON.stringify(repaired);
 }