fix: port permission idempotency model to question replies#515
Conversation
Eliminate opencode's "reply for unknown request" warning caused by orphaned/stale question replies in the CodeNomad UI. Previously the question request lifecycle lacked the idempotency and stale-guard layer that permissions already had, so a question popup answered after a reconnect, restart, or rapid double-submit could POST a reply for a request the backend no longer tracked. User-visible behavior change: - Answering a question prompt that has already expired no longer fails silently or mis-routes; the UI now surfaces a localized "this prompt expired, send your answer as a message instead" notice (new i18n key toolCall.question.errors.expired added to all 7 locales). - Rapid double-submit (Enter + button) can no longer fire two replies. - Reconnect/rehydrate no longer re-surfaces an already-answered question. Implementation: - New stores/question-replies.ts ledger (markQuestionReplied / hasRepliedQuestion / pruneRepliedQuestions / clearRepliedQuestions), a structural mirror of permission-replies.ts (no shared abstraction, per YAGNI; prune semantics may diverge). - sendQuestionReply/sendQuestionReject reconcile against question.list() before POSTing (no error-string parsing); on absent requestID they take the expired-prompt path instead of POSTing to a "root" fallback, and resolve the owning worktree when the stored slug is missing. markQuestionReplied is recorded on success before removeQuestionFromQueue. - syncPendingQuestions adopts the full timestamp-based prune+filter pattern so replied questions are never rehydrated. - The question ledger is cleared only on instance removal, never on rehydrate, so in-flight stale replies cannot slip through. - session-events stale guards on handleQuestionAsked/handleQuestionAnswered. - tool-call.tsx synchronous double-submit guard before the await boundary. Also fixes two pre-existing module-load crashes uncovered while making the UI test suite green (ownership policy): client-identity.ts dereferenced window storage before its typeof-window guard, and server-events.ts opened an EventSource at module load without an EventSource guard. Both fixes are minimal and production-safe. Validation: UI suite 49/49 pass (bun test --conditions browser), UI typecheck clean (tsc --noEmit). Evidence packet under evidences/058-question-reply-idempotency/.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26752782315 Artifacts expire in 7 days.
|
|
I want you to perform the gatekeeper checks for latest PR state check for regressions |
|
Gatekeeper review for latest PR state. Understood request: review PR #515 ruthlessly for regressions and better implementation options. Assumption: this is a review/check request only; I did not modify code. Findings
If every candidate client’s That is a real regression risk: a transient proxy/network/backend error during the reconcile probe can permanently retire an active question without posting the user’s answer. The code comment says a failed lookup is non-authoritative, but the final Suggested fix: track whether at least one
Quality Notes The overall design is directionally good: the ledger mirrors the permission model, the synchronous double-submit guard is in the right place, the i18n key is present in all configured locales, and the browser-only guards in I would not broaden this PR into full multi-worktree enumeration unless required; the current bounded candidate list is acceptable for the stated scope. The important correctness line is not treating transport failures as proof of expiration. Validation Run Passed:
Result: 13 pass, 0 fail. Could not fully validate in this workspace:
Result: 13 pass, 3 fail because
Result: failed with broad missing dependency errors ( Oversized Touched Files
-- |
|
Until we know how to reproduce this, we can't be sure this fixes issues. |
|
yes, happy to asisst - it happens often for me but now sure why |
3 participants
Problem
Answering a
questionprompt after a reconnect, process restart, or rapid double-submit could POST a reply for a request the backend no longer tracks, producing opencode'sreply for unknown requestwarning. The reply was either silently dropped or mis-routed to a fallback (root) target.Root cause: the
questionrequest lifecycle in the UI lacked the idempotency and stale-guard layer that thepermissionlifecycle already had. It reproduces even with a single root worktree (not primarily a worktree issue).Fix
Ports the existing permission idempotency model to questions (entirely
packages/ui, no opencode core changes):stores/question-replies.tsledger (markQuestionReplied/hasRepliedQuestion/pruneRepliedQuestions/clearRepliedQuestions) — a structural mirror ofpermission-replies.ts. No shared abstraction (YAGNI; prune semantics may diverge).sendQuestionReply/sendQuestionRejectcheckquestion.list()before POSTing. If the request is no longer pending, the UI surfaces a localized "this prompt expired — send your answer as a message instead" notice rather than POSTing to a fallback. No error-string parsing.syncPendingQuestionsadopts the full timestamp-based prune +hasRepliedQuestionfilter so replied questions are never rehydrated on reconnect.rehydrateInstance(only on instance removal), so in-flight stale replies can't slip through.handleQuestionAsked/handleQuestionAnswered.handleQuestionSubmit(Enter + button can no longer both fire).toolCall.question.errors.expiredadded to all 7 locales.Also fixes two pre-existing module-load crashes uncovered while making the UI suite green:
client-identity.tsdereferencedwindowstorage before itstypeof windowguard, andserver-events.tsopened anEventSourceat module load without a guard. Both minimal and production-safe.User-visible behavior
Validation
bun test --conditions browser)tsc --noEmit)origin/dev(upstream default branch)