Skip to content

refactor(durably-react): improve cohesion and eliminate duplication#12

Merged
coji merged 3 commits into
mainfrom
refactor/durably-react-cohesion
Jan 3, 2026
Merged

refactor(durably-react): improve cohesion and eliminate duplication#12
coji merged 3 commits into
mainfrom
refactor/durably-react-cohesion

Conversation

@coji
Copy link
Copy Markdown
Owner

@coji coji commented Jan 3, 2026

Summary

  • Extract shared subscription state management into subscription-reducer
  • Create EventSubscriber interface to unify Durably.on and SSE patterns
  • Extract LogEntry creation helpers (createLogEntry, appendLog)
  • Split useJob into useJobSubscription and useAutoResume hooks
  • Centralize InferInput/InferOutput type utilities in types.ts
  • Simplify use-run-subscription and use-sse-subscription to thin wrappers

Refactoring Methods Used (Martin Fowler)

Method Applied To
Extract Function createLogEntry, appendLog
Extract Type SubscriptionState, SubscriptionAction
Extract Interface EventSubscriber
Extract Hook useJobSubscription, useAutoResume, useSubscription
Replace with Delegation subscription hooks → shared reducer

New Files

  • shared/create-log-entry.ts - LogEntry creation and append helpers
  • shared/subscription-reducer.ts - Pure reducer for state transitions
  • shared/event-subscriber.ts - Common interface for event subscription
  • shared/durably-event-subscriber.ts - Durably.on implementation
  • shared/sse-event-subscriber.ts - SSE/EventSource implementation
  • shared/use-subscription.ts - Core subscription hook
  • hooks/use-job-subscription.ts - Job-specific subscription with followLatest
  • hooks/use-auto-resume.ts - Extracted auto-resume logic

Impact

  • Lines changed: +866 / -524 (net reduction in complexity)
  • All 125 tests pass
  • No breaking changes to public API

Test plan

  • pnpm validate passes (format, lint, typecheck, tests)
  • All 125 existing tests pass
  • TypeScript compilation succeeds

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Automatic resume to detect and continue pending/running job executions
    • New subscription hook and job-specific subscription for unified, real-time job state (status, output, logs, progress)
    • SSE and runtime event subscribers for consistent event delivery
    • Structured log entry creation and controlled log appending
  • Refactor

    • Centralized subscription state and reducer for simpler, consistent state handling across hooks
    • Internal type handling moved to shared types for clearer type inference and maintenance

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 3, 2026

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

Project Deployment Review Updated (UTC)
durably-demo Ready Ready Preview Jan 3, 2026 0:07am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

Refactors durably-react to centralize subscription/state logic: adds a shared EventSubscriber abstraction, subscription reducer, and useSubscription hook; implements SSE and Durably-backed subscribers; moves InferInput/InferOutput types to a central types file; and introduces useAutoResume and useJobSubscription hooks, updating existing hooks to use the subscription model.

Changes

Cohort / File(s) Summary
Type system consolidation
packages/durably-react/src/types.ts, packages/durably-react/src/client/create-durably-client.ts, packages/durably-react/src/client/create-job-hooks.ts
Move/introduce InferInput/InferOutput utilities into types.ts; replace local in-file inference with imports; extend RunStatus and add SubscriptionState<TOutput>.
Subscription core
packages/durably-react/src/shared/event-subscriber.ts, packages/durably-react/src/shared/subscription-reducer.ts, packages/durably-react/src/shared/use-subscription.ts
Add SubscriptionEvent union and EventSubscriber interface; implement pure subscriptionReducer and initialSubscriptionState; add useSubscription hook with UseSubscriptionOptions/UseSubscriptionResult.
Event subscriber implementations
packages/durably-react/src/shared/sse-event-subscriber.ts, packages/durably-react/src/shared/durably-event-subscriber.ts
New factories createSSEEventSubscriber(apiBaseUrl) and createDurablyEventSubscriber(durably) implementing the EventSubscriber interface and mapping incoming events to SubscriptionEvent.
Log utilities
packages/durably-react/src/shared/create-log-entry.ts
New createLogEntry and appendLog utilities plus CreateLogEntryParams interface; generate UUID/timestamp and enforce maxLogs.
Run/job subscription hooks
packages/durably-react/src/hooks/use-run-subscription.ts, packages/durably-react/src/hooks/use-job-subscription.ts, packages/durably-react/src/client/use-sse-subscription.ts
Replace in-file run/ SSE subscription implementations with aliases/delegation to useSubscription + memoized subscribers; add useJobSubscription with extended state/actions (set_run_id, switch_to_run, reset) and followLatest/maxLogs features.
Auto-resume hook
packages/durably-react/src/hooks/use-auto-resume.ts
New useAutoResume hook that queries a JobHandle for running/pending runs and invokes onRunFound with the first match; exports UseAutoResumeOptions and UseAutoResumeCallbacks.
useJob refactor
packages/durably-react/src/hooks/use-job.ts
Migrate useJob internals to use useJobSubscription and useAutoResume; remove manual event listeners and local state in favor of subscription-derived state and reset API.
Package metadata / changelog
CHANGELOG.md, packages/durably-react/package.json
Bumped package to 0.6.1 and updated changelog describing the internal reorganization and new hooks.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Hook as useSubscription / useJobSubscription
    participant Subscriber as EventSubscriber (SSE or Durably)
    participant Reducer as subscriptionReducer
    participant State as SubscriptionState

    User->>Hook: mount / provide runId
    Hook->>Subscriber: subscribe(runId, onEvent)
    
    rect rgba(200,220,240,0.9)
        Subscriber->>Hook: onEvent(SubscriptionEvent)
        Hook->>Reducer: dispatch(action)
        Reducer->>State: compute new state
        State->>Hook: updated state returned
        Hook->>User: component re-render with new state
    end

    User->>Hook: unmount or runId change
    Hook->>Subscriber: unsubscribe()
