fix: port permission idempotency model to question replies#515
Open
omercnet wants to merge 1 commit into
Open
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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)