Skip to content

chore(logging): remove redis-progress-markers feature flag#5287

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/ff
Jun 30, 2026
Merged

chore(logging): remove redis-progress-markers feature flag#5287
waleedlatif1 merged 2 commits into
stagingfrom
fix/ff

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Remove the redis-progress-markers feature flag now that it's fully rolled out, making the Redis progress-marker write path permanent behavior
  • Block markers always write to Redis (primary), keeping the durable jsonb_set UPDATE fallback when Redis is unavailable or the write fails
  • Drops the flag registry entry, its REDIS_PROGRESS_MARKERS env fallback, and the per-session flag resolution in the logging session
  • Cleanup: remove the now-vestigial readProgressMarkers param end-to-end (the completion fold always reads Redis markers), and delete the orphaned flag mock + flag-off/flag-throws marker tests that the code can no longer reach

Type of Change

  • Chore / maintenance

Testing

  • vitest for logging-session, logger, progress-markers, and feature-flags — all passing
  • Typecheck + lint clean

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)

Promote the Redis progress-marker write path to permanent behavior now that
the flag is fully rolled out. Block markers always write to Redis (primary),
keeping the durable jsonb_set UPDATE fallback when Redis is unavailable.

Removes the flag registry entry, its REDIS_PROGRESS_MARKERS env fallback, and
the per-session flag resolution in the logging session.
@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 30, 2026 4:30pm

Request Review

@cursor

cursor Bot commented Jun 30, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches the hot workflow execution logging path (progress markers and completion fold); behavior change is intentional rollout cleanup with SQL fallback retained, but misconfigured Redis could increase DB write load.

Overview
Removes the redis-progress-markers rollout flag and REDIS_PROGRESS_MARKERS env fallback now that Redis-backed live progress markers are the only path.

LoggingSession always tries Redis for lastStartedBlock / lastCompletedBlock and keeps the existing jsonb_set SQL fallback when Redis fails. Session startup no longer resolves or caches a flag, and completion no longer passes readProgressMarkers.

completeWorkflowExecution always reads Redis markers at terminal fold (and clears them when present). The readProgressMarkers option is removed from the logger API and ExecutionLoggerService types.

Tests drop flag mocks and scenarios for the legacy SQL-only path; remaining cases cover Redis success and SQL fallback.

Reviewed by Cursor Bugbot for commit 12a2a8a. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes Redis progress markers the permanent logging path. The main changes are:

  • Removed the redis-progress-markers feature flag and env fallback.
  • Always writes block progress markers to Redis first, with the existing SQL fallback.
  • Always reads Redis markers during completion.
  • Removed the obsolete readProgressMarkers option from the logger contract.
  • Updated logging-session tests for the permanent Redis-first behavior.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
apps/sim/lib/core/config/env.ts Removes the obsolete Redis progress marker env fallback.
apps/sim/lib/core/config/feature-flags.ts Removes the Redis progress marker feature flag from the registry.
apps/sim/lib/logs/execution/logger.ts Removes the marker-read option and always folds Redis markers during completion.
apps/sim/lib/logs/execution/logging-session.ts Removes per-session flag resolution and always uses the Redis-first marker write path.
apps/sim/lib/logs/execution/logging-session.test.ts Updates progress marker tests for the permanent Redis-first behavior.
apps/sim/lib/logs/types.ts Removes the obsolete readProgressMarkers option from the logger service type.

Reviews (2): Last reviewed commit: "chore(logging): drop vestigial readProgr..." | Re-trigger Greptile

Comment thread apps/sim/lib/logs/execution/logging-session.ts
Comment thread apps/sim/lib/logs/execution/logging-session.ts
Comment thread apps/sim/lib/logs/execution/logging-session.ts
Follow-up to the redis-progress-markers flag removal. With the flag gone the
completion fold always reads Redis markers, so the readProgressMarkers param
(and its dead false branch) is removed end-to-end:

- drop the param from completeWorkflowExecution and CompleteWorkflowExecutionParams
- read progress markers unconditionally in the completion fold
- delete the orphaned isFeatureEnabled mock and the flag-off / flag-throws
  marker tests (states the code can no longer produce)
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 12a2a8a. Configure here.

@waleedlatif1 waleedlatif1 merged commit e7635db into staging Jun 30, 2026
17 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/ff branch June 30, 2026 16:37
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