mirror of
https://github.com/NeuralNomadsAI/CodeNomad.git
synced 2026-07-10 00:14:27 +00:00
fix(ui): ignore stale permission events after reply (#425)
## Summary - Split out from #422 as the stale-permission-events fix. - Clears permission UI state immediately after a successful local permission reply instead of waiting for `permission.replied` SSE. - Tracks replied permission IDs until a newer pending-permission sync observes that the server no longer reports them pending. - Marks SSE `permission.replied` events into the same replied-ID path so delayed pending events/sync results cannot resurrect prompts that were just answered. ## Why A permission prompt could remain on screen after clicking Allow/Deny, or clicking Allow could look like it did not take effect and require another click. Reloading could fix the UI, which pointed to stale local permission state rather than the server still waiting. The UI receives permission state from local replies, SSE events, and pending-permission sync. If an older pending event or sync result is processed after a confirmed reply, the UI can re-add a permission that was already answered. Replied IDs stay suppressed until a sync started after the local reply proves the server has dropped that permission from the pending list. ## Validation - `git diff --check` - `node --test packages/ui/src/stores/permission-replies.test.ts` - `npm run typecheck --workspace @codenomad/ui`
This commit is contained in:
parent
8976d9985b
commit
4090989d12
4 changed files with 112 additions and 4 deletions
|
|
@ -32,6 +32,12 @@ import { setSessionPendingPermission, setSessionPendingQuestion } from "./sessio
|
|||
import { setHasInstances } from "./ui"
|
||||
import { messageStoreBus } from "./message-v2/bus"
|
||||
import { upsertPermissionV2, removePermissionV2, upsertQuestionV2, removeQuestionV2 } from "./message-v2/bridge"
|
||||
import {
|
||||
clearRepliedPermissions,
|
||||
hasRepliedPermission,
|
||||
markPermissionReplied,
|
||||
pruneRepliedPermissions,
|
||||
} from "./permission-replies"
|
||||
import { clearAutoAcceptPermission, drainAutoAcceptPermissions, isPermissionAutoAcceptEnabled, togglePermissionAutoAccept } from "./permission-auto-accept"
|
||||
import { clearCacheForInstance } from "../lib/global-cache"
|
||||
import { getLogger } from "../lib/logger"
|
||||
|
|
@ -79,6 +85,7 @@ function syncHasInstancesFlag() {
|
|||
const readyExists = Array.from(instances().values()).some((instance) => instance.status === "ready")
|
||||
setHasInstances(readyExists)
|
||||
}
|
||||
|
||||
interface DisconnectedInstanceInfo {
|
||||
id: string
|
||||
folder: string
|
||||
|
|
@ -187,12 +194,17 @@ async function syncPendingPermissions(instanceId: string): Promise<void> {
|
|||
if (!instance?.client) return
|
||||
|
||||
try {
|
||||
const syncStartedAt = Date.now()
|
||||
const remote = await requestData<PermissionRequestLike[]>(
|
||||
instance.client.permission.list(),
|
||||
"permission.list",
|
||||
)
|
||||
|
||||
const remoteIds = new Set(remote.map((item) => item.id))
|
||||
const remotePendingIds = new Set(remote.map((item) => item.id))
|
||||
pruneRepliedPermissions(instanceId, remotePendingIds, syncStartedAt)
|
||||
|
||||
const pendingRemote = remote.filter((item) => !hasRepliedPermission(instanceId, item.id))
|
||||
const remoteIds = new Set(pendingRemote.map((item) => item.id))
|
||||
const local = getPermissionQueue(instanceId)
|
||||
|
||||
// Remove any stale local permissions missing from server.
|
||||
|
|
@ -204,7 +216,7 @@ async function syncPendingPermissions(instanceId: string): Promise<void> {
|
|||
}
|
||||
|
||||
// Upsert all server-side pending permissions.
|
||||
for (const permission of remote) {
|
||||
for (const permission of pendingRemote) {
|
||||
addPermissionToQueue(instanceId, permission)
|
||||
upsertPermissionV2(instanceId, permission)
|
||||
}
|
||||
|
|
@ -514,6 +526,7 @@ function removeInstance(id: string) {
|
|||
removeLogContainer(id)
|
||||
clearCommands(id)
|
||||
clearPermissionQueue(id)
|
||||
clearRepliedPermissions(id)
|
||||
clearQuestionQueue(id)
|
||||
clearInstanceMetadata(id)
|
||||
|
||||
|
|
@ -1054,8 +1067,11 @@ async function sendPermissionResponse(
|
|||
"permission.reply",
|
||||
)
|
||||
|
||||
// Remove from queue after successful response
|
||||
markPermissionReplied(instanceId, requestId)
|
||||
// Remove from both local queues after successful response; the SSE replied event
|
||||
// is still accepted, but the UI no longer depends on receiving it.
|
||||
removePermissionFromQueue(instanceId, requestId)
|
||||
removePermissionV2(instanceId, requestId)
|
||||
} catch (error) {
|
||||
log.error("Failed to send permission response", error)
|
||||
throw error
|
||||
|
|
@ -1150,6 +1166,8 @@ export {
|
|||
getPermissionQueueLength,
|
||||
addPermissionToQueue,
|
||||
removePermissionFromQueue,
|
||||
markPermissionReplied,
|
||||
hasRepliedPermission,
|
||||
togglePermissionAutoAcceptForSession,
|
||||
clearPermissionQueue,
|
||||
sendPermissionResponse,
|
||||
|
|
|
|||
44
packages/ui/src/stores/permission-replies.test.ts
Normal file
44
packages/ui/src/stores/permission-replies.test.ts
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
import assert from "node:assert/strict"
|
||||
import { describe, it } from "node:test"
|
||||
|
||||
import {
|
||||
clearRepliedPermissions,
|
||||
hasRepliedPermission,
|
||||
markPermissionReplied,
|
||||
pruneRepliedPermissions,
|
||||
} from "./permission-replies.ts"
|
||||
|
||||
describe("replied permission tracking", () => {
|
||||
it("keeps replied ids when an older sync does not include them", () => {
|
||||
const instanceId = "instance-old-sync"
|
||||
const permissionId = "permission-1"
|
||||
|
||||
markPermissionReplied(instanceId, permissionId, 1_000)
|
||||
pruneRepliedPermissions(instanceId, new Set(), 900)
|
||||
|
||||
assert.equal(hasRepliedPermission(instanceId, permissionId), true)
|
||||
clearRepliedPermissions(instanceId)
|
||||
})
|
||||
|
||||
it("keeps replied ids while the server still reports them pending", () => {
|
||||
const instanceId = "instance-still-pending"
|
||||
const permissionId = "permission-1"
|
||||
|
||||
markPermissionReplied(instanceId, permissionId, 1_000)
|
||||
pruneRepliedPermissions(instanceId, new Set([permissionId]), 1_100)
|
||||
|
||||
assert.equal(hasRepliedPermission(instanceId, permissionId), true)
|
||||
clearRepliedPermissions(instanceId)
|
||||
})
|
||||
|
||||
it("clears replied ids once a newer sync observes them missing", () => {
|
||||
const instanceId = "instance-new-sync"
|
||||
const permissionId = "permission-1"
|
||||
|
||||
markPermissionReplied(instanceId, permissionId, 1_000)
|
||||
pruneRepliedPermissions(instanceId, new Set(), 1_100)
|
||||
|
||||
assert.equal(hasRepliedPermission(instanceId, permissionId), false)
|
||||
clearRepliedPermissions(instanceId)
|
||||
})
|
||||
})
|
||||
38
packages/ui/src/stores/permission-replies.ts
Normal file
38
packages/ui/src/stores/permission-replies.ts
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
const repliedPermissionIdsByInstance = new Map<string, Map<string, number>>()
|
||||
|
||||
function pruneRepliedPermissions(instanceId: string, remotePendingIds: Set<string>, syncStartedAt: number): void {
|
||||
const replied = repliedPermissionIdsByInstance.get(instanceId)
|
||||
if (!replied) return
|
||||
for (const [permissionId, repliedAt] of replied) {
|
||||
// Only a sync started after the local reply can prove the server no longer
|
||||
// considers this permission pending.
|
||||
if (!remotePendingIds.has(permissionId) && syncStartedAt >= repliedAt) {
|
||||
replied.delete(permissionId)
|
||||
}
|
||||
}
|
||||
if (replied.size === 0) {
|
||||
repliedPermissionIdsByInstance.delete(instanceId)
|
||||
}
|
||||
}
|
||||
|
||||
function markPermissionReplied(instanceId: string, permissionId: string, repliedAt = Date.now()): void {
|
||||
if (!permissionId) return
|
||||
let replied = repliedPermissionIdsByInstance.get(instanceId)
|
||||
if (!replied) {
|
||||
replied = new Map()
|
||||
repliedPermissionIdsByInstance.set(instanceId, replied)
|
||||
}
|
||||
replied.set(permissionId, repliedAt)
|
||||
}
|
||||
|
||||
function hasRepliedPermission(instanceId: string, permissionId: string): boolean {
|
||||
const replied = repliedPermissionIdsByInstance.get(instanceId)
|
||||
if (!replied) return false
|
||||
return replied.has(permissionId)
|
||||
}
|
||||
|
||||
function clearRepliedPermissions(instanceId: string): void {
|
||||
repliedPermissionIdsByInstance.delete(instanceId)
|
||||
}
|
||||
|
||||
export { clearRepliedPermissions, hasRepliedPermission, markPermissionReplied, pruneRepliedPermissions }
|
||||
|
|
@ -35,6 +35,8 @@ import {
|
|||
instances,
|
||||
addPermissionToQueue,
|
||||
removePermissionFromQueue,
|
||||
markPermissionReplied,
|
||||
hasRepliedPermission,
|
||||
addQuestionToQueue,
|
||||
removeQuestionFromQueue,
|
||||
} from "./instances"
|
||||
|
|
@ -689,8 +691,13 @@ function handleTuiToast(_instanceId: string, event: TuiToastEvent): void {
|
|||
function handlePermissionUpdated(instanceId: string, event: { type: string; properties?: PermissionRequestLike } | any): void {
|
||||
const permission = event?.properties as PermissionRequestLike | undefined
|
||||
if (!permission) return
|
||||
const permissionId = getPermissionId(permission)
|
||||
if (permissionId && hasRepliedPermission(instanceId, permissionId)) {
|
||||
log.info(`[SSE] Ignoring stale permission request after local reply: ${permissionId}`)
|
||||
return
|
||||
}
|
||||
|
||||
log.info(`[SSE] Permission request: ${getPermissionId(permission)} (${getPermissionKind(permission)})`)
|
||||
log.info(`[SSE] Permission request: ${permissionId} (${getPermissionKind(permission)})`)
|
||||
addPermissionToQueue(instanceId, permission)
|
||||
upsertPermissionV2(instanceId, permission)
|
||||
|
||||
|
|
@ -710,6 +717,7 @@ function handlePermissionReplied(instanceId: string, event: { type: string; prop
|
|||
if (!requestId) return
|
||||
|
||||
log.info(`[SSE] Permission replied: ${requestId}`)
|
||||
markPermissionReplied(instanceId, requestId)
|
||||
removePermissionFromQueue(instanceId, requestId)
|
||||
removePermissionV2(instanceId, requestId)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue