PRODUCER-5A2: close core diagnostic outputs - #293
Conversation
…currence-recovery fix(queue): preserve occurrence identity across recovery
…4-clock fix(queue): make Redis authoritative for retry deadlines
…4-v2-cutover fix(queue): contain mixed-version retry promotion conflicts
…5-retry-reconcile fix(worker): reconcile ambiguous retry outcomes
…6-receipt-binding fix(queue): bind canonical promotion receipts
…etry-upgrade fix(queue): upgrade legacy retry members atomically
…rovider-snapshot fix(worker): persist Architect provider snapshots
…ested-fence fix(worker): fence nested handoff mutations after claim loss
…ence fix(worker): fence work after queue claim loss
…erminal-replay test(queue): prove renewed nonce terminal replay
…laim-renewal fix(queue): renew live claim leases
…utage-shutdown fix(worker): bound Redis outage shutdown
…ecovery-quarantine fix(worker): quarantine malformed recovery members
1f2bf58
into
architecture/issue-179-context-packet-evidence
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55dfc52f8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| unanswered: unanswered.length, | ||
| }) | ||
| return | ||
| return recoveredRunningOccurrence ? 'retained' : 'completed' |
There was a problem hiding this comment.
Advance tasks after recovered replans persist questions
If an answered-question Architect run persists a new unanswered-question round and then loses its queue lease before changing the task from running to awaiting_answers, the first recovered attempt returns retained here. The runtime marks that recovery attempt indeterminate while retaining the occurrence; on the next recovery there is no running attempt, so recoveredOccurrence is false and the earlier running guard also returns retained. The task therefore remains running and cycles through recovery indefinitely instead of exposing the newly persisted questions.
Useful? React with 👍 / 👎.
| input.assertClaimOwned?.() | ||
| await tx.insert(approvalGates).values(rows.approvalGate) | ||
| }) | ||
| input.assertClaimOwned?.() |
There was a problem hiding this comment.
Make committed materialization idempotent
If the claim expires after this transaction commits but before this assertion, processTask retains the occurrence and recovery reruns Architect materialization. For normal plans containing QA or Reviewer dependencies, the committed work_package_dependencies rows reference the pending packages through ON DELETE RESTRICT; the rerun attempts to delete those packages without deleting their dependencies, so PostgreSQL rejects every subsequent materialization attempt and an otherwise successful task eventually fails. The committed graph must be resumed or replaced atomically rather than treated as uncommitted work.
Useful? React with 👍 / 👎.
| await pauseClaimRenewal() | ||
| claimLeaseFence.assertOwned() | ||
| try { | ||
| await queue.ack(raw) |
There was a problem hiding this comment.
Do not ack before attempt finalization is durable
When the business operation succeeds but the PostgreSQL finishTaskAttempt update fails transiently, the catch only logs the failure and execution still reaches this acknowledgement. A successful ack permanently removes the Redis occurrence, leaving its durable attempt row in running with no occurrence available for adoption or reconciliation; the dashboard can consequently report an active worker forever. The queue item should be retained, or the attempt update explicitly reconciled, until its terminal status is durable.
Useful? React with 👍 / 👎.
| } catch (err) { | ||
| if (claimLeaseFence.lost || isClaimLeaseLostError(err)) { | ||
| claimLeaseFence.throwIfLost() | ||
| } |
There was a problem hiding this comment.
Close superseded Architect runs during adoption
After runArchitect has inserted an agentRuns(status = 'running') row, any mid-run lease loss takes this early exit and bypasses the only terminal update for that run. Exact-occurrence recovery starts a new Architect run but never supersedes the abandoned one, while the dashboard treats any returned running run as active; a task that later recovers or completes can therefore display perpetual Architect execution and accumulate stale active runs. The recovered owner should terminalize the superseded run as part of adoption.
Useful? React with 👍 / 👎.
| for (let page = 0; page < pageCount; page += 1) { | ||
| const pageSize = Math.min(remaining, STUCK_RECOVERY_SCAN_LIMIT) | ||
| if (pageSize < 1) break | ||
| const processing = await this.client.lrange(this.processingQueueKey, 0, pageSize - 1) |
There was a problem hiding this comment.
Read processing-list members as raw bytes
If the processing list contains a malformed member with invalid UTF-8 bytes, this string-returning lrange changes the member bytes before removeMalformedRecoveryMember passes them to Lua. LREM cannot match the stored value, the recovery script returns 0, and the new removal path throws, aborting every sweep that includes that member and preventing valid stale jobs in the scanned page from being recovered. Redis list members are binary-safe, so this scan needs the same buffer-based handling already used for retry members.
Useful? React with 👍 / 👎.
Source Issue
Issue: #179
Summary
This is the integrated producer-reliability stack for PR #198. It keeps queue work recoverable when Redis or a worker becomes unreliable, and it fails closed before a stale worker can make later mutations.
The child progression is #294 → #295 → #300 (which contains #301, #302, #303, and #304) → #305 → #306.
The integrated stack covers:
Validation
Approved exact head:
55dfc52f8ec3f36ba785f7ed53281fe403a444e1Base:
bb6269adc2a73e31a53c23ebc244dd06a6f88d4730489823289, job90704612525.30489823287, job90704612774.90704616721passed.The hosted queue evidence emitted all required markers:
QUEUE_OCCURRENCE_REDIS_MULTIPLICITY_OKQUEUE_OCCURRENCE_REDIS_QUARANTINE_OKQUEUE_OCCURRENCE_REDIS_RECOVERY_OKQUEUE_OCCURRENCE_REDIS_SHUTDOWN_OKQUEUE_OCCURRENCE_ADOPTION_TASK_OKQUEUE_OCCURRENCE_ADOPTION_ANSWERS_OKQUEUE_OCCURRENCE_ADOPTION_APPROVAL_OKQUEUE_OCCURRENCE_NEGATIVE_RECOVERY_OKReviewer approval was recorded for the exact integrated head above.
Accepted limitation
An external input/output operation that has already started cannot be revoked transactionally. Later Forge mutations remain fenced after authority is lost. This is an accepted limitation of the integrated design, not permission for a stale worker to continue.
Scope
This remains the bounded producer-reliability child stack of PR #198. The body records the integrated R3A–R3D evidence and does not claim production rollout proof beyond the hosted validation listed above.