fix: preserve legacy Architect clarification storage - #309
Conversation
…covery fix: recover timestamp-only legacy claims
…isher-reuse fix: reuse the production task event publisher
…d-answers fix: make protected answer batches atomic
…n-inventory fix: certify protected clarification S4 objects
b1c99d6
into
architecture/issue-179-context-packet-evidence
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94396441a5
ℹ️ 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".
| const { updatedQuestions, allAnswered } = result | ||
|
|
||
| if (allAnswered) { | ||
| await redis.lpush('forge:answers', JSON.stringify({ taskId })) |
There was a problem hiding this comment.
When Redis is transiently unavailable, the legacy transaction has already persisted the encrypted answers and marked the questions answered before this LPUSH fails. The request then returns 500 without creating the job that the Forge worker consumes; resubmitting is rejected because the envelope is already answered, leaving the task indefinitely in awaiting_answers. Use a durable outbox or allow an idempotent retry to enqueue an already-completed answer set.
AGENTS.md reference: AGENTS.md:L30-L32
Useful? React with 👍 / 👎.
| claimLeaseFence.assertOwned() | ||
| const storage = architectPlanStorageConfiguration(process.env, runtimeMode) | ||
| const answeredQuestions = storage.mode === 'legacy' | ||
| ? await legacyAnsweredQuestionSnapshot(taskId, existingQuestions, claimLeaseFence) |
There was a problem hiding this comment.
When legacy history cannot be loaded—for example, after a retryable replan inserted a newer artifact but failed before completing—the awaited snapshot throws before the recovery try block. Every subsequent attempt, including the final one, therefore bypasses the status handling below; the answers job can be dead-lettered while the task remains permanently awaiting_answers with no usable questions. Move this preparation into the guarded recovery path or apply equivalent terminal-state handling.
Useful? React with 👍 / 👎.
| : { | ||
| id: question.questionId, | ||
| taskId, | ||
| status: 'open' as const, |
There was a problem hiding this comment.
When a legacy replan reaches persistOpenQuestions, these rows have no answerReferenceId, so the function's existing null-reference deletion removes the prior answered projection before later operations complete. If workforce materialization, run completion, or another later step fails on a non-final attempt, recovery resets the task to awaiting_answers but does not restore those rows; the next retry either fails for having no questions or treats a newly generated unanswered round as the requested continuation. Make artifact/question replacement transactional or restore the prior answered projection and artifact binding.
Useful? React with 👍 / 👎.
1 participant
Source Issue
Issue: #179
Summary
This child addresses PR #198 finding
PRRT_kwDOS9dWe86U9jdu/discussion_r3679390466, including the follow-up lifecycle review.Exact stack
90763b271438559ba5b1481ded8aee7746792636362e41939e3028d891e9d75898b9333c48b902ab70f8d69cf8df0a0ec96ca5d980d322ac90bfd17dFiles
web/lib/mcps/legacy-clarification.tsweb/worker/orchestrator.tsweb/app/api/tasks/[id]/questions/route.tsweb/app/dashboard/tasks/[id]/page.tsxweb/db/migrations/0027_epic_172_s4_packet_context.sqlweb/db/schema.tsweb/__tests__/api.test.tsweb/__tests__/architect-legacy-clarification.test.tsweb/__tests__/task-page-retry-handoff.test.tsValidation
npx tsc --noEmit: passedgit diff --check: passedlegacy_unavailablemade the focused lifecycle test fail at the requiredopenstate, then the restored tree passedHosted evidence
30519224986, job90795736915— success30558198109, job90924049153— success90795731401— successNo parent review thread was replied to or resolved. This PR remains draft.