Loading
sequenceDiagram
    actor User
    participant Auto as useAutoResume
    participant JobHandle
    participant JobSub as useJobSubscription
    participant Durably

    User->>Auto: mount (with jobHandle)
    Auto->>JobHandle: query running runs
    alt running found
        JobHandle-->>Auto: runId,status
        Auto->>JobSub: setCurrentRunId(runId)
    else none
        JobHandle-->>Auto: none
        Auto->>JobHandle: query pending runs
        JobHandle-->>Auto: pending runId
        Auto->>JobSub: setCurrentRunId(runId)
    end

    JobSub->>Durably: subscribe to events scoped to runId
    Durably-->>JobSub: run:progress / run:complete / log:write
    JobSub->>Reducer: dispatch actions -> update subscription state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐇 I hopped through code and cleaned the traces,

Events aligned in tidy places,
Subscriptions hum, logs shine bright,
Auto-resume finds runs at night,
The rabbit beams — subscriptions take flight!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(durably-react): improve cohesion and eliminate duplication' directly captures the main objective of the changeset: extracting shared logic, reducing duplication, and improving code organization across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (7)
packages/durably-react/src/shared/sse-event-subscriber.ts (1)

55-57: Consider logging parse errors in development mode.

Silently ignoring JSON parse errors is reasonable for resilience, but could mask malformed SSE payloads during development. Consider adding a development-mode warning if this becomes a debugging pain point.

packages/durably-react/src/shared/use-subscription.ts (1)

48-55: Render-time dispatch may cause unexpected behavior.

The reset dispatch on lines 51-53 executes during render, which is a React anti-pattern. This could cause issues with concurrent features and makes the component harder to reason about. The condition runIdRef.current !== runId will always be true on the first check since runIdRef.current is updated after the conditional block.

Consider moving this logic into the useEffect or using a dedicated effect for reset:

🔎 Suggested refactor using useEffect for reset
-  const runIdRef = useRef<string | null>(runId)
-  const prevRunIdRef = useRef<string | null>(null)
-
-  const maxLogs = options?.maxLogs ?? 0
-
-  // Reset state when runId changes
-  if (prevRunIdRef.current !== runId) {
-    prevRunIdRef.current = runId
-    if (runIdRef.current !== runId) {
-      dispatch({ type: 'reset' })
-    }
-  }
-  runIdRef.current = runId
+  const runIdRef = useRef<string | null>(runId)
+  const prevRunIdRef = useRef<string | null>(null)
+
+  const maxLogs = options?.maxLogs ?? 0
+
+  // Reset state when runId changes
+  useEffect(() => {
+    if (prevRunIdRef.current !== null && prevRunIdRef.current !== runId) {
+      dispatch({ type: 'reset' })
+    }
+    prevRunIdRef.current = runId
+    runIdRef.current = runId
+  }, [runId])
packages/durably-react/src/hooks/use-auto-resume.ts (2)

57-70: Type assertion on run.status assumes API contract.

The cast run.status as RunStatus on lines 59 and 69 assumes the API always returns a valid RunStatus. If the API returns an unexpected value, this could propagate invalid state. Consider validating the status or using a type guard.


78-78: callbacks in dependency array may cause excessive effect re-runs.

The callbacks object in the dependency array will trigger the effect on every render unless the caller memoizes it. This is documented implicitly by the pattern in use-job.ts (lines 120-127) where useMemo is used, but consider either:

  1. Documenting this requirement in the JSDoc
  2. Using useRef internally to always access the latest callback without re-triggering the effect
🔎 Alternative: use ref for stable callback access
+  const callbacksRef = useRef(callbacks)
+  callbacksRef.current = callbacks
+
   useEffect(() => {
     if (!jobHandle) return
     if (!enabled) return
     if (skipIfInitialRunId && initialRunId) return

     let cancelled = false

     const findActiveRun = async () => {
       // First check for running runs
       const runningRuns = await jobHandle.getRuns({ status: 'running' })
       if (cancelled) return

       if (runningRuns.length > 0) {
         const run = runningRuns[0]
-        callbacks.onRunFound(run.id, run.status as RunStatus)
+        callbacksRef.current.onRunFound(run.id, run.status as RunStatus)
         return
       }

       // Then check for pending runs
       const pendingRuns = await jobHandle.getRuns({ status: 'pending' })
       if (cancelled) return

       if (pendingRuns.length > 0) {
         const run = pendingRuns[0]
-        callbacks.onRunFound(run.id, run.status as RunStatus)
+        callbacksRef.current.onRunFound(run.id, run.status as RunStatus)
       }
     }

     findActiveRun()

     return () => {
       cancelled = true
     }
-  }, [jobHandle, enabled, skipIfInitialRunId, initialRunId, callbacks])
+  }, [jobHandle, enabled, skipIfInitialRunId, initialRunId])
packages/durably-react/src/hooks/use-job.ts (2)

