Skip to content

feat(frontend): offline-aware API queue for retryable actions (#241)#298

Open
Shikhar-404exe wants to merge 25 commits into
utksh1:mainfrom
Shikhar-404exe:241
Open

feat(frontend): offline-aware API queue for retryable actions (#241)#298
Shikhar-404exe wants to merge 25 commits into
utksh1:mainfrom
Shikhar-404exe:241

Conversation

@Shikhar-404exe

@Shikhar-404exe Shikhar-404exe commented May 24, 2026

Copy link
Copy Markdown
Contributor

Offline-Aware Queue System #241

Adds offline support for retryable write actions by queueing requests during connectivity loss and replaying them automatically when the connection returns.

Highlights

  • Added offlineQueue.ts service with:
    • enqueue
    • retry
    • retryAll
    • remove
    • clear
    • subscribe
  • Persists queue state in localStorage
  • Detects online/offline changes automatically

React Integration

Added OfflineQueueContext.tsx to provide:

  • isOnline
  • pendingCount
  • queue
  • queue action handlers

Integrated globally using <OfflineQueueProvider>.

API Changes

Enhanced request<T>() with:

  • retryable
  • label

Retryable mutations are queued while offline and replayed later.

Supported actions:

  • startTask
  • deleteTask
  • bulkDeleteTasks
  • clearAllTasks
  • cancelTask
  • createWorkflow
  • runWorkflow
  • updateWorkflow
  • deleteWorkflow

Read-only GET requests remain non-retryable.

UI

Added OfflineQueueIndicator in Sidebar.tsx featuring:

  • Offline status indicator
  • Pending queue count
  • Queue dropdown
  • “Retry All” action
  • Individual item removal

Testing

Added comprehensive coverage:

  • 17 queue service tests
  • 7 context tests
  • 15 API offline integration tests

@utksh1 utksh1 added area:frontend Frontend React/UI work type:feature Feature work category bonus label level:advanced 55 pts difficulty label for advanced contributor PRs labels May 26, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes. The offline queue currently treats destructive/non-idempotent actions such as delete, clear, cancel, run workflow, and delete workflow as replayable. Please limit queuing to safe/idempotent mutations or add explicit confirmation/conflict handling before replay, plus tests for stale task/workflow state.

@Shikhar-404exe Shikhar-404exe requested a review from utksh1 May 26, 2026 10:37
@Shikhar-404exe Shikhar-404exe force-pushed the 241 branch 2 times, most recently from d7e19d3 to 8cac9e8 Compare May 26, 2026 11:06

@Shikhar-404exe Shikhar-404exe left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@Shikhar-404exe Shikhar-404exe force-pushed the 241 branch 2 times, most recently from 4e03477 to 8cac9e8 Compare May 27, 2026 04:42
@utksh1

utksh1 commented May 28, 2026

Copy link
Copy Markdown
Owner

Thanks for following up. Clarifying the change request so it is actionable:

Why this is blocked:
Requesting changes. The offline queue currently treats destructive/non-idempotent actions such as delete, clear, cancel, run workflow, and delete workflow as replayable. Please limit queuing to safe/idempotent mutations or add explicit confirmation/conflict handling before replay, plus tests for stale task/workflow state.

What to do next:

  • Fix the specific issues called out above.
  • Push the updated branch and make sure the relevant CI checks pass.
  • Reply here when ready for re-review.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed after your update, but the offline queue still queues non-idempotent actions. startTask, createWorkflow, and updateWorkflow can create duplicate or stale server state when replayed automatically. Please restrict auto-replay to genuinely idempotent/safe operations or require explicit user confirmation with conflict checks before replay.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed latest state. The key blocker still stands: automatic offline replay must not include non-idempotent actions like startTask/createWorkflow/updateWorkflow without explicit user confirmation and conflict checks. Please restrict auto-replay to safe/idempotent operations or add a reviewed replay flow for mutating actions.

@Shikhar-404exe Shikhar-404exe requested a review from utksh1 May 31, 2026 04:59
@utksh1

utksh1 commented May 31, 2026

Copy link
Copy Markdown
Owner

Re-reviewed after the latest push. Still blocked: the offline action queue needs product-safe retry boundaries, clear persistence/error handling, and tests proving non-idempotent or unsafe actions are not silently replayed after reconnect.

…#241)

- Queue service: enqueue, retry(id), retryAll, remove, clear,
  persist to localStorage, auto-replay on reconnect
- Stale-state handling: 404/410 removes action from queue,
  maxRetries exceeded drops action
- React context provider wrapping queue service with isOnline,
  pendingCount, queue state
- API integration: only safe/idempotent mutations (startTask,
  createWorkflow, updateWorkflow) tagged retryable;
  destructive/non-idempotent actions (delete, cancel, clear, run)
  are NOT queued
- Sidebar indicator: cloud_off/cloud_sync icon, pending count badge,
  dropdown with item list and Retry All button
- 20 service tests (incl. stale state 404/410, maxRetries expiry),
  7 context tests, 15 API integration tests
…utating actions

- Add conflictCheck() for updateWorkflow (GET resource -> 404=gone),
  createWorkflow (GET list -> name match=conflict),
  startTask (GET tasks -> same plugin_id running=conflict)
- Pass actionType from api.ts enqueue calls for targeted checks
- 6 new tests: conflict removed, no-conflict proceeds, backward compat
…t guard (utksh1#241)

- MAX_QUEUE_SIZE=50, enqueue throws when full
- ACTION_TTL_MS=24h, stale entries dropped on load
- persist() falls back to in-memory on localStorage error
- onReconnect() only replays when autoReplayEnabled and only
  for SAFE_ACTION_TYPES (startTask, createWorkflow, updateWorkflow)
- replayAction rejects unsafe actionType as gone
- SAFE_ACTION_TYPES exported for transparency
- 9 new tests: max queue, TTL, persist fallback, reconnect guard,
  safe action types

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. This still needs changes before merge because startTask() is marked retryable and queued for replay while offline. Starting a scan is not idempotent: replaying a queued start after the user also retries manually, reloads, or changes context can create duplicate scans. Please remove startTask from the automatic offline replay path unless the backend supports an idempotency key/client request id, and add tests proving replay cannot create duplicate scan tasks. Keep offline retry limited to operations with a clear idempotency story.

- startTask is not idempotent; replaying offline can create duplicate scans
- Removed from SAFE_ACTION_TYPES and ActionType union
- Removed retryable:true / actionType from startTask() in api.ts
- Removed startTask conflict check from offlineQueue.ts
- Updated 3 tests to reflect startTask is no longer queued offline
Keep both API key auth exports (getStoredApiKey, setStoredApiKey,
AUTH_REQUIRED_EVENT) and offline queue features (OfflineQueueError,
retryable actions) in api.ts. Wrap App.tsx routes with both auth gate
and OfflineQueueProvider.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed after updating the branch against current main. This is still blocked because frontend-checks is failing on the current head. Please fix the frontend check failure and rerun CI before requesting review again.

The merge of main into 241 left a corrupted request() function with
two fetch calls - the first without try-finally, the second inside
try-finally. Remove the first (orphaned) block and keep only the
try-finally version which properly clears the timeout on all paths.
The autoReplayEnabled flag in offlineQueue.ts is a module-level let
that persists across tests in single-worker mode. Test
'setAutoReplay is preserved for backward compat' leaves it as true,
which triggers fetch via onReconnect in subsequent tests.

Also remove orphaned duplicate fetch block from api.ts request()
function that was left by a prior merge - the try-finally version
was correct.
Prevents cross-test leakage of autoReplayEnabled flag and fetch
spy interference when tests run sequentially in single worker.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is green now, but I found a functional blocker in the offline replay path.

When request() queues a retryable mutation, it stores init?.headers in the queued action before the normal request path adds the API key. For createWorkflow() and updateWorkflow() that means the queued action contains Content-Type but not X-Api-Key. Later retry() replays the stored request with fetch(action.url, ...), so protected endpoints will return 401 instead of replaying successfully.

Please ensure replayed queued actions include the current stored API key at replay time, or store the auth header safely when queuing. Add a unit test that queues a retryable workflow mutation while offline, then replays it and asserts the replay request includes X-Api-Key.

- Move apiKey/authHeaders resolution before the offline enqueue path
  in request() so queued actions include X-Api-Key alongside Content-Type
- Pass stored headers in conflictCheck GET calls (offlineQueue.ts)
- Add tests asserting the queued action and replay fetch both carry
  the stored API key
- Install @tanstack/react-virtual (used by Findings.tsx)
- Update Workflows test to expect execution_context in steps
- Update ToolConfigDynamic test to expect 5th arg (executionContext)
- Update ToolConfigTimeout test: waitFor max attr, mock missing deps

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the update. This remains blocked by failing backend-lint and skipped backend jobs. Please rebase on latest main and ensure queued replay preserves the same authenticated request context/API-key behavior as the original request, with tests for replaying authenticated actions.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest push. The offline API queue remains large and needs a clean rebase plus focused tests for retry ordering, auth failures, queue persistence, and duplicate submission behavior. Please reduce unrelated churn before merge.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecking after the latest merge from main: this is still blocked.

The frontend-checks job is failing on the current head. Please fix the required check before the offline-aware API queue feature can be reconsidered.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecking after the latest dependency/audit commit: this is still blocked.

This offline queue feature should not carry unrelated audit-exception or frontend dependency workaround changes. Please remove that churn, keep the patch focused on the retryable action queue, and ensure required checks are green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:frontend Frontend React/UI work level:advanced 55 pts difficulty label for advanced contributor PRs type:feature Feature work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants