Repository navigation
feat(backend): durable first-refusal clock for producer-pending seals - #24724
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
1 issue found across 16 files
Confidence score: 4/5
config/feature-flags.yamldocuments anops_killrollback that isn’t available through the repository’s runtime or chart settings. Wire the flag into the pusher/backend and sweeper configs.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="config/feature-flags.yaml">
<violation number="1" location="config/feature-flags.yaml:38">
P2: This `ops_kill` flag is missing from the runtime environment and chart configs, so the documented rollback cannot be applied through the repository's deployment settings. Wire it into the pusher/backend and sweeper configs with a default of `true`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| - {"key":"OMI_FIRESTORE_WRITE_PROBE_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; count committed Firestore writes by collection, op, caller and origin at the GAPIC Commit/BatchWrite boundary","fail":"open","owner":"fswrites","created":"2026-11-01","terminal":"keep","decision":"keep","notes":"Only the literal false skips installing the write probe in that process, so no labelled write series are created. Set false on a Cloud Run service whose max scrape is near its GMP sidecar samples limit; observe-only, no data path."} | ||
| - {"key":"RECORD_AUDIO_PARTIAL_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; finish unprovable stored recordings explicitly partial after 30 minutes and admit serial late-audio fills","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"ADR 0007; David 2026-10-31 and lead design acceptance 7e2a57. Literal false stops new partial finishes and CPU continuation admissions. Existing accepted worker jobs drain normally. No live-token age expiry, complete-proof relaxation, audio deletion or hard-3 threshold change. Set on repair and CPU drainer hosts for rollback; active repair Jobs retain startup config. Lead applies paired census patch with release."} | ||
| - {"key":"PUSHER_PROJECTION_QUEUE_WAIT_BOUND_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; bound only committed pusher projection queue capacity waits to the existing 300-second receive-liveness budget","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"Only literal false restores unbounded queue-capacity waiting for new sockets. Committed receipts retain a single-slot projection owner before ACK and capacity waiting. Queue plus slot must drain before producer clear; normal expired socket drains transfer to the capped process owner. Admission failure keeps the fence. Measurement and ACK departure delivery remain active. No idle reclaim, custody deadline, listen change or token age release. Lead owns pusher deploy and scrape verification."} | ||
| - {"key":"RECORD_SEAL_REFUSED_STAMP_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; persist first producer-pending seal refusal and observe its bounded sweeper age","fail":"open","owner":"omi-lifecycle","created":"2026-11-06","terminal":"keep","decision":"keep","notes":"Phase 1 approved by lead2 bus 430238. False disables refusal stamps and sweeper observations; successful seal still clears an existing stamp. No token, custody or seal-authority change. Existing stamps need no cleanup on rollback."} |
There was a problem hiding this comment.
P2: This ops_kill flag is missing from the runtime environment and chart configs, so the documented rollback cannot be applied through the repository's deployment settings. Wire it into the pusher/backend and sweeper configs with a default of true.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At config/feature-flags.yaml, line 38:
<comment>This `ops_kill` flag is missing from the runtime environment and chart configs, so the documented rollback cannot be applied through the repository's deployment settings. Wire it into the pusher/backend and sweeper configs with a default of `true`.</comment>
<file context>
@@ -35,6 +35,7 @@ flags:
- {"key":"OMI_FIRESTORE_WRITE_PROBE_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; count committed Firestore writes by collection, op, caller and origin at the GAPIC Commit/BatchWrite boundary","fail":"open","owner":"fswrites","created":"2026-11-01","terminal":"keep","decision":"keep","notes":"Only the literal false skips installing the write probe in that process, so no labelled write series are created. Set false on a Cloud Run service whose max scrape is near its GMP sidecar samples limit; observe-only, no data path."}
- {"key":"RECORD_AUDIO_PARTIAL_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; finish unprovable stored recordings explicitly partial after 30 minutes and admit serial late-audio fills","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"ADR 0007; David 2026-10-31 and lead design acceptance 7e2a57. Literal false stops new partial finishes and CPU continuation admissions. Existing accepted worker jobs drain normally. No live-token age expiry, complete-proof relaxation, audio deletion or hard-3 threshold change. Set on repair and CPU drainer hosts for rollback; active repair Jobs retain startup config. Lead applies paired census patch with release."}
- {"key":"PUSHER_PROJECTION_QUEUE_WAIT_BOUND_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; bound only committed pusher projection queue capacity waits to the existing 300-second receive-liveness budget","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"Only literal false restores unbounded queue-capacity waiting for new sockets. Committed receipts retain a single-slot projection owner before ACK and capacity waiting. Queue plus slot must drain before producer clear; normal expired socket drains transfer to the capped process owner. Admission failure keeps the fence. Measurement and ACK departure delivery remain active. No idle reclaim, custody deadline, listen change or token age release. Lead owns pusher deploy and scrape verification."}
+ - {"key":"RECORD_SEAL_REFUSED_STAMP_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; persist first producer-pending seal refusal and observe its bounded sweeper age","fail":"open","owner":"omi-lifecycle","created":"2026-11-06","terminal":"keep","decision":"keep","notes":"Phase 1 approved by lead2 bus 430238. False disables refusal stamps and sweeper observations; successful seal still clears an existing stamp. No token, custody or seal-authority change. Existing stamps need no cleanup on rollback."}
- {"key":"RECORD_SEAL_OVERLAP_TRIM_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; earlier retained objects own divergent overlap frames in seal proof and admission manifests","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"David 2026-10-31 ruling A. False restores overlap byte-equality refusal. Worker chart recordSealRules.overlapTrimEnabled configures repair and sweeper; CPU admission uses the same env. Active Jobs retain startup config; stop them for immediate rollback. No tail tolerance change."}
- {"key":"RECORD_SEAL_UPLOAD_FAILED_REPAIR_ENABLED","aliases":[],"kind":"env","lifecycle":"ops_kill","surfaces":["backend"],"summary":"Default on; repair job may atomically clear upload_failed only after full normal proof and unchanged-state fencing","fail":"open","owner":"two-lane lead","created":"2026-10-31","terminal":"keep","decision":"keep","notes":"David 2026-10-31 ruling C. Repair-only explicit opt-in; sweeper and inline finish retain sticky refusal. Chart recordSealRules.uploadFailedRepairEnabled; false restores refusal. Roll all failure-marker writers before enabling repair, or repeated marks from old writers cannot invalidate the plan. Active Jobs retain startup config; stop them for immediate rollback."}
</file context>
Problem
Producer-pending seal refusal retained retry state only in pusher memory. A process death or legacy retry exhaustion lost the first-refusal age even though custody tokens remained durable.
What changes
Persist
record_seal_refused_atonce inside the existing refusal transaction, preserve it on retries, and clear it atomically on successfulseal_record_audioclosure. Emit bounded job-page refused count/oldest-age observations in the sweeper summary, declare the state/writer, and add strict transaction and emulator pod-death regressions. Include the changes document and all three flag registry files.Flag:
RECORD_SEAL_REFUSED_STAMP_ENABLED, default true, approved by lead2 (bus 430238). False restores zero-write refusal and disables sweeper observations. Successfulseal_record_audioclosures still clear prior stamps.Rollback: set the flag false on relevant processes, or revert the PR. Old stamps can remain ignored; no backfill or cleanup is needed.
Custody/privacy: one content-free epoch field, bounded to the first refusal update inside the existing transaction. No audio or identifiers added; no token, seal-authority, privacy-proof, retry-budget or
is_debtchanges. No.githubchanges, GCP tier, production access, merge or deploy.Validation
CLOUDSDK_PYTHONand reachable host ADC, then hit a shared-port collision. No credential preflight or assertion was bypassed; all owned children were shut down.make preflightran its checks but rejected tracked-file edits made concurrently during test refinement. Frozen-treemake preflightpassed all 43 checks (351.45s);scripts/pr-preflight --pr-body-filepassed all 43 checks (260.36s), and its metadata-only pass also passed. First normal pre-push passed all 43 manifest checks and its Firestore guard (149.81s), then caught an inferreddict[str, bool]type for the nullable clear. Added an explicitdict[str, bool | None]annotation; the second normal pre-push passed, including all 43 manifest checks, zero-error typecheck, seven selected backend test files and pinned formatting. Its Firestore guard passed in 199.34s with no timeout. No bypass, force push or custom cap was used.exec-address-clay-code-purchase-bonus-index, resolved by agentctl to Grok on Cursor. Accepted narrow monitoring scope; flagged proof-repair seal clearing outside the named files. Harness import placement corrected. Parent independently verified the finding.Read-side scope ruling
The refusal clock is meaningful only while
record_audio_closedis false. Closed stamped rows are stale metadata and are excluded before the sweeper updates backlog, refusal count or maximum refusal age. Regressions cover a closed stamp alone, an older closed stamp beside a newer open one, and a real proof-repair seal that retains the stamp but is ignored on the next sweep.Per uicleanup's chat decision, repair/nojob writers and seal authority remain untouched. The normal successful
seal_record_audiobranch still clears the timestamp in its existing transaction. The state table and changes doc now specify the unclosed-only read contract. The previous scope ask is withdrawn and no longer blocks readiness. A reopened row may still carry its original first-refusal epoch; this PR does not redefine that clock or change a writer to reset it.The required Grok review was completed on the original implementation; this follow-up implements the lead's read-side resolution of its retained-clock finding. No further scope extension is requested.
Read-side follow-up validation at
add5c6e1254cca208c32040f38792baa07f8315b:make preflight: all 43 checks passed (160.61s). PR metadata validation passed.Measurement limits
No production stock/age, deployed flags, legacy-drain prevalence, released-client field consumption or end-to-end production recovery was measured. Sweeper observations cover only its existing bounded
record_pendingjob page, before apply, and do not cover no-job rows. This PR emits structured summary samples, not a deployed external metric/alert. Strict expected failures preserve missing authoritative fleet gauge/committed alert and automatic requeue/deadline obligations; phase derivation stays unchanged.Failure-Class: none
Failure-Class-Reason: Phase 1 adds a durable age observation; it preserves existing retry and custody authority. Behavioral guards cover timestamp idempotence, atomic clearing, legacy exhaustion and process death.