148-154: Empty then block with only a comment.

The then block on lines 149-154 does nothing. If no action is needed, the .then() call can be removed, or if future implementation is planned, a TODO comment would be clearer.

🔎 Simplify by removing empty then
     // Fetch initial state for the run
-    jobHandle.getRun(options.initialRunId).then((run) => {
-      if (run) {
-        // State will be updated via subscription events or we could
-        // dispatch initial state here if needed
-      }
-    })
+    // State will be updated via subscription events
+    jobHandle.getRun(options.initialRunId)

188-209: Polling with hardcoded 50ms interval may be aggressive.

The triggerAndWait implementation uses a 50ms polling interval (line 205). This could generate significant load for long-running jobs. Consider making this configurable or using exponential backoff.

packages/durably-react/src/hooks/use-job-subscription.ts (1)

109-110: Ref assignment pattern differs from use-subscription.ts.

Line 110 unconditionally assigns currentRunIdRef.current = state.currentRunId on every render. While this works, it's slightly different from the pattern in use-subscription.ts. This is acceptable since the ref is primarily used for stale closure protection in event handlers, but consider consistency across hooks.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29c1781 and ef2cff5.

📒 Files selected for processing (14)
  • packages/durably-react/src/client/create-durably-client.ts
  • packages/durably-react/src/client/create-job-hooks.ts
  • packages/durably-react/src/client/use-sse-subscription.ts
  • packages/durably-react/src/hooks/use-auto-resume.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/hooks/use-job.ts
  • packages/durably-react/src/hooks/use-run-subscription.ts
  • packages/durably-react/src/shared/create-log-entry.ts
  • packages/durably-react/src/shared/durably-event-subscriber.ts
  • packages/durably-react/src/shared/event-subscriber.ts
  • packages/durably-react/src/shared/sse-event-subscriber.ts
  • packages/durably-react/src/shared/subscription-reducer.ts
  • packages/durably-react/src/shared/use-subscription.ts
  • packages/durably-react/src/types.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,mjs,mts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{js,ts,mjs,mts}: This library is ESM-only. CommonJS is not supported. Always use top-level await for async initialization (e.g., await durably.migrate()). Do not wrap in async IIFE or Promise chains.
Jobs must be defined via defineJob() and registered with durably.register(), receiving a step context and payload
Steps are created via step.run(), with each step's success state and return value persisted automatically

Files:

  • packages/durably-react/src/shared/sse-event-subscriber.ts
  • packages/durably-react/src/shared/event-subscriber.ts
  • packages/durably-react/src/shared/durably-event-subscriber.ts
  • packages/durably-react/src/client/create-job-hooks.ts
  • packages/durably-react/src/shared/use-subscription.ts
  • packages/durably-react/src/client/use-sse-subscription.ts
  • packages/durably-react/src/shared/create-log-entry.ts
  • packages/durably-react/src/hooks/use-run-subscription.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/hooks/use-job.ts
  • packages/durably-react/src/types.ts
  • packages/durably-react/src/shared/subscription-reducer.ts
  • packages/durably-react/src/client/create-durably-client.ts
  • packages/durably-react/src/hooks/use-auto-resume.ts
