Skip to content

feat: shared Redis PR state for cross-platform coding agent#265

Merged
sweetmantech merged 3 commits intotestfrom
sweetmantech/myc-4434-github-pr-aware-coding-agent-flow
Mar 7, 2026
Merged

feat: shared Redis PR state for cross-platform coding agent#265
sweetmantech merged 3 commits intotestfrom
sweetmantech/myc-4434-github-pr-aware-coding-agent-flow

Conversation

@sweetmantech
Copy link
Contributor

@sweetmantech sweetmantech commented Mar 7, 2026

Summary

  • Adds shared Redis key coding-agent:pr:{repo}:{branch} so Slack and GitHub can share PR context for the same branch
  • Callbacks (pr_created, updated) write to shared state alongside thread state
  • handleFeedback syncs shared state when triggering update-pr
  • onMergeAction cleans up the shared key on merge
  • onNewMention extracts repo/branch from message.raw and falls back to shared PR state when thread state is null (enables GitHub PR comments to trigger updates)
  • New resolvePRState() helper unifies state resolution from thread state or shared key

Test plan

  • 44 coding-agent tests pass (11 files)
  • Slack: @mention bot → PRs Created → reply with feedback → PRs Updated → Merge All PRs
  • Verify shared Redis key is written on pr_created callback
  • Verify shared Redis key is deleted on merge

Closes MYC-4434

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Implemented persistent PR state storage, enabling more reliable tracking of pull requests across workflow operations.
  • Refactor

    • Improved PR context resolution mechanisms and state management cleanup procedures for better consistency across operations.

Introduces a shared Redis key (coding-agent:pr:{repo}:{branch}) so both
Slack and GitHub can share PR context for the same branch. Callbacks,
feedback, and merge actions now read/write this shared state alongside
thread state.

- Add prState.ts with get/set/delete helpers
- Add resolvePRState.ts to fall back to shared key when thread state is null
- Update handlePRCreated, handleCodingAgentCallback to write shared state
- Update handleFeedback to sync shared state on update-pr trigger
- Update onMergeAction to clean up shared key on merge
- Update onNewMention to extract repo/branch from message.raw for GitHub PRs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Contributor

vercel bot commented Mar 7, 2026

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

Project Deployment Actions Updated (UTC)
recoup-api Ready Ready Preview Mar 7, 2026 1:02am

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

Warning

Rate limit exceeded

@sweetmantech has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 35 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 039fc5e0-fac3-4729-ba74-986214bef449

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc0c3a and dc70815.

📒 Files selected for processing (1)
  • lib/coding-agent/handlers/onMergeAction.ts
📝 Walkthrough

Walkthrough

This PR introduces a persistent PR state management system for the coding agent, utilizing Redis for shared state storage. It adds utilities for building state keys, retrieving and persisting PR state, and a resolver function that merges thread state with shared PR state. Existing handlers are updated to integrate state persistence and resolution throughout their workflows.

Changes

Cohort / File(s) Summary
PR State Persistence Infrastructure
lib/coding-agent/prState/buildPRStateKey.ts, lib/coding-agent/prState/setCodingAgentPRState.ts, lib/coding-agent/prState/getCodingAgentPRState.ts, lib/coding-agent/prState/deleteCodingAgentPRState.ts, lib/coding-agent/prState/types.ts, lib/coding-agent/prState/index.ts
New modules establishing Redis-backed PR state management, including key construction, CRUD operations, type definitions, and a public API barrel export.
Handler Integration with State Persistence
lib/coding-agent/handleCodingAgentCallback.ts, lib/coding-agent/handlePRCreated.ts, lib/coding-agent/handlers/handleFeedback.ts
Updates to handlers to call setCodingAgentPRState after PR creation and feedback processing, persisting state when branch and PR data are present.
Handler Integration with State Resolution
lib/coding-agent/handlers/onNewMention.ts
Introduces PR context extraction from message and delegates state resolution to resolvePRState prior to feedback handling, enabling context-aware state retrieval.
Cleanup and Merge Completion
lib/coding-agent/handlers/onMergeAction.ts
Adds deleteCodingAgentPRState call after merge completion to clear persisted PR state when branch and repo data are available.
State Resolution Utility
lib/coding-agent/resolvePRState.ts
New resolver function that prioritizes thread state but falls back to shared PR state from Redis via PR context, with type definitions for PRContext.

