[freeqaz]

Problem

#2763 records a PROCESS_EXIT model-probe cause detail that is outside the closed vocabulary as UNRECOGNIZED, or UNRECOGNIZED-attempt2 for a retried check. A controller from before #2763 rejects that detail with ScopeViolationError: Runtime failure cause is invalid.. During a rollback or a rolling upgrade, that controller then cannot read the deployment's status. This was raised in the review of #2763, which I merged before addressing it.

Change

• Writer: packages/occ/src/runtime-failure-cause.ts now omits such a detail and records {kind: "PROCESS_EXIT"}. Every controller version reads that, and the unknown text is still never exposed.
• Reader: still accepts UNRECOGNIZED and UNRECOGNIZED-attempt2 from rows recorded since refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763.
• Docs: the cause table in docs/reference/agents/deployment.md now says other details are dropped. Word count is unchanged.

Verification

• Mixed-version test: a new test checks every recorded cause against the cause check of a controller from before refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763, frozen from 787bb9731^. The masking test now expects the omitted detail and the stored UNRECOGNIZED read-back. Both fail on main.
• Against the real older reader: I ran the actual pre-refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 runtime-failure-cause.ts against this writer's JSON-round-tripped rows for exit-1, signal-SIGKILL-attempt2, error-ENOBUFS, exit-256, error-EVILEXAMPLECOM and signal-SIGSEEEXAMPLE-attempt2. It read every one.
• Mutants: 5/5 killed. They covered:
  • the stored UNRECOGNIZED refused on read, in the regex and in the set;
  • an unknown detail kept;
  • an unknown detail replaced by UNRECOGNIZED;
  • every detail dropped.
• Other tests: contracts, controller-stop-yields, runtime-credentials-api, codex-native-startup, kubernetes-compute, plugin-compute and codex-model-probe all pass.
• Internal review: approve with nits. Applied: the test now shows its earlier-controller check refuses UNRECOGNIZED, plus a comment reword. The older rows are named under Risks.
• Static checks: tsc --build, eslint and prettier are clean on the touched files.
• Stability: contracts.test.mjs passed 20/20 at 2 CPUs.

Risks

• Low. An unrecognized detail loses its -attempt2 retry marker. Known details keep it.
• Rows already written: rows recorded between refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 and this change can still hold UNRECOGNIZED, and a pre-refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 controller cannot read those. That window is a few hours of main, no release tag, and needs a Sandbox-chosen exit token. API clients can still see UNRECOGNIZED from those rows.
• Still pending from the refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 review: the request for a contract-owner review of the diagnostics masking.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[freeqaz-openai]

Free’s clanker here. The mixed-version observation in the Problem section is real, but you don’t owe this PR to satisfy my earlier review. I withdrew that requirement on #2763 after checking the repository’s compatibility boundary. Sorry I sent you down that path.

Two distinctions for the contract owner: the supported upgrade script quiesces both API and worker, and this writer change cannot make a sentinel already persisted readable by the earlier controller (as your Risks section correctly notes). If omitting unknown detail is independently the better current contract, assess that smaller change on its merits. I wouldn’t enshrine a frozen development-era reader as a supported contract solely because I asked. I’m not requesting a revert, closing this, or assuming the owner’s decision.

[clawsweeper]

Codex review: blocked before merge.

What this changes

This PR omits unknown process-exit details while retaining reader support for previously recorded UNRECOGNIZED values.

Example: A model probe reports signal-SIGSEEEXAMPLE-attempt2.

• Before: OCC records PROCESS_EXIT with detail UNRECOGNIZED-attempt2.
• After: OCC records PROCESS_EXIT without a detail or retry marker.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The narrow normalization change is source-understandable, but verification is validator-level and the historical compatibility test conflicts with repository policy.
Proof confidence	🦐 gold shrimp (3/6)	The ordinary contributor proof gate does not apply to this collaborator-authored PR; supplied verification reports actual validator round trips but no shipped-server demonstration.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Product

Kind: Preference · Worth it: No · Fix scope: Partial
User problem: An earlier development controller cannot read some failure rows written by current main.
Reason: The collaborator-authored direction needs no second owner approval, but its stated motivation is outside the compatibility boundary, the original request was withdrawn, and already persisted sentinel rows remain unreadable by the earlier controller.

Merge readiness

⛔ Blocked before merge - 2 items remain

Keep this PR open for maintainer triage. The mixed-version mismatch is real, but the earlier reviewer withdrew the compatibility requirement, and current main's writer and reader already agree.

Priority: P3
Reviewed head: b18d153358891607767fc3fe31900f14e7fc32d8

Before merge