🧠 Learnings (1)
📚 Learning: 2026-01-03T06:23:01.913Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T06:23:01.913Z
Learning: Applies to **/*.{js,ts,mjs,mts} : Jobs must be defined via `defineJob()` and registered with `durably.register()`, receiving a step context and payload

Applied to files:

  • packages/durably-react/src/client/create-job-hooks.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/hooks/use-job.ts
  • packages/durably-react/src/types.ts
  • packages/durably-react/src/client/create-durably-client.ts
🧬 Code graph analysis (10)
packages/durably-react/src/shared/sse-event-subscriber.ts (3)
packages/durably-react/src/shared/event-subscriber.ts (2)
  • EventSubscriber (28-39)
  • SubscriptionEvent (7-22)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/types.ts (1)
  • DurablyEvent (63-103)
packages/durably-react/src/shared/event-subscriber.ts (2)
packages/durably-react/src/types.ts (1)
  • Progress (37-41)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/shared/use-subscription.ts (4)
packages/durably-react/src/types.ts (1)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/event-subscriber.ts (1)
  • EventSubscriber (28-39)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/shared/subscription-reducer.ts (2)
  • subscriptionReducer (37-83)
  • initialSubscriptionState (25-31)
packages/durably-react/src/shared/create-log-entry.ts (1)
packages/durably-react/src/types.ts (1)
  • LogEntry (43-51)
packages/durably-react/src/hooks/use-run-subscription.ts (3)
packages/durably-react/src/types.ts (1)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/use-subscription.ts (3)
  • UseSubscriptionOptions (9-14)
  • UseSubscriptionResult (16-27)
  • useSubscription (33-112)
packages/durably-react/src/shared/durably-event-subscriber.ts (1)
  • createDurablyEventSubscriber (8-81)
packages/durably-react/src/hooks/use-job-subscription.ts (3)
packages/durably-react/src/types.ts (1)
  • SubscriptionState (54-60)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/shared/subscription-reducer.ts (3)
  • SubscriptionAction (5-23)
  • initialSubscriptionState (25-31)
  • subscriptionReducer (37-83)
packages/durably-react/src/hooks/use-job.ts (3)
packages/durably-react/src/hooks/use-job-subscription.ts (1)
  • useJobSubscription (94-213)
packages/durably-react/src/types.ts (1)
  • RunStatus (30-35)
packages/durably-react/src/hooks/use-auto-resume.ts (1)
  • useAutoResume (32-79)
packages/durably-react/src/types.ts (2)
packages/durably-react/src/client.ts (3)
  • RunStatus (47-47)
  • LogEntry (47-47)
  • Progress (47-47)
packages/durably-react/src/index.ts (3)
  • RunStatus (14-14)
  • LogEntry (14-14)
  • Progress (14-14)
packages/durably-react/src/shared/subscription-reducer.ts (2)
packages/durably-react/src/types.ts (2)
  • Progress (37-41)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/create-log-entry.ts (2)
  • createLogEntry (15-25)
  • appendLog (30-40)
packages/durably-react/src/hooks/use-auto-resume.ts (2)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/types.ts (1)
  • RunStatus (30-35)
🔇 Additional comments (18)
packages/durably-react/src/shared/durably-event-subscriber.ts (1)

8-81: LGTM! Clean EventSubscriber implementation.

The factory pattern properly implements the EventSubscriber interface with appropriate event filtering by runId and comprehensive cleanup handling. The use of an array to collect unsubscribe functions ensures all listeners are properly removed.

packages/durably-react/src/client/create-job-hooks.ts (1)

2-2: LGTM!

Centralizing InferInput and InferOutput imports from ../types eliminates duplication and establishes a single source of truth for these type utilities.

packages/durably-react/src/client/create-durably-client.ts (1)

1-1: LGTM!

Consistent with create-job-hooks.ts, consolidating the type inference utilities to a shared module.

packages/durably-react/src/shared/sse-event-subscriber.ts (1)

44-53: Verify stepName handling consistency with the direct subscriber.

The SSE subscriber hardcodes stepName: null for log:write events (line 48), while durably-event-subscriber.ts forwards event.stepName from the Durably event. This appears intentional since DurablyEvent in types.ts doesn't include stepName for log:write, but please verify this is the expected behavior for SSE-based subscriptions.

packages/durably-react/src/types.ts (2)

5-28: Well-designed type inference utilities.

The InferInput and InferOutput types handle multiple extraction patterns (JobDefinition generics and trigger method signatures) with safe fallbacks to Record<string, unknown>. This centralization eliminates duplication across the client modules.


53-60: LGTM!

The SubscriptionState interface provides a clean, shared shape for subscription state management across both direct and SSE subscription strategies.

packages/durably-react/src/shared/create-log-entry.ts (2)

30-40: LGTM!

The appendLog function correctly maintains immutability and handles the log trimming logic. The slice(-maxLogs) approach efficiently keeps the most recent entries.


15-25: No action needed. The use of crypto.randomUUID() is appropriate for this ESM-only library targeting ES2022. The API is available in Node.js 15.7.0+ and all modern browsers, making it a suitable choice without requiring fallbacks or additional dependencies.

Likely an incorrect or invalid review comment.

packages/durably-react/src/shared/subscription-reducer.ts (2)

37-83: Clean and well-structured reducer implementation.

The reducer correctly handles all state transitions with proper immutability. The separation of concerns (transport-level connection_error vs run-level status changes) is appropriate.


77-78: Consider whether connection_error should affect status.

Currently, connection_error only sets the error field without changing status. This is reasonable if the intent is to show a connection warning while preserving the last known run status. However, if the connection is lost mid-run, consumers may want to distinguish between "failed run" and "lost connection to running job." The current approach seems intentional—just confirming this is the desired UX.

packages/durably-react/src/shared/event-subscriber.ts (2)

7-22: Well-designed discriminated union for subscription events.

The SubscriptionEvent type provides a clean abstraction that unifies Durably.on and SSE event patterns. The discriminated union with type field enables exhaustive type checking in consumers.


28-39: Clean interface design with proper cleanup semantics.

The EventSubscriber interface correctly returns a cleanup function from subscribe, which aligns well with React's useEffect cleanup pattern. The generic TOutput parameter provides type safety across the subscription chain.

packages/durably-react/src/hooks/use-run-subscription.ts (2)

11-19: Good backward compatibility with deprecation guidance.

The type aliases with @deprecated JSDoc tags maintain backward compatibility while guiding consumers toward the shared types. This is a clean migration path.


32-37: Clean delegation to shared subscription infrastructure.

The hook now serves as a thin wrapper that creates a DurablyEventSubscriber via useMemo and delegates to useSubscription. The memoization dependency on [durably] is correct.

packages/durably-react/src/shared/use-subscription.ts (1)

57-97: Solid subscription lifecycle management.

The useEffect properly handles subscription setup and teardown. The stale closure protection via runIdRef.current !== runId check (line 62) prevents processing events for outdated runIds. The dependency array [subscriber, runId, maxLogs] correctly captures all values used in the effect.

packages/durably-react/src/client/use-sse-subscription.ts (1)

31-36: Clean delegation matching the Durably counterpart.

The implementation mirrors useRunSubscription with SSE-specific subscriber creation. The memoization on [api] is correct, and the delegation to useSubscription properly abstracts the transport layer.

packages/durably-react/src/hooks/use-job-subscription.ts (2)

59-88: Well-structured reducer composition.

The jobSubscriptionReducer correctly extends the base subscriptionReducer by handling job-specific actions (set_run_id, switch_to_run, reset) and delegating others to the base reducer while preserving currentRunId. The switch_to_run action appropriately resets state and sets status to 'running'.


115-191: Comprehensive event handling with proper filtering.

The event subscription setup correctly filters events by currentRunIdRef.current for most handlers, while run:start uses jobName filtering to support the followLatest feature. The cleanup function properly unsubscribes all listeners. The dependency array [durably, jobName, followLatest, maxLogs] captures all relevant values.

Comment thread packages/durably-react/src/hooks/use-job.ts
- Extract shared subscription state management into subscription-reducer
- Create EventSubscriber interface to unify Durably.on and SSE patterns
- Extract LogEntry creation helpers (createLogEntry, appendLog)
- Split useJob into useJobSubscription and useAutoResume hooks
- Centralize InferInput/InferOutput type utilities in types.ts
- Simplify use-run-subscription and use-sse-subscription to thin wrappers

This refactoring follows Martin Fowler's methods:
- Extract Function: createLogEntry, appendLog
- Extract Type: SubscriptionState, SubscriptionAction
- Extract Interface: EventSubscriber
- Extract Hook: useJobSubscription, useAutoResume, useSubscription
- Replace with Delegation: subscription hooks now delegate to shared reducer

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address CodeRabbit review: the empty .then() block was unnecessary
as state is updated via subscription events.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coji coji merged commit 10af60a into main Jan 3, 2026
7 checks passed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
packages/durably-react/src/hooks/use-auto-resume.ts (1)

52-71: Consider adding error handling for getRuns failures.

The async findActiveRun function does not handle potential errors from jobHandle.getRuns(). If the query fails (network issues, database errors), the hook silently fails without notifying the parent.

🔎 Suggested error handling approach
 const findActiveRun = async () => {
+  try {
     // First check for running runs
     const runningRuns = await jobHandle.getRuns({ status: 'running' })
     if (cancelled) return

     if (runningRuns.length > 0) {
       const run = runningRuns[0]
       callbacks.onRunFound(run.id, run.status as RunStatus)
       return
     }

     // Then check for pending runs
     const pendingRuns = await jobHandle.getRuns({ status: 'pending' })
     if (cancelled) return

     if (pendingRuns.length > 0) {
       const run = pendingRuns[0]
       callbacks.onRunFound(run.id, run.status as RunStatus)
     }
+  } catch (error) {
+    // Silently fail or log - auto-resume is a convenience feature
+    console.warn('Failed to auto-resume run:', error)
+  }
 }
packages/durably-react/src/hooks/use-job.ts (2)

177-198: Polling has no timeout or max retry limit.

The triggerAndWait polling loop will continue indefinitely if a job stays in running or pending state forever (e.g., due to a bug or network issue). Consider adding a configurable timeout or maximum poll count to prevent infinite loops.

🔎 Optional: Add timeout protection
 const triggerAndWait = useCallback(
   async (input: TInput): Promise<{ runId: string; output: TOutput }> => {
     const jobHandle = jobHandleRef.current
     if (!jobHandle || !durably) {
       throw new Error('Job not ready')
     }

     // Reset state before triggering
     subscription.reset()

     const run = await jobHandle.trigger(input)
     subscription.setCurrentRunId(run.id)

     // Wait for completion by polling
     return new Promise((resolve, reject) => {
+      const startTime = Date.now()
+      const timeoutMs = 30000 // 30 second default timeout
+
       const checkCompletion = async () => {
+        if (Date.now() - startTime > timeoutMs) {
+          reject(new Error('Timeout waiting for job completion'))
+          return
+        }
+
         const updatedRun = await jobHandle.getRun(run.id)
         // ... rest of implementation
       }
       checkCompletion()
     })
   },
   [durably, subscription],
 )

146-162: Consider narrowing callback dependencies.

The trigger callback depends on the entire subscription object, which changes whenever subscription state updates. Since trigger only uses subscription.reset() and subscription.setCurrentRunId(), consider depending on just those stable callbacks to prevent unnecessary callback recreation.

🔎 Narrow dependencies
   const trigger = useCallback(
     async (input: TInput): Promise<{ runId: string }> => {
       // ... implementation
     },
-    [subscription],
+    [subscription.reset, subscription.setCurrentRunId],
   )

Apply similarly to triggerAndWait on line 200.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef2cff5 and 2e17d9f.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • packages/durably-react/package.json
  • packages/durably-react/src/client/create-durably-client.ts
  • packages/durably-react/src/client/create-job-hooks.ts
  • packages/durably-react/src/client/use-sse-subscription.ts
  • packages/durably-react/src/hooks/use-auto-resume.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/hooks/use-job.ts
  • packages/durably-react/src/hooks/use-run-subscription.ts
  • packages/durably-react/src/shared/create-log-entry.ts
  • packages/durably-react/src/shared/durably-event-subscriber.ts
  • packages/durably-react/src/shared/event-subscriber.ts
  • packages/durably-react/src/shared/sse-event-subscriber.ts
  • packages/durably-react/src/shared/subscription-reducer.ts
  • packages/durably-react/src/shared/use-subscription.ts
  • packages/durably-react/src/types.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/durably-react/package.json
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/durably-react/src/shared/durably-event-subscriber.ts
  • packages/durably-react/src/shared/event-subscriber.ts
  • packages/durably-react/src/shared/create-log-entry.ts
  • packages/durably-react/src/shared/use-subscription.ts
  • packages/durably-react/src/client/create-job-hooks.ts
  • packages/durably-react/src/shared/sse-event-subscriber.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,mjs,mts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{js,ts,mjs,mts}: This library is ESM-only. CommonJS is not supported. Always use top-level await for async initialization (e.g., await durably.migrate()). Do not wrap in async IIFE or Promise chains.
Jobs must be defined via defineJob() and registered with durably.register(), receiving a step context and payload
Steps are created via step.run(), with each step's success state and return value persisted automatically

Files:

  • packages/durably-react/src/types.ts
  • packages/durably-react/src/shared/subscription-reducer.ts
  • packages/durably-react/src/client/use-sse-subscription.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/hooks/use-run-subscription.ts
  • packages/durably-react/src/client/create-durably-client.ts
  • packages/durably-react/src/hooks/use-auto-resume.ts
  • packages/durably-react/src/hooks/use-job.ts
🧠 Learnings (3)
📚 Learning: 2026-01-03T06:23:01.913Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T06:23:01.913Z
Learning: Applies to **/*.{js,ts,mjs,mts} : Jobs must be defined via `defineJob()` and registered with `durably.register()`, receiving a step context and payload