Sequence Diagram

sequenceDiagram
    participant Handler as Handler (e.g., onNewMention)
    participant Message as Message/Thread
    participant PRContext as PR Context<br/>(repo, branch)
    participant ThreadState as Thread State
    participant Redis as Redis
    participant Resolver as resolvePRState
    
    Handler->>Handler: Extract repo, branch from message.raw
    Handler->>PRContext: Build PRContext object
    Handler->>Resolver: Call resolvePRState(thread, prContext)
    
    Resolver->>ThreadState: Check thread.state
    alt Thread State Exists
        Resolver-->>Handler: Return CodingAgentThreadState
    else Thread State Missing
        Resolver->>Redis: getCodingAgentPRState(repo, branch)
        Redis-->>Resolver: Fetch shared PR state
        alt Shared State Found
            Resolver->>Resolver: Map to CodingAgentThreadState
            Resolver-->>Handler: Return mapped state
        else Shared State Not Found
            Resolver-->>Handler: Return null
        end
    end
    
    Handler->>Handler: Use resolved state for feedback handling
Loading
sequenceDiagram
    participant Handler as PR Creation Handler
    participant Thread as Thread/State
    participant Redis as Redis
    participant Merge as onMergeAction
    
    Handler->>Handler: Process PR creation
    Handler->>Thread: Store prs array in thread
    Handler->>Redis: setCodingAgentPRState(repo, branch, state)
    Redis->>Redis: Build key, serialize, store
    Note over Redis: PR state persists across sessions
    
    Merge->>Thread: Detect merge completion
    Merge->>Redis: deleteCodingAgentPRState(repo, branch)
    Redis->>Redis: Build key, delete entry
    Note over Redis: Clean up after merge
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🎭 A state that once was fleeting,
Now persists in Redis, greeting
Each handler with a Redis key—
Repo meets branch in harmony. 🔑
State lives long, survives the test,
Shared and resolved at its best! 🚀

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Implementation violates DRY, SRP, error handling, and type safety principles across state persistence and error handling patterns. Refactor state persistence to iterate all repos, wrap setCodingAgentPRState calls in try-catch, use narrower types for shared state, correct JSDoc references, and implement safe key delimiters.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sweetmantech/myc-4434-github-pr-aware-coding-agent-flow

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.

One function per file under lib/coding-agent/prState/:
- buildPRStateKey.ts
- getCodingAgentPRState.ts
- setCodingAgentPRState.ts
- deleteCodingAgentPRState.ts
- types.ts
- index.ts (barrel re-export)

Barrel index preserves existing import paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@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: 7

🧹 Nitpick comments (1)
lib/coding-agent/prState/setCodingAgentPRState.ts (1)

12-19: Consider adding a TTL to prevent orphaned keys from accumulating.

The Redis key is set without an expiration. If a PR is abandoned, fails, or the merge callback never fires, this key persists indefinitely. Over time, orphaned keys could accumulate and consume Redis memory.

Consider adding a reasonable TTL (e.g., 7-30 days) to ensure keys eventually expire:

♻️ Proposed fix to add TTL
 export async function setCodingAgentPRState(
   repo: string,
   branch: string,
   state: CodingAgentPRState,
 ): Promise<void> {
   const key = buildPRStateKey(repo, branch);
-  await redis.set(key, JSON.stringify(state));
+  const TTL_SECONDS = 60 * 60 * 24 * 14; // 14 days
+  await redis.set(key, JSON.stringify(state), "EX", TTL_SECONDS);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/prState/setCodingAgentPRState.ts` around lines 12 - 19, The
Redis key set in setCodingAgentPRState currently persists forever; update
setCodingAgentPRState to set an expiration (TTL) after storing the JSON state
(use buildPRStateKey to get the key) — e.g., set a 7–30 day TTL (30 days
recommended) by using redis.set with an EX option or calling redis.expire
immediately after redis.set so orphaned PR state keys are automatically removed.
Ensure the TTL value is configurable or documented so it can be tuned later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/coding-agent/handleCodingAgentCallback.ts`:
- Around line 67-76: The current call to setCodingAgentPRState after
thread.setState and thread.post can fail and leave thread state inconsistent
with shared PR state; update the code around setCodingAgentPRState to guard
against persistence failures by wrapping the call in a try-catch, logging the
error (include contextual info like state.prs[0].repo and state.branch) and
continuing so the thread remains usable, or (alternatively) move persistence
before thread operations and implement a rollback path that clears or reverts
thread state if setCodingAgentPRState fails; reference setCodingAgentPRState,
thread.setState, thread.post and state.prs/state.branch to locate and modify the
logic.

In `@lib/coding-agent/handlePRCreated.ts`:
- Around line 27-34: The current logic only persists shared state for prs[0],
causing other PR repos to miss the Redis entry; update the block that calls
setCodingAgentPRState to iterate over all entries in prs (or map them) and write
the same object ({ status: "pr_created", snapshotId: body.snapshotId, branch:
body.branch, repo: pr.repo, prs }) for each pr.repo, using await or Promise.all
to ensure all Redis writes complete; use the existing setCodingAgentPRState
function and the prs variable so every PR repo gets its own persisted state
rather than only the first.

In `@lib/coding-agent/handlers/handleFeedback.ts`:
- Around line 25-34: The thread can get stuck in "updating" if
setCodingAgentPRState throws; wrap the persistence call in a try/catch so
failures don't block progress: call thread.setState({status:"updating"}) then
call triggerUpdatePR(...) (or call triggerUpdatePR first) and make
setCodingAgentPRState a best-effort operation inside a try/catch that logs
errors (but does not re-throw), or if you prefer keep the current order then
catch errors from setCodingAgentPRState and restore the thread state (via
thread.setState) or set a fallback status; update the code locations referencing
thread.setState, setCodingAgentPRState, and triggerUpdatePR accordingly.

In `@lib/coding-agent/handlers/onMergeAction.ts`:
- Around line 53-55: The call to deleteCodingAgentPRState in the onMergeAction
handler must be made best-effort so Redis unavailability won’t abort the
handler; wrap the await deleteCodingAgentPRState(state.prs[0].repo,
state.branch) call in a try/catch (while keeping the existing if (state.branch
&& state.prs?.[0]?.repo) check), swallow the error (do not rethrow) and log the
failure (including the error) so the merge summary still posts even if cleanup
fails.

In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 13-14: Update the JSDoc above the onNewMention handler to
accurately reflect where the PR comment lookup fields come from: change the
description that currently says message.meta contains { repo, branch } to state
that message.raw contains { repo, branch } (or both if applicable), and include
any necessary note about precedence or format; ensure the JSDoc references the
exact symbols used in the code (message.raw, repo, branch, and the onNewMention
handler) so consumers read the correct integration contract.

In `@lib/coding-agent/prState/buildPRStateKey.ts`:
- Around line 9-11: The key-building in buildPRStateKey currently concatenates
KEY_PREFIX, repo and branch with ":" which can collide if repo or branch contain
":"; update buildPRStateKey to safely encode or delimit segments (e.g.,
URL-encode repo and branch with encodeURIComponent or replace ":" with a safe
delimiter like "|" before concatenation) so produced keys are unambiguous and
reversible; ensure any callers that parse keys (if any) are updated to decode
the chosen encoding or expect the new delimiter and add unit tests covering
repo/branch values containing ":" and other edge chars.

In `@lib/coding-agent/resolvePRState.ts`:
- Around line 29-35: The fallback in resolvePRState (in
lib/coding-agent/resolvePRState.ts) returns a fabricated prompt: "" which makes
CodingAgentThreadState appear valid; update the domain types and resolver so the
prompt is not faked: either add an optional prompt field to CodingAgentPRState
and populate the returned object from prState.prompt, or change resolvePRState
to return a narrower union/resolved type that models "shared PR context without
prompt" (so CodingAgentThreadState is not constructed when prompt is missing).
Adjust the return type and callers of resolvePRState accordingly to preserve
TypeScript safety and avoid passing an empty prompt downstream.

---

Nitpick comments:
In `@lib/coding-agent/prState/setCodingAgentPRState.ts`:
- Around line 12-19: The Redis key set in setCodingAgentPRState currently
persists forever; update setCodingAgentPRState to set an expiration (TTL) after
storing the JSON state (use buildPRStateKey to get the key) — e.g., set a 7–30
day TTL (30 days recommended) by using redis.set with an EX option or calling
redis.expire immediately after redis.set so orphaned PR state keys are
automatically removed. Ensure the TTL value is configurable or documented so it
can be tuned later.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dc13be22-e67c-4121-a71b-59fafeb62350

📥 Commits

Reviewing files that changed from the base of the PR and between 00f5459 and 0fc0c3a.

⛔ Files ignored due to path filters (10)
  • lib/coding-agent/__tests__/buildPRStateKey.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/deleteCodingAgentPRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/getCodingAgentPRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handleCodingAgentCallback.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handlePRCreated.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handlers.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/onMergeAction.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/onSubscribedMessage.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/resolvePRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/setCodingAgentPRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (12)
  • lib/coding-agent/handleCodingAgentCallback.ts
  • lib/coding-agent/handlePRCreated.ts
  • lib/coding-agent/handlers/handleFeedback.ts
  • lib/coding-agent/handlers/onMergeAction.ts
  • lib/coding-agent/handlers/onNewMention.ts
  • lib/coding-agent/prState/buildPRStateKey.ts
  • lib/coding-agent/prState/deleteCodingAgentPRState.ts
  • lib/coding-agent/prState/getCodingAgentPRState.ts
  • lib/coding-agent/prState/index.ts
  • lib/coding-agent/prState/setCodingAgentPRState.ts
  • lib/coding-agent/prState/types.ts
  • lib/coding-agent/resolvePRState.ts

Comment on lines +67 to +76

if (state?.branch && state?.prs?.length) {
await setCodingAgentPRState(state.prs[0].repo, state.branch, {
status: "pr_created",
snapshotId: validated.snapshotId,
branch: state.branch,
repo: state.prs[0].repo,
prs: state.prs,
});
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider error handling for shared state persistence failure.

If setCodingAgentPRState fails after thread.setState and thread.post have succeeded, the thread state will be "pr_created" but the shared PR state may be missing or stale. This creates a potential inconsistency between the two state stores.

Options to consider:

  1. Wrap in try-catch and log the failure (non-critical if thread state is source of truth)
  2. Persist shared state before thread operations and rollback on failure

If shared state is meant as a fallback for cross-platform scenarios, option 1 is likely sufficient:

🛡️ Proposed defensive handling
       if (state?.branch && state?.prs?.length) {
-        await setCodingAgentPRState(state.prs[0].repo, state.branch, {
-          status: "pr_created",
-          snapshotId: validated.snapshotId,
-          branch: state.branch,
-          repo: state.prs[0].repo,
-          prs: state.prs,
-        });
+        try {
+          await setCodingAgentPRState(state.prs[0].repo, state.branch, {
+            status: "pr_created",
+            snapshotId: validated.snapshotId,
+            branch: state.branch,
+            repo: state.prs[0].repo,
+            prs: state.prs,
+          });
+        } catch (err) {
+          console.error("[coding-agent] Failed to persist shared PR state:", err);
+        }
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (state?.branch && state?.prs?.length) {
await setCodingAgentPRState(state.prs[0].repo, state.branch, {
status: "pr_created",
snapshotId: validated.snapshotId,
branch: state.branch,
repo: state.prs[0].repo,
prs: state.prs,
});
}
if (state?.branch && state?.prs?.length) {
try {
await setCodingAgentPRState(state.prs[0].repo, state.branch, {
status: "pr_created",
snapshotId: validated.snapshotId,
branch: state.branch,
repo: state.prs[0].repo,
prs: state.prs,
});
} catch (err) {
console.error("[coding-agent] Failed to persist shared PR state:", err);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handleCodingAgentCallback.ts` around lines 67 - 76, The
current call to setCodingAgentPRState after thread.setState and thread.post can
fail and leave thread state inconsistent with shared PR state; update the code
around setCodingAgentPRState to guard against persistence failures by wrapping
the call in a try-catch, logging the error (include contextual info like
state.prs[0].repo and state.branch) and continuing so the thread remains usable,
or (alternatively) move persistence before thread operations and implement a
rollback path that clears or reverts thread state if setCodingAgentPRState
fails; reference setCodingAgentPRState, thread.setState, thread.post and
state.prs/state.branch to locate and modify the logic.

Comment on lines +27 to +34
if (body.branch && prs.length) {
await setCodingAgentPRState(prs[0].repo, body.branch, {
status: "pr_created",
snapshotId: body.snapshotId,
branch: body.branch,
repo: prs[0].repo,
prs,
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Persist shared state for every PR repo, not just prs[0].

The Redis key is repo-scoped, but this only writes the entry for the first PR’s repo. If one run opens PRs across multiple repos, comments on the other PRs won’t find shared state, and the cleanup path will leave those repo keys inconsistent.

Suggested change
   if (body.branch && prs.length) {
-    await setCodingAgentPRState(prs[0].repo, body.branch, {
-      status: "pr_created",
-      snapshotId: body.snapshotId,
-      branch: body.branch,
-      repo: prs[0].repo,
-      prs,
-    });
+    for (const repo of new Set(prs.map(pr => pr.repo))) {
+      await setCodingAgentPRState(repo, body.branch, {
+        status: "pr_created",
+        snapshotId: body.snapshotId,
+        branch: body.branch,
+        repo,
+        prs,
+      });
+    }
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (body.branch && prs.length) {
await setCodingAgentPRState(prs[0].repo, body.branch, {
status: "pr_created",
snapshotId: body.snapshotId,
branch: body.branch,
repo: prs[0].repo,
prs,
});
if (body.branch && prs.length) {
for (const repo of new Set(prs.map(pr => pr.repo))) {
await setCodingAgentPRState(repo, body.branch, {
status: "pr_created",
snapshotId: body.snapshotId,
branch: body.branch,
repo,
prs,
});
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handlePRCreated.ts` around lines 27 - 34, The current logic
only persists shared state for prs[0], causing other PR repos to miss the Redis
entry; update the block that calls setCodingAgentPRState to iterate over all
entries in prs (or map them) and write the same object ({ status: "pr_created",
snapshotId: body.snapshotId, branch: body.branch, repo: pr.repo, prs }) for each
pr.repo, using await or Promise.all to ensure all Redis writes complete; use the
existing setCodingAgentPRState function and the prs variable so every PR repo
gets its own persisted state rather than only the first.

Comment on lines 25 to +34
if (state?.status === "pr_created" && state.snapshotId && state.branch && state.prs?.length) {
await thread.setState({ status: "updating" });
await setCodingAgentPRState(state.prs[0].repo, state.branch, {
status: "updating",
snapshotId: state.snapshotId,
branch: state.branch,
repo: state.prs[0].repo,
prs: state.prs,
});

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Thread can be left stuck in "updating" state if setCodingAgentPRState fails.

The current flow is:

  1. thread.setState({ status: "updating" }) — mutates thread state
  2. setCodingAgentPRState(...) — can throw (Redis/JSON failure)
  3. triggerUpdatePR(...) — never reached if step 2 throws

If setCodingAgentPRState fails, the thread remains in "updating" status permanently with no recovery path. Users will see "I'm still working on this" forever.

Consider wrapping the shared state persistence in try-catch (non-blocking) or moving it after triggerUpdatePR:

🐛 Proposed fix: Make shared state persistence non-blocking
   if (state?.status === "pr_created" && state.snapshotId && state.branch && state.prs?.length) {
     await thread.setState({ status: "updating" });
-    await setCodingAgentPRState(state.prs[0].repo, state.branch, {
-      status: "updating",
-      snapshotId: state.snapshotId,
-      branch: state.branch,
-      repo: state.prs[0].repo,
-      prs: state.prs,
-    });
-
+    
     const handle = await triggerUpdatePR({
       feedback: messageText,
       snapshotId: state.snapshotId,
       branch: state.branch,
       repo: state.prs[0].repo,
       callbackThreadId: thread.id,
     });

     const card = buildTaskCard("Updating PRs", "Got your feedback. Updating the PRs...", handle.id);
     await thread.post({ card });
+
+    // Persist shared state after critical operations succeed (non-blocking)
+    setCodingAgentPRState(state.prs[0].repo, state.branch, {
+      status: "updating",
+      snapshotId: state.snapshotId,
+      branch: state.branch,
+      repo: state.prs[0].repo,
+      prs: state.prs,
+    }).catch((err) => {
+      console.error("[coding-agent] Failed to persist shared PR state:", err);
+    });
+
     return true;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handlers/handleFeedback.ts` around lines 25 - 34, The thread
can get stuck in "updating" if setCodingAgentPRState throws; wrap the
persistence call in a try/catch so failures don't block progress: call
thread.setState({status:"updating"}) then call triggerUpdatePR(...) (or call
triggerUpdatePR first) and make setCodingAgentPRState a best-effort operation
inside a try/catch that logs errors (but does not re-throw), or if you prefer
keep the current order then catch errors from setCodingAgentPRState and restore
the thread state (via thread.setState) or set a fallback status; update the code
locations referencing thread.setState, setCodingAgentPRState, and
triggerUpdatePR accordingly.

Comment on lines +53 to +55
if (state.branch && state.prs?.[0]?.repo) {
await deleteCodingAgentPRState(state.prs[0].repo, state.branch);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make shared-state cleanup best-effort.

This delete happens after the GitHub merges are done but before the merge summary is posted. If Redis is briefly unavailable here, the handler throws and the user gets no results even though the PRs were already merged.

Suggested change
     await thread.setState({ status: "merged" });
     if (state.branch && state.prs?.[0]?.repo) {
-      await deleteCodingAgentPRState(state.prs[0].repo, state.branch);
+      try {
+        await deleteCodingAgentPRState(state.prs[0].repo, state.branch);
+      } catch (error) {
+        console.error("[coding-agent] failed to delete shared PR state:", error);
+      }
     }
     await thread.post(`Merge results:\n${results.map(r => `- ${r}`).join("\n")}`);
As per coding guidelines, `lib/**/*.ts`: For domain functions, ensure: Proper error handling.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (state.branch && state.prs?.[0]?.repo) {
await deleteCodingAgentPRState(state.prs[0].repo, state.branch);
}
if (state.branch && state.prs?.[0]?.repo) {
try {
await deleteCodingAgentPRState(state.prs[0].repo, state.branch);
} catch (error) {
console.error("[coding-agent] failed to delete shared PR state:", error);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handlers/onMergeAction.ts` around lines 53 - 55, The call to
deleteCodingAgentPRState in the onMergeAction handler must be made best-effort
so Redis unavailability won’t abort the handler; wrap the await
deleteCodingAgentPRState(state.prs[0].repo, state.branch) call in a try/catch
(while keeping the existing if (state.branch && state.prs?.[0]?.repo) check),
swallow the error (do not rethrow) and log the failure (including the error) so
the merge summary still posts even if cleanup fails.

Comment on lines +13 to +14
* For GitHub PR comments, message.meta may contain { repo, branch } to look up
* the shared PR state key when thread state is empty.
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update the JSDoc to match the actual field.

The new lookup reads repo/branch from message.raw, but the comment still points to message.meta. That mismatch makes the integration contract easy to wire incorrectly later.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/handlers/onNewMention.ts` around lines 13 - 14, Update the
JSDoc above the onNewMention handler to accurately reflect where the PR comment
lookup fields come from: change the description that currently says message.meta
contains { repo, branch } to state that message.raw contains { repo, branch }
(or both if applicable), and include any necessary note about precedence or
format; ensure the JSDoc references the exact symbols used in the code
(message.raw, repo, branch, and the onNewMention handler) so consumers read the
correct integration contract.

Comment on lines +9 to +11
export function buildPRStateKey(repo: string, branch: string): string {
return `${KEY_PREFIX}:${repo}:${branch}`;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider escaping or using a safer delimiter to prevent key collisions.

The current format coding-agent:pr:{repo}:{branch} uses : as a delimiter, but repo/branch names could theoretically contain : characters, creating ambiguous keys. For example, foo:bar repo with main branch would be indistinguishable from foo repo with bar:main branch.

This is unlikely in practice but worth noting. If you want to be defensive, consider URL-encoding the segments or using a delimiter that's invalid in Git ref names (e.g., \0 isn't practical, but | is less common).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/prState/buildPRStateKey.ts` around lines 9 - 11, The
key-building in buildPRStateKey currently concatenates KEY_PREFIX, repo and
branch with ":" which can collide if repo or branch contain ":"; update
buildPRStateKey to safely encode or delimit segments (e.g., URL-encode repo and
branch with encodeURIComponent or replace ":" with a safe delimiter like "|"
before concatenation) so produced keys are unambiguous and reversible; ensure
any callers that parse keys (if any) are updated to decode the chosen encoding
or expect the new delimiter and add unit tests covering repo/branch values
containing ":" and other edge chars.

Comment on lines +29 to +35
return {
status: prState.status,
prompt: "",
branch: prState.branch,
snapshotId: prState.snapshotId,
prs: prState.prs,
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t fabricate a blank prompt in the fallback state.

CodingAgentThreadState requires a real prompt, but shared PR state does not carry one. Returning prompt: "" makes missing data look valid and can send an incomplete state through the feedback/update path. Either persist prompt in CodingAgentPRState as well, or return a narrower resolved-state type that models “shared PR context without prompt” explicitly.

As per coding guidelines, lib/**/*.ts: For domain functions, ensure: Use TypeScript for type safety.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/coding-agent/resolvePRState.ts` around lines 29 - 35, The fallback in
resolvePRState (in lib/coding-agent/resolvePRState.ts) returns a fabricated
prompt: "" which makes CodingAgentThreadState appear valid; update the domain
types and resolver so the prompt is not faked: either add an optional prompt
field to CodingAgentPRState and populate the returned object from
prState.prompt, or change resolvePRState to return a narrower union/resolved
type that models "shared PR context without prompt" (so CodingAgentThreadState
is not constructed when prompt is missing). Adjust the return type and callers
of resolvePRState accordingly to preserve TypeScript safety and avoid passing an
empty prompt downstream.

Co-Authored-By: Claude Opus 4.6 <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