fix(extension): invalidate ended browser task approvals - #5698
Open
iscekic wants to merge 9 commits into
Open
Conversation
This was referenced Aug 29, 2026
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (12 files)
Reviewed by grok-4.6 · Input: 513.7K · Output: 38.7K · Cached: 1.9M Review guidance: REVIEW.md from base branch |
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
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.
Summary
DelegatedApprovalScopeadds trustedisLiveandexecutionGuardcallbacks torequestApproval;PendingApprovalEntryrequires normalized drafts, and persisted metadata grants no delegated authority.applyApprovalDecisionreturnsabortedfor expired, cancelled, replayed, or unproven reloaded approvals and rechecks authority before writes; Stop cannot undo an issued save.settleMatchingApprovalkeeps failed live delegated approvals pending for explicit retry, while terminal settlement clears only matching approval state.Files
apps/extension/entrypoints/sidepanel/pending-approval.ts— Source, modified, 570 changed lines. Generates fresh approval IDs, validates draft kinds, and adds expiry timers with first-wins settlement. Keeps one approval lock across settings reads and draft persistence; automatic approval retains the settings fallback. Preserves local cleanup errors, but delegated cleanup failures cannot retract a successful save. Prevents retained callbacks from clearing another request’s entry or lock.apps/extension/entrypoints/sidepanel/pending-approval.test.ts— Test, modified, 369 changed lines. Updates the approval lifecycle tests.ApprovalOriginusesapprovalOriginSchemato distinguish local drafts fromDelegatedApprovalOrigin, whosedelegatedApprovalOriginSchemarequiresinvocationId,approvalId, andexpiresAt.PendingAgentMemoryDraftandPendingAgentWorkflowDraftkeep optional origins;NormalizedPendingAgentMemoryDraftandNormalizedPendingAgentWorkflowDraftrequire them.pendingAgentMemoryDraftSchemaandpendingAgentWorkflowDraftSchemadefault absent origins to local, preserving existing drafts and background selections without migration.Files
apps/extension/src/shared/agent-memories.ts— Source, modified, 33 changed lines. Defines origin validation, exact delegated approval matching, and normalized memory drafts. Requires nonempty delegated IDs and a nonnegative integer expiry;buildPendingMemoryDraftpasses through an optional origin.apps/extension/src/shared/agent-workflows.ts— Source, modified, 8 changed lines. Shares origin validation and adds the normalized workflow draft type while preserving optional producer metadata.withPendingDraftStorageLockserializes draft access with a queue and Web Locks; the fallback retains local, in-context behavior.addAgentMemory,addAgentWorkflow,updateAgentWorkflow,savePendingAgentMemoryDraft, andsavePendingWorkflowDraftaccept optionalExecutionGuardcallbacks immediately before writes.clearPendingAgentMemoryDraftandclearPendingWorkflowDraftaccept an expectedDelegatedApprovalOrigin, preventing stale cleanup from deleting replacements; old callers still clear unconditionally.Files
apps/extension/src/shared/agent-memories-storage.ts— Source, modified, 110 changed lines. Adds the shared queue and native lock, preserves origins in serialization, and returns normalized drafts. Guards saves after awaited preparation and serializes comparison with removal, including writes from background selections.apps/extension/src/shared/agent-workflows-storage.ts— Source, modified, 89 changed lines. Uses the shared lock for draft reads, saves, and conditional removal. Preserves origins, returns normalized drafts, and guards workflow creation, updates, and draft writes.apps/extension/src/shared/agent-memories-storage.test.ts— Test, modified, 126 changed lines. Updates the memory draft persistence tests.apps/extension/src/shared/agent-workflows-storage.test.ts— Test, modified, 99 changed lines. Updates the workflow draft persistence tests.PendingMemorySaveCardandPendingWorkflowSaveCarduseapprovalDraftKey,isApprovalDraftLive, anddiscardInactiveApprovalDraftto hide stale delegated cards and protect newer drafts. Empty reloads clear old workflow forms, and save failures keep retry or rejection controls while the delegated invocation remains live. Local drafts still reload, background memory selections keep priority, and per-draft save locks prevent repeated clicks from starting duplicate saves.Files
apps/extension/entrypoints/sidepanel/pending-memory-save-card.tsx— Source, modified, 168 changed lines. Normalizes stored drafts, prefers the active entry when delegation is involved, and reads decisions from the component’s atom store. Resets note, error, and busy state by draft identity; ignores older save or cancellation results. Renders the supervision slot in every visible state, including saved confirmation.apps/extension/entrypoints/sidepanel/pending-workflow-save-card.tsx— Source, modified, 198 changed lines. Rechecks liveness after asynchronous loads and clears form, error, comparison, and busy state when drafts disappear or change. Ignores superseded loads and decisions, retains explicit recovery after live delegated save failures, and renders the supervision slot throughout the modal.apps/extension/entrypoints/sidepanel/pending-workflow-save-card.test.ts— Test, modified, 561 changed lines. Updates the approval card tests.BrowserTaskSupervisionContextdefaults tonull;BrowserTaskSupervisionSlotplaces supplied content inside both approval dialogs without adding a wrapper or spacing. The slot remains available during editing, saving, errors, full storage, and memory confirmation, so supervision stays within the modal boundary. This level adds no provider controls and does not enable complete delegated execution.Files
apps/extension/entrypoints/sidepanel/browser-task-supervision-slot.tsx— Source, added, 8 changed lines. Adds the shared context and slot; an absent provider renders nothing and leaves the local layout unchanged.Tests: 4 modified files—
pending-approval.test.ts,pending-workflow-save-card.test.ts,agent-memories-storage.test.ts, andagent-workflows-storage.test.ts—with 1,155 changed lines.Generated: 0 files changed.
Verification
Manual browser verification has not run; this level does not enable the complete provider feature. Live browser verification remains pending, and the handoff includes no end-to-end (E2E) report.
Visual Changes
Visual Changes: N/A
Reviewer Notes
Human steps
This level needs no new environment values, secrets, data migration, feature flags, or cache resets.
Context
browser-task-0787-s8tobrowser-task-0787-s9, not the complete section.Notes
Live browser verification is pending. Native Chrome and Firefox behavior, the real CLI, and the local relay remain required before human-ready.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Runtime verification (E2E, user advocacy, simplify) runs on the tip PR over every level.
Every level keeps its own checks, its own bot review, and its own threads; each one is answered on its own PR.
Each level is its own deliverable: it builds and passes its own checks alone.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
browser-task-0787— feat(browser-task): define negotiated job contracts #5638browser-task-0787-s2— feat(session-ingest): persist owned browser jobs #5644browser-task-0787-s3— feat(session-ingest): route fenced browser providers #5648browser-task-0787-s4— feat(cloud-agent-sdk): support browser provider jobs #5653browser-task-0787-s7— fix(extension): expose honest browser runner outcomes #5681browser-task-0787-s8— feat(extension): coordinate browser execution across panels #5694browser-task-0787-s9— fix(extension): invalidate ended browser task approvals #5698 ← this PRbrowser-task-0787-s10— feat(extension): persist browser jobs and profile consent #5702 (tip)