• Remove the frozen development-reader compatibility test (P3) - This test copies the pre-consolidation reader and requires future writes to satisfy it. AGENTS.md explicitly rejects preserving development helpers or fixture shapes solely for backward compatibility, and the reviewer who requested this requirement withdrew it. Consequently, a valid current-contract sentinel fails this test despite working with the supported reader. Remove this historical contract assertion and retain the adjacent current writer/read-back coverage.
• Product: not worth merging - The collaborator-authored direction needs no second owner approval, but its stated motivation is outside the compatibility boundary, the original request was withdrawn, and already persisted sentinel rows remain unreadable by the earlier controller.

Findings

• [P3] Remove the frozen development-reader compatibility test — tests/conformance/contracts.test.mjs:688-696

Tests

• Low-value test tests/conformance/contracts.test.mjs: The new historical-reader test freezes an unsupported development implementation and repeats the adjacent normalization cases without protecting a supported additional workflow.
• Missing end-to-end proof: The reported JSON-round-trip checks exercise validator compatibility, not a supported deployment-status workflow through the shipped server.

Agent review details

How this fits together

OCC validates runtime failure evidence before persistence, deployment-status projection, and sanitized log output; bounded cause records leave this boundary for API, CLI, and console consumers.

flowchart TD
  Runtime[Runtime model probe] --> Validator[OCC cause validator]
  Validator --> Store[Deployment work records]
  Store --> Status[Deployment status]
  Runtime --> Logs[Sanitized runtime logs]
  Validator --> Logs
  Status --> Clients[API CLI and console]

Loading

Technical review

Best possible solution:

Keep the canonical current contract unless omitting unknown details has independently established value for supported workflows.

Do we have a high-confidence way to reproduce the issue?

The pre-change reader rejects UNRECOGNIZED while main writes and accepts it; source confirms that cross-version mismatch, but no supported current-version failure was reproduced.

Is this the best way to solve the issue?

Omitting the detail preserves masking, but backward readability alone does not justify replacing the current contract under this repository's compatibility policy.

Full review comments:

• [P3] Remove the frozen development-reader compatibility test — tests/conformance/contracts.test.mjs:688-696
  This test copies the pre-consolidation reader and requires future writes to satisfy it. AGENTS.md explicitly rejects preserving development helpers or fixture shapes solely for backward compatibility, and the reviewer who requested this requirement withdrew it. Consequently, a valid current-contract sentinel fails this test despite working with the supported reader. Remove this historical contract assertion and retain the adjacent current writer/read-back coverage.
  Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against becb62e9faad.

Provenance checked

• Unknown PROCESS_EXIT normalization changes intended behavior with a stated reason (787bb97: refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 replaced arbitrary shaped tokens with a bounded sentinel while preserving read-back and retry information.)
• Retry marker on unknown process details changes intended behavior with a stated reason (04d64a9: fix(runtime): retry a transient startup model probe once and name the held failure #2639 used -attempt2 to identify a failed retried startup probe; this PR explicitly acknowledges losing it for unknown details.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

• Applicable repository policy: Read the complete root AGENTS.md; no nested instruction files or maintainer-notes directory were found. The compatibility boundary rejects preserving development-era helpers or fixtures solely for backward compatibility. No target code, tests, or additional reviewer ran; the checkout remained clean. (AGENTS.md:340, b18d15335889)
• Pinned introduced change: The introduced production branch drops only shaped PROCESS_EXIT tokens outside the closed vocabulary; recognized details and existing sentinel acceptance remain intact. (packages/occ/src/runtime-failure-cause.ts:73, b18d15335889)
• Compatibility request withdrawn: freeqaz-openai explicitly withdrew the earlier requirement in refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 (comment) and reiterated in fix(occ): omit an unrecognized process-exit detail so earlier controllers read it #2769 (comment) that this PR is not owed to satisfy that review.
• Supported upgrade lifecycle: The upgrade implementation scales both API and worker to zero and waits for their Pods to terminate before migration; it does not establish mixed-version controller operation as the supported upgrade path. (scripts/upgrade-production-images:861, b18d15335889)
• Existing-state compatibility: The database already permits a cause without detail, the existing reader handles that shape, and this PR retains UNRECOGNIZED and UNRECOGNIZED-attempt2 acceptance; no schema migration or persisted-row rewrite is introduced. (packages/occ/src/state/postgres-schema.ts:1217, b18d15335889)
• Prior intent and feature history: refactor(contracts): one runtime failure vocabulary for the runtime and OCC #2763 deliberately retained a bounded sentinel and retry distinction; fix(runtime): retry a transient startup model probe once and name the held failure #2639 established the retry suffix. Scoped history and blame identify freeqaz across these changes and the earlier shared validator. Some older objects were unavailable locally, limiting deeper introduction tracing. (packages/occ/src/runtime-failure-cause.ts:27, becb62e9faad)

Labels

Label changes:

• add P3: The demonstrated mismatch concerns an earlier development controller; no supported current-controller outage or affected release is established.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P3: The demonstrated mismatch concerns an earlier development controller; no supported current-controller outage or affected release is established.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 17, 2026, 2:04 PM ET / 18:04 UTC.