Applied to files:

  • packages/durably-react/src/types.ts
  • packages/durably-react/src/hooks/use-job-subscription.ts
  • packages/durably-react/src/client/create-durably-client.ts
  • packages/durably-react/src/hooks/use-job.ts
📚 Learning: 2026-01-03T06:23:01.913Z
Learnt from: CR
Repo: coji/durably PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-03T06:23:01.913Z
Learning: Applies to **/*.{js,ts,mjs,mts} : This library is ESM-only. CommonJS is not supported. Always use top-level `await` for async initialization (e.g., `await durably.migrate()`). Do not wrap in async IIFE or Promise chains.

Applied to files:

  • CHANGELOG.md
📚 Learning: 2026-01-02T14:03:31.517Z
Learnt from: coji
Repo: coji/durably PR: 11
File: examples/browser-vite-react/src/components/dashboard.tsx:24-34
Timestamp: 2026-01-02T14:03:31.517Z
Learning: In example code directories (e.g., examples/browser-vite-react, examples/fullstack-react-router), error handling may be intentionally omitted to keep code simple and readable, focusing on demonstrating the Durably API rather than production best practices.

Applied to files:

  • CHANGELOG.md
🧬 Code graph analysis (5)
packages/durably-react/src/shared/subscription-reducer.ts (2)
packages/durably-react/src/types.ts (2)
  • Progress (37-41)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/create-log-entry.ts (2)
  • createLogEntry (15-25)
  • appendLog (30-40)
packages/durably-react/src/client/use-sse-subscription.ts (4)
packages/durably-react/src/types.ts (1)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/use-subscription.ts (3)
  • UseSubscriptionOptions (9-14)
  • UseSubscriptionResult (16-27)
  • useSubscription (33-112)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/shared/sse-event-subscriber.ts (1)
  • createSSEEventSubscriber (8-70)
packages/durably-react/src/hooks/use-job-subscription.ts (2)
packages/durably-react/src/types.ts (1)
  • SubscriptionState (54-60)
packages/durably-react/src/shared/subscription-reducer.ts (3)
  • SubscriptionAction (5-23)
  • initialSubscriptionState (25-31)
  • subscriptionReducer (37-83)
packages/durably-react/src/hooks/use-auto-resume.ts (2)
packages/durably/src/context.ts (1)
  • runId (19-21)
packages/durably-react/src/types.ts (1)
  • RunStatus (30-35)
packages/durably-react/src/hooks/use-job.ts (3)
packages/durably-react/src/hooks/use-job-subscription.ts (1)
  • useJobSubscription (94-213)
packages/durably-react/src/types.ts (1)
  • RunStatus (30-35)
packages/durably-react/src/hooks/use-auto-resume.ts (1)
  • useAutoResume (32-79)
🔇 Additional comments (16)
CHANGELOG.md (1)

26-33: LGTM! Clear documentation of internal refactoring.

The changelog entry accurately describes the internal code organization improvements without exposing implementation details. The "no API changes" note correctly sets expectations for users.

packages/durably-react/src/types.ts (2)

3-28: Well-designed type inference utilities.

The conditional type logic correctly extracts Input/Output types from both JobDefinition and trigger function signatures, with safe fallbacks to Record<string, unknown>. This centralization eliminates duplication across the codebase.


53-60: LGTM! Unified subscription state interface.

The SubscriptionState interface effectively consolidates the state shape used by both direct Durably subscriptions and SSE-based subscriptions, eliminating duplication and improving maintainability.

packages/durably-react/src/client/create-durably-client.ts (1)

1-1: LGTM! Successfully migrated to centralized type utilities.

The import change from local type definitions to the shared InferInput and InferOutput utilities in ../types.ts eliminates duplication and aligns with the PR's cohesion goals.

packages/durably-react/src/hooks/use-run-subscription.ts (1)

27-37: LGTM! Clean refactoring to shared subscription infrastructure.

The hook successfully delegates to useSubscription with a memoized Durably event subscriber, eliminating local state management and manual event handling. The deprecated type aliases maintain backward compatibility while guiding users toward the shared types.

packages/durably-react/src/hooks/use-auto-resume.ts (2)

5-26: LGTM! Well-designed interface for auto-resume functionality.

The options and callback interfaces provide clear, flexible control over auto-resume behavior. The JSDoc comments effectively document the purpose and defaults.


45-78: The callbacks object is properly memoized with useMemo in the consuming component (use-job.ts, lines 120-127), making it a stable reference across renders. The dependency on callbacks in the useEffect is safe and will not cause infinite loops or unnecessary re-executions.

Likely an incorrect or invalid review comment.

packages/durably-react/src/shared/subscription-reducer.ts (2)

1-31: Well-structured reducer foundation with clean type definitions.

The action union type and initial state are clearly defined. The separation of concerns is good.


37-82: Clean reducer implementation with appropriate state transitions.

The reducer correctly handles all action types and follows immutable update patterns. The delegation to createLogEntry within the reducer introduces minor impurity (UUID/timestamp generation), but this is a reasonable trade-off for cleaner action payloads.

packages/durably-react/src/client/use-sse-subscription.ts (1)

1-36: Clean refactoring with good backward compatibility.

The hook is now a thin wrapper delegating to the shared useSubscription, maintaining backward compatibility through deprecated type aliases. The useMemo correctly ensures the subscriber is only recreated when the api URL changes.

packages/durably-react/src/hooks/use-job.ts (2)

203-218: Clean return structure with well-derived helpers.

The derived boolean helpers (isRunning, isPending, etc.) provide good ergonomics for consumers. The inline computation is appropriate given the minimal overhead of equality checks.


119-137: The ref timing concern is mitigated by proper null guards and dependency tracking.

The jobHandleRef.current passes through useAutoResume as null on initial render, but the hook guards against this with an early return (if (!jobHandle) return). When the registration effect runs and populates jobHandleRef.current, the dependency array in useAutoResume includes jobHandle, so the effect properly re-executes with the new handle on the next render. If jobDefinition changes, the registration effect re-runs, updates jobHandleRef.current, and useAutoResume will re-run in the next render cycle due to its dependency tracking. Tests confirm this behavior works correctly in practice.

packages/durably-react/src/hooks/use-job-subscription.ts (4)

10-41: Well-documented interface definitions.

The options and result interfaces are clearly documented with JSDoc comments. Extending SubscriptionState<TOutput> for the result type maintains good type composition.


59-88: Solid reducer composition pattern.

The reducer correctly extends the base subscriptionReducer by handling job-specific actions (set_run_id, switch_to_run) and delegating others while preserving currentRunId. The switch_to_run action appropriately resets state for the new run.


115-191: Correct event subscription management with proper cleanup.

The effect correctly:

  • Subscribes to all relevant Durably events
  • Filters events by jobName and currentRunIdRef.current
  • Synchronously updates the ref after switch_to_run dispatch to prevent race conditions
  • Cleans up all subscriptions on unmount or dependency change

The dependency array correctly includes all referenced values.


193-212: Stable callback implementations with proper ref synchronization.

The callbacks correctly use empty dependency arrays (since dispatch from useReducer is stable) and synchronize the ref for immediate consistency. Good pattern for avoiding stale closure issues in event handlers.

@coji coji deleted the refactor/durably-react-cohesion branch March 5, 2026 12:14
coji added a commit that referenced this pull request Mar 24, 2026
…st count

- Add releaseExpiredLeases + unique index interaction section:
  expired lease resets to 'failed' (not 'pending') when another
  pending run exists for the same concurrencyKey
- Fix scope: "Two breaking changes + one new feature" (was "Two" with 3 items)
- Fix test count: 25 cases (was 20)
- Add 2 test cases for releaseExpiredLeases interaction
- Add design decision #12 for the expired lease behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coji added a commit that referenced this pull request Mar 25, 2026
…146)

* docs: add RFC for coalesce option on trigger (#143)

Implementation plan covering skip mode, merge mode, storage layer
changes, type definitions, HTTP API, and test cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: rewrite coalesce RFC for skip-mode-only scope

Align with updated issue #143. Key changes:
- Remove merge mode (deferred to future)
- Return type is { run, coalesced } tuple (breaking)
- No new storage method — coalesce as branch in enqueue()
- Add Design Decisions section with rationale

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: incorporate Codex review feedback into coalesce RFC

Major revisions based on external review:
- Use string union (`coalesce: 'skip'`) instead of boolean for extensibility
- No breaking change: trigger() unchanged, add triggerDetailed() instead
- Internal `disposition` model unifies idempotency and coalesce returns
- Partial unique index for atomic coalesce (no PostgreSQL race condition)
- Add run:coalesced event for observability
- Document semantic caveats (input/labels ignored on coalesce)
- Add ORDER BY created_at for deterministic pending selection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: rewrite coalesce RFC — full breaking change approach

Major design changes:
- concurrencyKey now enforces max 1 pending (partial unique index)
- Without coalesce: ConflictError on duplicate pending
- coalesce: 'skip' for graceful handling
- trigger() returns TriggerResult (TypedRun & { disposition })
  No destructuring needed, existing Run property access unchanged
- disposition: 'created' | 'idempotent' | 'coalesced'
- INSERT-first, catch conflict (no TOCTOU race)
- Fail-fast migration if duplicate pending data exists
- batchTrigger() also returns TriggerResult[]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: address Codex final review — 4 implementation fixes

1. parseUniqueViolation() to distinguish idempotency vs concurrency
   constraint violations (prevents misclassification)
2. Post-conflict SELECT queries pending OR leased (handles race
   where worker leases between INSERT failure and SELECT)
3. batchTrigger() uses sequential enqueue (bulk INSERT cannot handle
   per-item coalesce/conflict within same batch)
4. Fix idempotent event docs — current code emits run:trigger on
   idempotency hits, this is a behavior change not status quo

Also: 3 new test cases (concurrent races, post-conflict race),
design decisions expanded to 13 items.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: fix RFC issues — releaseExpiredLeases conflict, scope count, test count

- Add releaseExpiredLeases + unique index interaction section:
  expired lease resets to 'failed' (not 'pending') when another
  pending run exists for the same concurrencyKey
- Fix scope: "Two breaking changes + one new feature" (was "Two" with 3 items)
- Fix test count: 25 cases (was 20)
- Add 2 test cases for releaseExpiredLeases interaction
- Add design decision #12 for the expired lease behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: address Codex fresh review — 6 implementation fixes

1. batchTrigger: remove "partial success" — stays atomic within
   transaction, ConflictError rolls back entire batch
2. Post-conflict SELECT: query all non-cancelled statuses, retry
   INSERT if conflicting run was purged (closes the race)
3. releaseExpiredLeases: 2-phase approach — identify conflicting
   rows first (fail them), then reset the rest to pending
4. triggerAndWait: add disposition to TriggerAndWaitResult
5. parseUniqueViolation: SQLite uses column names not index names
   in error messages — document the correct detection strategy
6. run:coalesced event: carry skippedInput + skippedLabels (both
   from the caller), not mixed old/new data

28 test cases now. Design decisions updated to 14.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: fix post-conflict SELECT — only return active runs, guard retry

- Post-conflict SELECT returns only pending/leased (not completed/failed)
- Terminal state = index slot freed → retry INSERT once
- Retry guarded by _retried flag to prevent infinite recursion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: address CodeRabbit review — SAVEPOINT, tie-breaker, migration v2, batchTrigger contract

1. PostgreSQL SAVEPOINT: INSERT wrapped in SAVEPOINT/ROLLBACK TO SAVEPOINT
   to allow catch-and-recover after unique violation (PostgreSQL aborts
   transaction on constraint error without SAVEPOINT)
2. ORDER BY tie-breaker: add `id` alongside `created_at` for deterministic
   ordering when timestamps are identical
3. Migration v2 rationale: explain why v2 (not v1 consolidation) — v1 is
   already released and applied to existing user databases
4. batchTrigger contract: sequential item-level processing with per-item
   disposition is a contract, not an implementation detail
5. Design decisions expanded to 15 items

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: v1 migration consolidation + Codex final 3 fixes

Migration:
- Consolidate partial unique index into v1 (not v2) — only
  production user (upflow) will recreate their database
- Remove migration-v2.md, fail-fast duplicate check, version bump

Codex fixes:
1. Post-conflict SELECT returns only pending (not leased — could
   be expired orphan), retry INSERT if not found
2. run:coalesced SSE/React integration: wire into SSE handler,
   add to DurablyEvent union, add labels for SSE filtering
3. Runtime validation of coalesce value ('skip' only) in both
   trigger() and batchTrigger() per-item

Design decisions now 16. Test cases still 28.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: retry fallback SELECT + _retried implementation note

- Retry INSERT failure: one more SELECT before throwing, returns
  pending run if another trigger won the race
- Error message enriched: "Conflict after retry; concurrent modification"
- Note: _retried flag is a placeholder, implement as context parameter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: address Codex final check — 4 fixes

1. Dual constraint violation: catch block always re-checks idempotency
   first, regardless of which constraint the DB reported. A single
   INSERT can violate both constraints non-deterministically.
2. Store refactoring for batchTrigger: document _enqueueInTx(trx)
   pattern — enqueue() must accept optional transaction object for
   batch to share a single atomic transaction.
3. followLatest hook: add run:coalesced to the event types that
   use-job.ts followLatest reacts to.
4. Migration note: fix inaccurate claim about TypeScript compiler
   flagging all call sites — TriggerResult extends TypedRun so most
   assignments are compatible.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: address Codex final check round 2 — 4 fixes

1. releaseExpiredLeases Phase 2 race: per-row UPDATE with SAVEPOINT
   to handle concurrent trigger() inserting pending between phases
2. followLatest: add use-job-subscription.ts (direct/SPA hook) to
   the list of files needing run:coalesced support
3. run:coalesced event: add `labels` (existing run's) alongside
   skippedInput/skippedLabels — required for SSE label-scoped filtering
4. Design Decision 16: fix TypeScript compiler claim to match
   Migration note (compiler will NOT flag all call sites)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add note that pseudo-code is illustrative, tests define contract

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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