Skip to content

fix(logs,workspace): prevent cancelled status overwrite on race and move impersonation banner#4617

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/cancel-status-overwrite-and-impersonation-banner-move
May 15, 2026
Merged

fix(logs,workspace): prevent cancelled status overwrite on race and move impersonation banner#4617
waleedlatif1 merged 1 commit into
stagingfrom
fix/cancel-status-overwrite-and-impersonation-banner-move

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Fixed a race condition where pressing cancel in the Logs panel would briefly show `cancelled` but revert to `error` on refresh — the cancel route writes `cancelled` to the DB optimistically, but a block error racing the 500ms Redis check could finalize with `failed` before the engine detected cancellation; added the same DB status guard to `completeWithError` that `completeWithCancellation` already had
  • Added 5 tests covering the guard: cancelled DB status skips the write, non-cancelled status proceeds normally, empty log proceeds normally, DB failure falls through to cost-only fallback, and subsequent calls are deduped after the guard marks the session complete
  • Moved `ImpersonationBanner` from workspace root into `components/` folder

Type of Change

  • Bug fix

Testing

  • Tested manually
  • 394 test files, 6395 tests passing

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…ove impersonation banner

- Guard completeWithError against overwriting a cancelled execution status — cancel route writes cancelled to DB optimistically, but a block error racing the 500ms Redis check could finalize with failed before the engine detects cancellation
- Add tests covering the guard: cancelled DB status skips the write, non-cancelled proceeds normally, DB failure falls through to cost-only fallback, and subsequent attempts are deduped after guard marks session complete
- Move ImpersonationBanner from workspace root into components/ folder
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 15, 2026 6:44pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 15, 2026

PR Summary

Medium Risk
Changes terminal execution logging behavior by adding a DB status guard before writing a failed completion, which could affect how race conditions and completion deduping behave under load. UI impact is limited to an import/path move for ImpersonationBanner.

Overview
Prevents a race where an execution already marked cancelled in the DB could later be finalized as failed: LoggingSession.completeWithError now reads the current log status and early-returns (marking the session completed) when it is cancelled.

Adds a focused test suite covering the new guard behavior (cancelled skip, running/empty proceed, guard dedupes later completions, and DB read failure falling back to the existing cost-only path). Separately updates workspace layout to import ImpersonationBanner from a new components/ location.

Reviewed by Cursor Bugbot for commit 112e028. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR fixes a race condition in the workflow execution logging where pressing cancel could briefly show cancelled but revert to error on refresh. It adds the same DB status guard to completeWithError that already existed in completeWithCancellation, and moves ImpersonationBanner into the components/ subfolder.

  • Race condition fix: completeWithError now queries the current DB status before writing failed; if the status is already cancelled, it skips the write and marks the session complete — mirroring the guard already present in completeWithCancellation.
  • Tests: Five new test cases cover the guard (cancelled skip, running proceed, empty-log proceed, DB-failure fallback, and deduplication after early-return).
  • Refactor: ImpersonationBanner is renamed/moved from the workspace root into components/, with the import in layout.tsx updated accordingly.

Confidence Score: 5/5

Safe to merge — the change is surgical, well-tested, and consistent with the existing cancellation guard pattern.

The new DB status guard in completeWithError directly mirrors the identical guard already present in completeWithCancellation, keeping both code paths consistent. The five new tests cover every meaningful branch including the DB-failure fallback and deduplication. The ImpersonationBanner move is a pure rename with no behavioral change. No edge cases were found that the existing guard and deduplication machinery don't already handle.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/logs/execution/logging-session.ts Adds a DB status pre-check in completeWithError to skip writing failed when the execution is already cancelled; logic mirrors the existing guard in completeWithCancellation.
apps/sim/lib/logs/execution/logging-session.test.ts Adds five tests covering the cancelled-status guard: skip-on-cancelled, proceed-on-running, proceed-on-missing-log, DB-failure fallback, and deduplication after early-return.
apps/sim/app/workspace/[workspaceId]/components/impersonation-banner.tsx File renamed/moved from workspace root to components/ subfolder; content unchanged.
apps/sim/app/workspace/[workspaceId]/layout.tsx Import path for ImpersonationBanner updated to reflect its new location in components/.

Sequence Diagram

sequenceDiagram
    participant User
    participant CancelRoute
    participant DB
    participant Engine
    participant completeWithError

    User->>CancelRoute: Press Cancel
    CancelRoute->>DB: "Write status = 'cancelled'"
    Note over Engine: Block errors simultaneously
    Engine->>completeWithError: safeCompleteWithError(blockError)
    completeWithError->>DB: "SELECT status WHERE executionId = ?"
    DB-->>completeWithError: "status = 'cancelled'"
    completeWithError->>completeWithError: "Guard fires → set completed=true, return"
    Note over completeWithError: Skips writing 'failed' ✓
Loading

Reviews (1): Last reviewed commit: "fix(logs,workspace): prevent cancelled s..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 merged commit d3d8f9c into staging May 15, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/cancel-status-overwrite-and-impersonation-banner-move branch May 15, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant