feat: Slack always uses durable execution mode with continuation stream

[anubra266]

Summary

• Force executionMode: 'durable' on all Slack chat requests, overriding per-agent config
• After durable tool approval via Slack button, consume the continuation SSE stream from /run/api/chat and post the result back to the Slack thread

[changeset-bot]

⚠️ No Changeset found

Latest commit: c611c59

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
agents-api	Ready	Preview, Comment	Apr 4, 2026 0:10am
agents-manage-ui	Ready	Preview, Comment	Apr 4, 2026 0:10am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
agents-docs	Skipped		Apr 4, 2026 0:10am

[pullfrog]

TL;DR — Forces all Slack agent interactions into durable execution mode so tool approvals suspend and resume correctly, then consumes the continuation SSE stream after approval to post the agent's follow-up response back into the Slack thread. Also adds delegated tool approval propagation — when a sub-agent needs approval during delegation, the request bubbles up through the parent agent's durable workflow.

Key changes

• Force durable execution for Slack — adds executionMode: 'durable' to every Slack chat request, overriding per-agent configuration
• Consume continuation stream after tool approval — new consumeApprovalContinuationStream() reads the SSE response from the approval endpoint, accumulates text deltas, and posts the result to the Slack thread
• Propagate delegated sub-agent tool approvals — when a sub-agent returns a durable-approval-required artifact, the parent agent detects it, suspends with the sub-agent's tool call ID as hook token, and forwards the approval decision back via delegation metadata
• Convert Markdown to Slack mrkdwn — applies markdownToMrkdwn() to continuation responses before posting, avoiding double-escaping that would destroy Slack links
• Persist delegation tool results in durable mode — stops skipping history storage for delegate_to_ tools when inside a durable workflow so post-approval callLlmStep has context
• Add post-approval continuation prompt — uses a fixed prompt after approval instead of the original user message, relying on tool results already in conversation history
• Restore OTel tracing in durable steps — re-applies otelContext.with / tracer.startActiveSpan wrappers in callLlmStep and executeToolStep, integrating the new durable delegation features within the existing tracing infrastructure
• Initialize credential stores in durable mode — buildAgentForStep() now creates a CredentialStoreRegistry so MCP tools have working credentials during durable execution
• Fix delegation baseUrl — appends /run/agents to apiBaseUrl for durable-mode delegation URLs so A2A calls resolve correctly
• Add error resilience around agent.generate() — if generate() throws while a pendingDurableApproval is set, the approval artifact is still returned instead of failing the workflow
• Extract buildDurableApprovalResult() helper — centralizes approval artifact construction, including an error-path fallback so pending approvals aren't lost when generate() throws
• Stream delegated approval UI events — writes tool-input and approval-request SSE events using the sub-agent's actual tool name/args so the client sees the real tool, not delegate_to_*
• Remove delegated agent approval UI publishing — deletes the competing toolApprovalUiBus publish path for delegated agents since durable mode handles approval through the parent workflow

_{Summary ｜ 11 files ｜ 4 commits ｜ base: main ← feat/slack-durable-execution}

Durable execution mode forced for all Slack interactions

Before: Slack used whatever execution mode each agent was configured with — tool approvals could fail silently in non-durable mode.
After: Every Slack chat request sets executionMode: 'durable', ensuring tool calls always suspend and resume correctly regardless of agent config.

A single line in streamAgentResponse() adds the override. The durable workflow engine handles suspension, hook registration, and resumption — Slack just needs to opt in.

streaming.ts

Continuation stream consumption after tool approval

Before: After a user approved a tool in Slack, no follow-up response was posted — the agent's continuation output was discarded.
After: The approval response's SSE stream is read in full, text deltas are accumulated, converted to Slack mrkdwn, and posted back to the thread — with error feedback when the stream fails.

consumeApprovalContinuationStream() posts a thinking indicator, reads the text/event-stream body with a 120-second timeout, parses data: lines for text-delta events, then replaces the thinking message with the formatted response including an agent attribution context block. If the stream errors out before accumulating any text, a user-facing error message is posted instead.

Why was escapeSlackMrkdwn removed from the pipeline?

The original code ran escapeSlackMrkdwn(markdownToMrkdwn(fullText)). The inner call converts Markdown links text to Slack's <url|text> format. The outer escapeSlackMrkdwn then escaped the angle brackets to </>, destroying the links. The fix drops the outer call since markdownToMrkdwn already handles link text escaping internally via escapeSlackLinkText.

block-actions.ts · streaming.ts

Delegated sub-agent tool approval propagation

Before: Only direct tool calls on the primary agent could trigger durable approval — sub-agent tool approvals had no propagation path.
After: Sub-agent approval requests bubble up as durable-approval-required artifacts; the parent suspends with the sub-agent's tool call ID and forwards the decision back via approved_tool_calls delegation metadata.

The flow spans several files:

Step	Where
Sub-agent returns durable-approval-required artifact	generateTaskHandler.ts via buildDurableApprovalResult()
Parent's tool-wrapper detects artifact, sets pendingDurableApproval.delegatedApproval	tool-wrapper.ts
Workflow suspends, streams approval UI to client via SSE helpers	agentExecutionSteps.ts
After approval, decision forwarded as delegatedToolApproval on AgentRunContext	agentExecution.ts → agentExecutionSteps.ts
Re-delegation includes approved_tool_calls in delegation metadata	relationTools.ts
Delegated agent approval UI removed from sub-agent context	tool-approval.ts

Why use the sub-agent's tool call ID as hook token?

The Slack approval button callback fires with the tool call ID shown in the approval UI. For delegated approvals, that's the sub-agent's tool call ID — not the parent's delegate_to_ call. Using the sub-agent's ID ensures the hook matches when Slack sends the approval.

tool-wrapper.ts · agentExecutionSteps.ts · relationTools.ts · generateTaskHandler.ts

OTel tracing restored with durable delegation features layered in

Before: An intermediate commit removed OTel otelContext.with / tracer.startActiveSpan wrappers from callLlmStep and executeToolStep.
After: Tracing is restored — both step functions retain their original OTel span wrappers, with the new durable delegation logic integrated inside the existing tracing infrastructure.

The latest commit (ecd719c) reset agentExecutionSteps.ts to main's version and re-applied the durable delegation features on top, preserving setSpanWithError, errorOp, and the full span lifecycle management. The baseUrl fix (/run/agents suffix) and CredentialStoreRegistry initialization are also integrated within the restored structure.

agentExecutionSteps.ts · agentExecution.ts

Delegation tool results persisted in durable mode

Before: delegate_to_* tool calls were always treated as internal — their results were never saved to conversation history.
After: In durable workflows, delegation results are persisted so the next LLM call has the context it needs and doesn't re-delegate in a loop.

The skipHistoryStorage flag replaces the blanket isInternalToolForUi check, carving out an exception when durableWorkflowRunId is present. A companion change in callLlmStep uses a fixed continuation prompt ("Continue the conversation. The tool results above contain the information needed to respond to the user.") on post-approval iterations instead of repeating the original user message.

tool-wrapper.ts · agentExecutionSteps.ts

  ^{｜ View workflow run ｜ Triggered by Pullfrog ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

Solid feature — the overall architecture of bubbling durable-approval-required artifacts up through delegation chains and consuming the continuation SSE stream from Slack is well thought-out. Two actionable issues below: one bug that will break Slack link rendering at runtime, and a console.info that should use the structured logger.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[claude]

PR Review Summary

(5) Total Issues | Risk: Medium

🟠⚠️ Major (2) 🟠⚠️

🟠 1) agentExecutionSteps.ts OTEL tracing removed from durable workflow steps

files: agentExecutionSteps.ts

Issue: The refactor removes all OpenTelemetry context propagation and span recording from callLlmStep and executeToolStep. Previously, these functions wrapped execution in otelContext.with(ctxWithBaggage, ...) to propagate conversation/tenant/project/agent IDs through spans, and recorded errors via setSpanWithError(span, rootCause). The imports for SPAN_NAMES, otelContext, propagation, setSpanWithError, and tracer are all removed.

Why: This eliminates observability for durable workflow execution paths:

• Span-level error recording is gone — failures won't appear in tracing dashboards
• Baggage propagation is removed — correlation IDs won't flow through the trace
• The errorOp streaming to clients is also removed, so clients may not see error states

If this is intentional (delegating tracing to the workflow engine infrastructure), it should be documented. If not, this is a significant observability regression.

Fix: Either:

1. Restore OTEL tracing with baggage propagation and error span recording
2. Document that the 'use step' workflow infrastructure provides equivalent tracing
3. Add a follow-up issue to re-instrument these paths once the durable workflow tracing story is clear

Refs:

• Removed OTEL imports
• Original otelContext.with() usage (on main branch)

---

🟠 2) agentExecutionSteps.ts Error handling removed — task status not updated to 'failed'

files: agentExecutionSteps.ts

Issue: The original code had explicit catch blocks in both callLlmStep and executeToolStep that:

1. Wrote errorOp() to the SSE stream so clients see the failure
2. Updated task status to 'failed' with error metadata in the database
3. Logged and recorded spans before re-throwing

The new code removes these error handlers entirely. Errors now propagate up without updating task status or notifying the stream.

Why: Failed executions won't be marked as 'failed' in the tasks table, and clients won't receive error events through the stream. The parent workflow's markWorkflowFailedStep only updates workflow_executions status, not the individual task status. This could leave tasks in an ambiguous state.

Fix: Restore the error handling that:

1. Updates task status to 'failed' with error metadata
2. Streams errorOp() to clients before the stream closes
3. Or confirm that the workflow-level error handler is sufficient and document the intentional behavior change

Refs:

• executeToolStep without error handling

---

Inline Comments:

• 🟠 Major: agentExecutionSteps.ts:509-521 Exception swallowed when pendingDurableApproval exists
• 🟠 Major: block-actions.ts:506-512 Stream read errors leave users without feedback

🟡 Minor (3) 🟡

Inline Comments:

• 🟡 Minor: agentExecution.ts:85-93 Use structured logger instead of console.info
• 🟡 Minor: block-actions.ts:541 escapeSlackMrkdwn destroys Slack links
• 🟡 Minor: block-actions.ts:497-504 Empty catch block silently swallows JSON parse errors
• 🟡 Minor: block-actions.ts:476-479 Timeout does not release reader lock after cancellation

💭 Consider (2) 💭

💭 1) tool-wrapper.ts:196-220 Helper functions defined inside hot path

Issue: findApprovalRequired and findApprovalInArtifacts are defined inline within the execute wrapper, creating new function instances on every tool execution.

Why: While JavaScript engines can optimize this, these pure functions could be moved to module scope for clarity. The nested search logic also duplicates structure knowledge that exists in A2A types.

Fix: Consider moving to module scope and importing A2A artifact type definitions to make the structure explicit rather than using Record<string, unknown> casts.

Refs: tool-wrapper.ts:196-220

💭 2) agent-types.ts:273-290 Type shape duplicated across 3 locations

Issue: The pendingDurableApproval and delegatedApproval type shapes are defined inline in:

1. agent-types.ts (AgentRunContext)
2. Agent.ts (getPendingDurableApproval return type)
3. agentExecutionSteps.ts (DelegatedApprovalContext)

Why: Changes to the shape require updates in multiple places. Consider extracting to a shared type definition.

Fix: Define DurableApprovalInfo and DelegatedApprovalContext once in agent-types.ts and import elsewhere.

Refs:

• agent-types.ts:273-290
• Agent.ts:161-172

🕐 Pending Recommendations (1)

• 🟡 block-actions.ts:541 escapeSlackMrkdwn bug already flagged by Pullfrog

---

🚫 REQUEST CHANGES

Summary: The core durable approval propagation logic looks well-designed — bubbling durable-approval-required artifacts through delegation chains and consuming continuation streams from Slack is architecturally sound. However, the removal of OTEL tracing and error handling from the workflow steps is a significant observability/reliability regression that should be addressed before merge. The Slack link rendering bug (escapeSlackMrkdwn) flagged by Pullfrog also needs fixing.

Discarded (8)

Location	Issue	Reason Discarded
generateTaskHandler.ts:56-89	buildDurableApprovalResult extraction	Valid DRY refactoring — used in 2 distinct locations, consolidates 20 lines
agent-types.ts:270-290	Type extensions for delegated approval	Load-bearing types required for the feature
tool-approval.ts:99-106	Removed toolApprovalUiBus publish for delegated agents	Intentional — durable mode handles approval UI via agentExecutionSteps
agentExecutionSteps.ts:580-607	Delegated approval SSE streaming	Feature code, not a defect
relationTools.ts:426-458	delegationMeta construction	Correct implementation of approval propagation
streaming.ts:198	executionMode: 'durable' addition	Core feature — forces durable mode for Slack
tool-wrapper.ts:112-116	skipHistoryStorage for durable delegation	Correct fix to prevent delegation loops
agentExecution.ts:49	isPostApproval state tracking	Valid state management for continuation prompts

Reviewers (6)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-standards	5	1	0	0	1	0	3
pr-review-errors	6	0	0	0	3	0	3
pr-review-sre	5	1	0	0	1	0	3
pr-review-types	4	0	1	0	0	0	3
pr-review-consistency	4	1	0	0	1	1	1
pr-review-precision	8	0	1	0	0	0	7
Total	32	3	2	0	6	1	20

[github-actions]

Preview URLs

Use these stable preview aliases for testing this PR:

• UI: https://pr-3006-ui.preview.inkeep.com
• API: https://pr-3006-api.preview.inkeep.com
• API health: https://pr-3006-api.preview.inkeep.com/health

These point to the same Vercel preview deployment as the bot comment, but they stay stable and easier to find.

Raw Vercel deployment URLs

• UI deployment: https://agents-manage-gzp3ececf.preview.inkeep.com
• API deployment: https://agents-ar7pd37p7.preview.inkeep.com

[claude]

PR Review Summary

(0) New Issues | Risk: Low (delta-only review)

This is a delta review covering commit a010a0e66 — the only changes since the last automated review.

✅ Issues Addressed in This Commit

The commit successfully addresses 3 prior review findings:

Prior Issue	Fix Applied
🟡 escapeSlackMrkdwn destroys Slack links (Pullfrog, Claude)	✅ Removed escapeSlackMrkdwn() call — now uses markdownToMrkdwn() only
🟠 Stream read errors leave users without feedback (Claude)	✅ Added error message posted to Slack when fullText is empty after stream failure
🟡 Missing context in error log ([implicit])	✅ Added channel and threadTs to error log context

🕐 Pending Recommendations (4)

Prior review findings that remain unaddressed (outside the scope of this delta commit):

• 🟠 agentExecutionSteps.ts OTEL tracing removed from durable workflow steps
• 🟠 agentExecutionSteps.ts Error handling removed — task status not updated to 'failed'
• 🟡 agentExecution.ts:93 Use structured logger instead of console.info
• 🟡 block-actions.ts:510-512 Empty catch block silently swallows JSON parse errors (consider adding debug logging)

---

🚫 REQUEST CHANGES

Summary: The delta commit correctly fixes 3 prior review issues — good work! The escapeSlackMrkdwn bug is resolved and users now get feedback when stream consumption fails. However, the Major-severity items from the prior review (OTEL tracing removal, error handling removal in workflow steps) remain unaddressed. These observability/reliability concerns should be resolved before merge.

Reviewers (1)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
orchestrator (delta)	0	0	0	0	0	4	0
Total	0	0	0	0	0	4	0

Note: Delta review — no sub-reviewers dispatched. Only analyzed the 1 file changed since last review.

[claude]

PR Review Summary

(1) Total Issues | Risk: Low

This is a delta review covering the latest commit 6a7c806b6 — "Restore OTEL tracing that was dropped during rebase conflict resolution".

✅ Prior Issues Addressed

Great work addressing the major concerns from previous reviews:

Prior Issue	Status	Details
🟠 OTEL tracing removed from durable workflow steps	✅ Fixed	Commit 6a7c806b6 restores otelContext.with(), tracer.startActiveSpan, setSpanWithError, and baggage propagation
🟠 Error handling removed — task status not updated	✅ Fixed	Error handling in callLlmStep and executeToolStep restored with task status updates, errorOp streaming, and span error recording
🟡 escapeSlackMrkdwn destroys Slack links	✅ Fixed	Commit a010a0e66 removed the redundant escapeSlackMrkdwn() call
🟠 Stream errors leave users without feedback	✅ Fixed	Commit a010a0e66 adds user-facing error message when fullText is empty after stream failure

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: agentExecution.ts:85-93 Use structured logger instead of console.info

💭 Consider (1) 💭

💭 1) block-actions.ts:510-512 Empty catch block for JSON parse errors

Issue: Invalid JSON lines in the SSE stream are silently ignored with just a // skip invalid JSON comment.

Why: While this may be intentional (skip malformed lines without disrupting the stream), there's no visibility into how often this happens or what data is being dropped.

Fix: Consider adding debug-level logging to preserve visibility:

catch (parseError) {
  logger.debug({ jsonStr, error: parseError }, 'Skipping invalid JSON in SSE stream');
}

Refs:

• api-logging-guidelines skill — debug level for expected failures

📝 Implementation Note

The inline comment suggestion for console.info → logger.info requires adding an import:

import { getLogger } from '../../../../logger';
const logger = getLogger('agentExecution');

---

💡 APPROVE WITH SUGGESTIONS

Summary: The latest commit successfully restores the OTEL tracing and error handling that was flagged in prior reviews — nice work! 🎉 The core feature implementation (durable execution mode for Slack, delegated approval propagation, continuation stream consumption) is architecturally sound. Only minor logging consistency issues remain. Ship it!

Discarded (0)

All prior major findings have been addressed. No new findings discarded in this delta review.

Reviewers (1)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
orchestrator (delta)	2	0	1	0	1	0	0
Total	2	0	1	0	1	0	0

Note: Delta review — no sub-reviewers dispatched. Focused on validating fixes for prior review findings and the new OTEL restoration commit.

[claude]

PR Review Summary

(6) Total Issues | Risk: High

⚠️ Force-push detected: The PR branch was force-pushed after previous reviews. Commits 6a7c806b6 and a010a0e66 (which contained fixes for OTEL tracing, error handling, and the escapeSlackMrkdwn bug) are no longer present. This review reflects the current state of the code.

🔴❗ Critical (0) ❗🔴

None.

🟠⚠️ Major (4) 🟠⚠️

🟠 1) agentExecutionSteps.ts OTEL tracing removed from durable workflow steps

files: agentExecutionSteps.ts

Issue: OpenTelemetry context propagation and span recording were removed from callLlmStep and executeToolStep. The file previously imported SPAN_NAMES, otelContext, propagation, setSpanWithError, and tracer, wrapped execution in otelContext.with(ctxWithBaggage, ...) with baggage for conversation.id, tenant.id, project.id, and agent.id. All of this is now gone.

Why: This eliminates observability for durable workflow execution paths:

• Span-level error recording via setSpanWithError is removed — failures won't appear in tracing dashboards with proper error status
• Baggage propagation is removed — trace correlation IDs won't flow through execution
• Debugging production incidents in durable mode becomes significantly harder

Fix: Restore OTEL instrumentation:

1. Re-add imports: SPAN_NAMES, otelContext, propagation, setSpanWithError, tracer
2. Wrap try block in callLlmStep with otelContext.with(ctxWithBaggage, ...) using baggage entries
3. Add tracer.startActiveSpan(SPAN_NAMES.EXECUTION_HANDLER_EXECUTE, ...) around completion/error handling
4. Call setSpanWithError(span, error) in catch blocks
5. Same pattern for executeToolStep with SPAN_NAMES.DURABLE_TOOL_EXECUTION

Refs:

• Original OTEL usage on main

---

🟠 2) agentExecutionSteps.ts Error handling removed — task status not updated to 'failed'

files: agentExecutionSteps.ts

Issue: The original callLlmStep and executeToolStep had catch blocks that: (1) updated task status to 'failed' with error metadata, (2) streamed errorOp to clients so they see the failure, (3) recorded errors on spans. The current implementation has no catch blocks — errors propagate directly to the workflow wrapper.

Why: Without error handling:

1. Task records remain in 'pending' or 'running' state when they should be 'failed'
2. Clients don't receive errorOp events — they may hang waiting for a response
3. The workflow-level catch only calls markWorkflowFailedStep, which doesn't update task records or notify clients

Fix: Add try-catch blocks that:

1. Import and call errorOp to stream error via sseHelper.writeOperation(errorOp(...))
2. Call updateTask to set status: 'failed' with error metadata
3. Re-throw the error so the workflow wrapper can also handle it

Refs:

• executionHandler.ts error handling pattern

---

Inline Comments:

• 🟠 Major: block-actions.ts:532 escapeSlackMrkdwn destroys Slack links
• 🟠 Major: block-actions.ts:531-546 No user feedback when stream fails

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: agentExecution.ts:85-93 Use structured logger instead of console.info

💭 Consider (1) 💭

Inline Comments:

• 💭 Consider: block-actions.ts:511-513 Empty catch block silently swallows JSON parse errors

---

🚫 REQUEST CHANGES

Summary: The core feature implementation (durable execution mode for Slack, delegated approval propagation through A2A artifacts, continuation stream consumption) is architecturally sound. However, the PR was force-pushed and lost critical fixes that were previously reviewed and approved:

1. OTEL tracing removal — eliminates observability for durable workflow paths
2. Error handling removal — tasks won't be marked as failed, clients won't receive error notifications
3. escapeSlackMrkdwn bug — destroys Slack links in approval continuation responses
4. Missing stream error feedback — users get no notification when continuation fails

Please restore the commits that contained the OTEL and error handling fixes (6a7c806b6), and apply the Slack bug fixes from a010a0e66.

Discarded (1)

Location	Issue	Reason Discarded
block-actions.ts:208-209	Missing Zod validation for InlineSelectorMetadata	Pre-existing issue, not introduced by this PR

Reviewers (5)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-standards	2	0	0	0	2	0	0
pr-review-errors	2	0	0	0	2	0	0
pr-review-sre	5	2	1	0	2	0	0
pr-review-consistency	0	0	0	0	0	0	0
pr-review-appsec	4	0	0	0	3	0	1
Total	13	2	1	0	4	0	1

Note: Several findings were deduplicated across reviewers (OTEL removal, escapeSlackMrkdwn bug, etc.)

[claude]

PR Review Summary

(6) Total Issues | Risk: High

🟠⚠️ Major (4) 🟠⚠️

🟠 1) agentExecutionSteps.ts OTEL tracing removed from durable workflow steps

files: agentExecutionSteps.ts

Issue: OpenTelemetry context propagation and span recording were removed from callLlmStep and executeToolStep. The file previously imported SPAN_NAMES, otelContext, propagation, setSpanWithError, and tracer, wrapped execution in otelContext.with(ctxWithBaggage, ...) with baggage for conversation.id, tenant.id, project.id, and agent.id. All of this instrumentation is now gone.

Why: This eliminates observability for durable workflow execution paths:

• Span-level error recording via setSpanWithError is removed — failures won't appear in tracing dashboards with proper error status
• Baggage propagation is removed — trace correlation IDs won't flow through execution
• Debugging production incidents in durable mode becomes significantly harder

Fix: Restore OTEL instrumentation:

1. Re-add imports: SPAN_NAMES, otelContext, propagation, setSpanWithError, tracer
2. Wrap try block in callLlmStep with otelContext.with(ctxWithBaggage, ...) using baggage entries
3. Add tracer.startActiveSpan(SPAN_NAMES.EXECUTION_HANDLER_EXECUTE, ...) around completion/error handling
4. Call setSpanWithError(span, error) in catch blocks
5. Same pattern for executeToolStep with SPAN_NAMES.DURABLE_TOOL_EXECUTION

Refs:

• Original OTEL usage on main

---

🟠 2) agentExecutionSteps.ts Error handling removed — task status not updated to 'failed'

files: agentExecutionSteps.ts

Issue: The callLlmStep function (lines 507-743) has a try-finally block with NO catch block. The original code had explicit catch blocks that: (1) updated task status to 'failed' with error metadata, (2) streamed errorOp to clients so they see the failure, (3) recorded errors on spans. Errors now propagate directly to the workflow wrapper without any of this handling.

Why: Without error handling:

1. Task records remain in 'pending' or 'running' state when they should be 'failed'
2. Clients don't receive errorOp events — they may hang waiting for a response
3. The workflow-level catch only calls markWorkflowFailedStep, which doesn't update task records or notify clients

Fix: Add try-catch blocks that:

1. Import and call errorOp to stream error via sseHelper.writeOperation(errorOp(...))
2. Call updateTask to set status: 'failed' with error metadata
3. Re-throw the error so the workflow wrapper can also handle it

Refs:

• executionHandler.ts error handling pattern

---

Inline Comments:

• 🟠 Major: block-actions.ts:532 escapeSlackMrkdwn destroys Slack links
• 🟠 Major: block-actions.ts:531-546 No user feedback when stream fails

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: agentExecution.ts:87-95 Use structured logger instead of console.info

💭 Consider (1) 💭

Inline Comments:

• 💭 Consider: block-actions.ts:506-513 Empty catch block silently swallows JSON parse errors

🕐 Pending Recommendations (5)

Prior review findings that remain unaddressed:

• 🟠 agentExecutionSteps.ts OTEL tracing removed from durable workflow steps (from earlier Claude review)
• 🟠 agentExecutionSteps.ts Error handling removed — task status not updated to 'failed' (from earlier Claude review)
• 🟡 block-actions.ts:532 escapeSlackMrkdwn bug (from Pullfrog review)
• 🟡 agentExecution.ts:93 Use structured logger instead of console.info (from Pullfrog review)
• 🟡 agentExecution.ts:51 Consider aligning isPostApproval and approvalRound state (from Pullfrog review)

---

🚫 REQUEST CHANGES

Summary: The core feature implementation (durable execution mode for Slack, delegated approval propagation through A2A artifacts, continuation stream consumption) is architecturally sound. However, there are significant regressions that need to be addressed before merge:

1. OTEL tracing removal — eliminates observability for durable workflow paths. This appears to have been lost during a rebase or refactor. The prior reviews noted commits existed that fixed this but were force-pushed away.

2. Error handling removal — tasks won't be marked as failed, clients won't receive error notifications. The workflow-level catch only updates workflow_executions status, not individual task status.

3. Slack link rendering bug — escapeSlackMrkdwn(markdownToMrkdwn(...)) double-escapes angle brackets, breaking links.

4. Missing stream error feedback — users get no notification when continuation stream fails.

Please restore the OTEL tracing and error handling from the original implementation, and apply the Slack bug fixes.

Discarded (3)

Location	Issue	Reason Discarded
generateTaskHandler.ts:56-89	buildDurableApprovalResult extraction	Valid DRY refactoring — used in 2 distinct locations, consolidates code
tool-wrapper.ts:196-220	Helper functions defined inside hot path	Minor optimization concern — JavaScript engines optimize this well
agentExecution.ts:49-51	isPostApproval and approvalRound track same concept differently	Developer preference — both work correctly

Reviewers (4)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-standards	3	2	0	0	1	0	0
pr-review-errors	4	0	0	0	2	0	2
pr-review-sre	6	2	0	0	2	0	2
pr-review-consistency	2	0	0	0	2	0	0
Total	15	4	0	0	4	0	4

Note: Several findings were deduplicated across reviewers (OTEL removal, escapeSlackMrkdwn bug flagged by multiple reviewers).

[claude]

PR Review Summary

(0) New Issues | Risk: Low (delta-only review)

This is a delta review covering commit 1c50dc17a — "Address review: remove escapeSlackMrkdwn that destroys links, add error feedback for stream failures".

✅ Prior Issues Addressed in This Commit

The commit successfully addresses 3 prior review findings:

Prior Issue	Fix Applied
🟡 block-actions.ts:540 escapeSlackMrkdwn destroys Slack links (Pullfrog, Claude)	✅ Removed escapeSlackMrkdwn() call — now uses markdownToMrkdwn() only
🟠 block-actions.ts:531-546 Stream errors leave users without feedback	✅ Added error message posted to Slack when fullText is empty after stream failure (lines 520-528)
🟡 Missing context in error log	✅ Added channel and threadTs to error log context (line 517)

🕐 Pending Recommendations (4)

Prior review findings that remain unaddressed (outside the scope of this delta commit):

• 🟠 agentExecutionSteps.ts OTEL tracing removed from durable workflow steps — span recording, baggage propagation, and setSpanWithError calls removed
• 🟠 agentExecutionSteps.ts Error handling removed — task status not updated to 'failed', clients don't receive errorOp events
• 🟡 agentExecution.ts:87-95 Use structured logger instead of console.info(JSON.stringify({...}))
• 💭 block-actions.ts:510-512 Empty catch block silently swallows JSON parse errors (consider adding debug logging)

---

🚫 REQUEST CHANGES

Summary: The delta commit correctly fixes 3 prior review issues — nice work! 🎉 The escapeSlackMrkdwn bug is resolved and users now get feedback when stream consumption fails. The error logging context improvement is a nice touch.

However, the Major-severity items from prior reviews remain unaddressed:

1. OTEL tracing removal in agentExecutionSteps.ts — this eliminates observability for durable workflow execution paths
2. Error handling removal in agentExecutionSteps.ts — tasks won't be marked as 'failed' and clients won't receive error notifications

These observability/reliability concerns should be resolved before merge.

Discarded (0)

No new findings discarded — this delta commit introduced no issues.

Reviewers (1)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
orchestrator (delta)	0	0	0	0	0	4	0
Total	0	0	0	0	0	4	0

Note: Delta review — no sub-reviewers dispatched. Focused on validating fixes for prior review findings in the 1 file changed since last review.

[claude]

PR Review Summary

(5) Total Issues | Risk: High

🟠⚠️ Major (2) 🟠⚠️

🟠 1) agentExecutionSteps.ts OTEL tracing removed from durable workflow steps

files: agentExecutionSteps.ts

Issue: OpenTelemetry context propagation and span recording were completely removed from callLlmStep and executeToolStep. The main branch imports SPAN_NAMES, otelContext, propagation, setSpanWithError, and tracer, wraps execution in otelContext.with(ctxWithBaggage, ...) with baggage for conversation.id, tenant.id, project.id, and agent.id, and records errors via setSpanWithError(span, rootCause). All of this instrumentation is now gone.

Why: This eliminates observability for durable workflow execution paths:

• Span-level error recording via setSpanWithError is removed — failures won't appear in tracing dashboards with proper error status
• Baggage propagation is removed — trace correlation IDs (conversation.id, tenant.id, etc.) won't flow through execution
• The spans for SPAN_NAMES.EXECUTION_HANDLER_EXECUTE and SPAN_NAMES.DURABLE_TOOL_EXECUTION are no longer created
• Debugging production incidents in durable mode becomes significantly harder

Fix: Restore OTEL instrumentation:

1. Re-add imports: SPAN_NAMES, otelContext, propagation, setSpanWithError, tracer
2. Wrap try block in callLlmStep with otelContext.with(ctxWithBaggage, ...) using baggage entries
3. Add tracer.startActiveSpan(SPAN_NAMES.EXECUTION_HANDLER_EXECUTE, ...) around completion/error handling
4. Call setSpanWithError(span, error) in catch blocks
5. Same pattern for executeToolStep with SPAN_NAMES.DURABLE_TOOL_EXECUTION

Refs:

• Main branch OTEL imports

---

🟠 2) agentExecutionSteps.ts Error handling removed — task status not updated to 'failed'

files: agentExecutionSteps.ts

Issue: The callLlmStep function (lines 507-755) has a try-finally block with NO catch block. The main branch has explicit catch blocks that: (1) update task status to 'failed' with error metadata via updateTask(), (2) stream errorOp to clients so they see the failure, (3) record errors on OTEL spans. Errors now propagate directly to the workflow wrapper without any of this handling.

Why: Without error handling:

1. Task records remain in 'pending' or 'running' state when they should be 'failed'
2. Clients don't receive errorOp events — they may hang waiting for a response that never comes
3. The workflow-level catch only calls markWorkflowFailedStep, which updates workflow_executions status but doesn't update individual task status or notify clients

Fix: Add catch blocks that:

1. Import and call errorOp to stream error via sseHelper.writeOperation(errorOp(...))
2. Call updateTask to set status: 'failed' with error metadata
3. Re-throw the error so the workflow wrapper can also handle it

Refs:

• Main branch error handling pattern

Inline Comments:

• 🟠 Major: block-actions.ts:539-554 Empty success case leaves users with no feedback

🟡 Minor (2) 🟡

🟡 1) agent-types.ts + agentExecutionSteps.ts + Agent.ts Type duplication across 3 locations

files: agent-types.ts, agentExecutionSteps.ts, Agent.ts

Issue: The DelegatedApprovalContext type shape ({ toolCallId, toolName, args, subAgentId }) is defined in three places:

1. agentExecutionSteps.ts lines 34-39 as DelegatedApprovalContext
2. agent-types.ts lines 277-283 inline within AgentRunContext.pendingDurableApproval.delegatedApproval
3. Agent.ts lines 166-171 inline in getPendingDurableApproval() return type

Why: Having three parallel definitions of the same shape creates maintenance burden and risks drift. The codebase convention for shared types is to define them once in agent-types.ts.

Fix: Define DelegatedApprovalContext once in agent-types.ts and export it, then import and reference it in the other locations.

Refs:

• agent-types.ts:277-283

Inline Comments:

• 🟡 Minor: agentExecution.ts:87-95 Use structured logger instead of console.info

💭 Consider (1) 💭

💭 1) block-actions.ts:510-512 Empty catch block for JSON parsing follows existing convention

Issue: The JSON parsing in consumeApprovalContinuationStream has an empty catch block with only a // skip invalid JSON comment.

Why: This actually follows the established convention in streaming.ts (line 554), which uses the same pattern. However, adding debug-level logging would improve visibility into parsing issues without breaking the pattern.

Fix: Optionally add debug logging: catch (parseError) { logger.debug({ jsonStr: jsonStr.slice(0, 100) }, 'Skipping invalid JSON in SSE stream'); }

Refs:

• streaming.ts:554 — same pattern

🕐 Pending Recommendations (5)

Prior review findings that remain unaddressed:

• 🟠 agentExecutionSteps.ts OTEL tracing removed from durable workflow steps (from earlier Claude review)
• 🟠 agentExecutionSteps.ts Error handling removed — task status not updated to 'failed' (from earlier Claude review)
• 🟡 agentExecution.ts:93 Use structured logger instead of console.info (from Pullfrog review)
• 🟡 agentExecution.ts:51 Consider aligning isPostApproval and approvalRound state (from Pullfrog review)
• 💭 block-actions.ts:510-512 Empty catch block silently swallows JSON parse errors (from earlier Claude review)

---

🚫 REQUEST CHANGES

Summary: The core feature implementation is architecturally sound — forcing durable execution mode for Slack, propagating delegated approvals through A2A artifacts, and consuming continuation streams are well-designed. The latest commit (c637f868) correctly fixed the escapeSlackMrkdwn bug and added error feedback for stream failures.

However, two Major-severity regressions remain unaddressed:

1. OTEL tracing removal — eliminates observability for durable workflow paths, making production debugging significantly harder
2. Error handling removal — tasks won't be marked as 'failed' and clients won't receive error notifications

These appear to have been lost during a rebase or refactor rather than intentionally removed. Please restore the OTEL instrumentation and error handling from the main branch.

Discarded (3)

Location	Issue	Reason Discarded
generateTaskHandler.ts:56-89	buildDurableApprovalResult extraction	Valid DRY refactoring — consolidates duplicate artifact construction code
tool-wrapper.ts:196-220	Helper functions defined inside hot path	Follows JavaScript patterns, functions need no external state, negligible perf impact
streaming.ts:198	executionMode: 'durable' addition	Core feature — intentional forcing of durable mode for Slack

Reviewers (4)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
pr-review-standards	0	0	0	0	0	0	0
pr-review-errors	1	0	0	0	1	0	0
pr-review-sre	0	0	0	0	0	0	0
pr-review-consistency	4	1	1	0	1	0	1
orchestrator	2	2	0	0	0	5	2
Total	7	3	1	0	2	5	3

Note: Most findings were deduplicated against prior reviews to avoid repetition. The OTEL and error handling issues were already raised in previous reviews and are tracked as Pending Recommendations.

[claude]

🟡 Minor: Use structured logger instead of console.info

Issue: Using console.info(JSON.stringify({...})) bypasses the structured logging infrastructure used consistently throughout the workflow code.

Why: The sibling file agentExecutionSteps.ts uses getLogger('agentExecutionSteps') for consistent log formatting, redaction support, and correlation IDs. Using console.info creates inconsistent log output that won't appear in log aggregation with proper metadata.

Fix: Add import and logger at module scope, then use structured logging:

import { getLogger } from '../../../../logger';
const logger = getLogger('agentExecution');

// Then replace console.info with:
logger.info(
  {
    hookToolCallId,
    parentToolCallId: toolCall.toolCallId,
    isDelegated: !!llmResult.delegatedApproval,
    workflowRunId,
  },
  '[agentExecution] Creating tool approval hook'
);

Refs:

• agentExecutionSteps.ts:6 — existing logger pattern

[claude]

🟠 MAJOR: No user feedback when stream completes with empty text

Issue: When consumeApprovalContinuationStream completes successfully but fullText is empty (e.g., stream emits only non-text-delta events, or uses an unexpected format), the user receives no feedback. The 'thinking...' message is deleted but no follow-up is posted.

Why: After approving a tool, users see "agentName is thinking..." appear then disappear with no explanation. Unlike the error case (lines 520-528) which posts "Something went wrong...", the empty-success case leaves users unable to determine if the operation completed successfully.

Fix: Add an else branch to inform users when no text was produced:

Suggested change

	if (fullText.length > 0) {
	const slackText = markdownToMrkdwn(fullText);
	await slackClient.chat
	.postMessage({
	channel,
	...threadParam,
	text: slackText,
	blocks: [
	{ type: 'section', text: { type: 'mrkdwn', text: slackText } },
	createContextBlock({ agentName }),
	],
	})
	.catch((e) =>
	logger.warn({ error: e, channel, threadTs }, 'Failed to post approval continuation result')
	);
	}
	if (fullText.length > 0) {
	const slackText = markdownToMrkdwn(fullText);
	await slackClient.chat
	.postMessage({
	channel,
	...threadParam,
	text: slackText,
	blocks: [
	{ type: 'section', text: { type: 'mrkdwn', text: slackText } },
	createContextBlock({ agentName }),
	],
	})
	.catch((e) =>
	logger.warn({ error: e, channel, threadTs }, 'Failed to post approval continuation result')
	);
	} else {
	logger.warn({ channel, threadTs, agentName }, 'Approval continuation stream produced empty response');
	await slackClient.chat
	.postMessage({
	channel,
	...threadParam,
	text: `_${agentName} completed the tool execution but produced no visible response._`,
	})
	.catch((e) =>
	logger.warn({ error: e, channel, threadTs }, 'Failed to post empty response notification')
	);
	}
	}

Refs:

• pr-review-errors guidance — error feedback should inform users

[claude]

PR Review Summary

(1) Total Issues | Risk: Low

This is a delta review covering commit ecd719cc4 — "Restore OTEL tracing and error handling in agentExecutionSteps".

✅ Prior Major Issues Addressed

Great work addressing the critical concerns from previous reviews! 🎉

Prior Issue	Status	Details
🟠 OTEL tracing removed from durable workflow steps	✅ Fixed	Commit restores otelContext.with(), tracer.startActiveSpan, setSpanWithError, and baggage propagation for conversation.id, tenant.id, project.id, agent.id
🟠 Error handling removed — task status not updated	✅ Fixed	Catch blocks restored in both callLlmStep and executeToolStep with task status updates to 'failed', errorOp streaming to clients, and span error recording
🟡 escapeSlackMrkdwn destroys Slack links	✅ Fixed	Commit c637f868 removed the redundant escapeSlackMrkdwn() call
🟠 Stream errors leave users without feedback	✅ Fixed	Error message posted to Slack when fullText is empty after stream failure

🟡 Minor (1) 🟡

Inline Comments:

• 🟡 Minor: agentExecution.ts:87-95 Use structured logger instead of console.info

💭 Consider (1) 💭

💭 1) block-actions.ts:510-512 Empty catch block for JSON parse errors

Issue: Invalid JSON lines in the SSE stream are silently ignored with just a // skip invalid JSON comment.

Why: While skipping malformed JSON is often correct for SSE streams (and follows the pattern in streaming.ts), there's no visibility into how often this happens. During incidents, this makes it harder to diagnose parsing failures.

Fix: Consider adding debug-level logging:

catch (parseError) {
  logger.debug({ jsonStr: jsonStr.slice(0, 100) }, 'Skipping invalid JSON in SSE stream');
}

Refs:

• api-logging-guidelines skill — debug level for expected failures

---

💡 APPROVE WITH SUGGESTIONS

Summary: The latest commit successfully restores the OTEL tracing and error handling that was the main blocker from prior reviews — excellent work! 🎉 The core feature implementation (durable execution mode for Slack, delegated approval propagation through A2A artifacts, continuation stream consumption) is architecturally sound. Only one minor logging consistency issue remains. Ship it!

Discarded (0)

All prior major findings have been addressed. No new findings discarded in this delta review.

Reviewers (1)

Reviewer	Returned	Main Findings	Consider	While You're Here	Inline Comments	Pending Recs	Discarded
orchestrator (delta)	2	0	1	0	1	0	0
Total	2	0	1	0	1	0	0

Note: Delta review — no sub-reviewers dispatched. Focused on validating fixes for prior review findings and the OTEL restoration commit.

[itoqa]

Ito Test Report ❌

12 test cases ran. 1 failed, 2 additional findings, 9 passed.

Overall, 12 test cases were executed with 9 passing and 3 failing, confirming that Slack approval continuation flows, delegated approval context propagation, key edge cases (non-SSE gating, malformed SSE tolerance, timeout cleanup, and duplicate-approve idempotency), and core security checks (ownership enforcement, invalid signature rejection, and stale-timestamp replay rejection) behaved as expected. The most important findings were three real defects: a high-severity approval payload integrity gap that allows tampered identifiers to retarget approvals across contexts, a medium-severity mrkdwn sanitization issue that permits mention/formatting injection in continuation messages, and a high-severity approve/deny race condition that can show contradictory terminal Slack outcomes for the same approval.

❌ Failed (1)

Category	Summary	Screenshot
Adversarial	🟠 Continuation output is posted as mrkdwn after markdown conversion without full mrkdwn escaping, allowing mention/link injection-style content.

🟠 Malicious markdown payload is sanitized in continuation Slack message

• What failed: Continuation text is converted with markdownToMrkdwn and posted as mrkdwn, but no full mrkdwn escaping is applied before posting dynamic model output.
• Impact: Malicious or untrusted continuation output can trigger unintended mentions or formatting injection in Slack threads. This can create noisy or misleading notifications for channel members.
• Steps to reproduce:
  1. Approve a tool call that produces continuation output containing mrkdwn-sensitive tokens such as @channel and crafted link-like text.
  2. Allow the approval continuation stream to complete.
  3. Inspect the posted Slack continuation message and verify the dynamic output is rendered as active mrkdwn rather than fully escaped text.
• Stub / mock context: Outgoing Slack Web API responses were simulated for this scenario to keep continuation verification deterministic in local execution, so no real workspace posts were required.
• Code analysis: I inspected continuation stream consumption and markdown conversion utilities. The continuation path posts model text directly after markdown transformation, and the conversion helper does formatting transforms but does not perform comprehensive escaping for all mrkdwn-sensitive constructs in arbitrary untrusted output.
• Why this is likely a bug: Untrusted continuation text is emitted as Slack mrkdwn without robust escaping, enabling active mention/formatting side effects instead of inert display.

Relevant code:

packages/agents-work-apps/src/slack/services/events/block-actions.ts (lines 539-548)

if (fullText.length > 0) {
  const slackText = markdownToMrkdwn(fullText);
  await slackClient.chat.postMessage({
    channel,
    ...threadParam,
    text: slackText,
    blocks: [
      { type: 'section', text: { type: 'mrkdwn', text: slackText } },
      createContextBlock({ agentName }),
    ],
  });
}

packages/agents-work-apps/src/slack/services/events/utils.ts (lines 120-148)

export function markdownToMrkdwn(markdown: string): string {
  if (!markdown) return markdown;

  let result = markdown;
  result = result.replace(/^#{1,6}\s+(.+)$/gm, '*$1*');
  result = result.replace(
    /\[([^\]]+)\]\(([^)]+)\)/g,
    (_, text, url) => `<${url}|${escapeSlackLinkText(text)}>`
  );
  result = result.replace(/\*\*([^*]+)\*\*/g, '*$1*');
  result = result.replace(/__([^_]+)__/g, '*$1*');
  result = result.replace(/~~([^~]+)~~/g, '~$1~');
  return result;
}

✅ Passed (9)

Category	Summary	Screenshot
Adversarial	Ownership mismatch request was acknowledged but did not process tool approval, enforcing originator-only approval behavior.
Adversarial	Invalid Slack signature was rejected with HTTP 401, confirming signature verification.
Adversarial	Valid HMAC with stale timestamp was rejected with HTTP 401, confirming replay-window enforcement.
Edge	Confirmed not a real bug. Source gate requires text/event-stream before continuation parsing, so non-SSE approved responses do not invoke parser.
Edge	Confirmed not a real bug. Continuation parser ignores malformed JSON frames and preserves valid text-delta chunks.
Edge	Confirmed not a real bug. Timeout path cancels stream reads, cleans the temporary thinking message, and only posts fallback when no text was accumulated.
Edge	Confirmed not a real bug. Duplicate approvals are handled idempotently in durable and classic approval handling paths.
Logic	Delegated approval child tool identifiers are propagated into durable approval context and Slack approval payload construction as intended.
Happy-path	Re-run and code review confirmed approved actions consume continuation SSE and post continuation output.

ℹ️ Additional Findings (2)

These findings are unrelated to the current changes but were observed during testing.

Category	Summary	Screenshot
Adversarial	⚠️ Slack tool-approval handler trusts mutable button payload identifiers without integrity binding, enabling tampered approval targeting.
Adversarial	⚠️ Approve/deny race can produce conflicting terminal Slack outcomes for the same approval.

⚠️ Tampered action payload cannot approve cross-project or wrong conversation

• What failed: The server parses and trusts identifiers from the mutable button payload as authority for approval targeting, instead of validating them with a server-bound integrity check.
• Impact: A validly signed but tampered approval payload can target unintended approval contexts. This creates a cross-conversation/project authorization risk in approval workflows.
• Steps to reproduce:
  1. Trigger a Slack tool-approval card for a pending durable approval.
  2. Modify identifiers in the action value (for example projectId, agentId, conversationId, or toolCallId) before submitting the signed request.
  3. Send the tampered payload with a valid Slack signature and observe that server-side approval targeting depends on those mutable fields.
• Stub / mock context: This run used local Slack workspace fixtures and seeded test user mappings to exercise approval ownership checks without a live Slack workspace. Outbound Slack API behavior was simulated for deterministic local execution.
• Code analysis: I inspected Slack action parsing and approval resume flow. The handler consumes approval identifiers directly from action.value, then forwards them into /run/api/chat; the durable resume token is built from those supplied identifiers, with no cryptographic integrity binding between the posted button payload and original server-issued approval context.
• Why this is likely a bug: The approval target is chosen from client-mutable payload fields without an integrity-bound server-issued proof, so tampering can alter authorization scope.

Relevant code:

packages/agents-work-apps/src/slack/services/events/block-actions.ts (lines 49-113)

const buttonValue = ToolApprovalButtonValueSchema.parse(JSON.parse(actionValue));
const { toolCallId, conversationId, projectId, agentId, toolName } = buttonValue;

const approvalResponse = await getInProcessFetch()(`${apiUrl}/run/api/chat`, {
  method: 'POST',
  headers: {
    'Content-Type': 'application/json',
    Authorization: `Bearer ${slackUserToken}`,
    'x-inkeep-project-id': projectId,
    'x-inkeep-agent-id': agentId,
    'x-inkeep-invocation-type': 'slack',
    'x-inkeep-invocation-entry-point': 'tool_approval',
  },
  body: JSON.stringify({ conversationId, messages: [/* ...approval payload... */] }),
});

agents-api/src/domains/run/routes/chatDataStream.ts (lines 162-170)

if (isDurable && durableExecution) {
  await Promise.allSettled(
    approvalParts.map(async (approvalPart: any) => {
      const toolCallId = approvalPart.toolCallId as string;
      const approved = !!approvalPart.approval?.approved;
      const reason = approvalPart.approval?.reason as string | undefined;
      const token = `tool-approval:${conversationId}:${durableExecution.id}:${toolCallId}`;
      await toolApprovalHook.resume(token, { approved, reason: approved ? undefined : reason });
    })
  );
}

⚠️ Approve and deny race from duplicated tabs yields single consistent outcome

• What failed: The system can acknowledge both requests and emit conflicting terminal Slack states (✅ Approved and ❌ Denied) even though only one workflow approval decision should be authoritative.
• Impact: Reviewers can see contradictory terminal outcomes for one approval, reducing trust in Slack-side state. This can cause incorrect operational decisions based on the last visible Slack update rather than actual workflow state.
• Steps to reproduce:
  1. Create a suspended durable workflow run with a pending tool approval in a Slack thread.
  2. Open duplicated Slack contexts for that same approval and submit Approve and Deny nearly simultaneously for the same toolCallId.
  3. Observe Slack terminal updates and verify that conflicting Approved and Denied outcomes can both be shown for the same approval request.
• Stub / mock context: The scenario was exercised with local direct API calls using a test bypass token instead of real Slack button clicks. A synthetic conversation and tool call context was used to force near-simultaneous approve/deny requests against the same approval target.
• Code analysis: I inspected the Slack block-actions approval path and the durable approval-resume route. The Slack handler posts terminal approval/denial UI as soon as /run/api/chat returns OK, while the durable resume endpoint intentionally swallows duplicate/conflicting resume errors (not found/already) and still returns a success path, enabling inconsistent downstream Slack terminal messaging under races.
• Why this is likely a bug: The production code allows duplicate/conflicting approval resumes to appear successful while Slack UI completion messaging is emitted per request, which directly permits contradictory terminal states for one approval.

Relevant code:

packages/agents-work-apps/src/slack/services/events/block-actions.ts (lines 147-153)

if (responseUrl) {
  await sendResponseUrlMessage(responseUrl, {
    text: approved ? `✅ Approved \`${toolName}\`` : `❌ Denied \`${toolName}\``,
    replace_original: true,
    blocks: buildToolApprovalDoneBlocks({ toolName, approved, actorUserId: slackUserId }),
  }).catch((e) => logger.warn({ error: e }, 'Failed to update approval message'));
}

agents-api/src/domains/run/routes/chatDataStream.ts (lines 168-179)

const token = `tool-approval:${conversationId}:${durableExecution.id}:${toolCallId}`;
try {
  await toolApprovalHook.resume(token, {
    approved,
    reason: approved ? undefined : reason,
  });
} catch (error) {
  const message = error instanceof Error ? error.message : String(error);
  if (!message.includes('not found') && !message.includes('already')) {
    throw error;
  }
}

agents-api/src/domains/run/routes/chatDataStream.ts (lines 188-199)

c.header('Content-Type', 'text/event-stream');
c.header('Cache-Control', 'no-cache');
c.header('Connection', 'keep-alive');

return stream(c, async (s) => {
  try {
    const readable = run.getReadable({ namespace });
    const reader = readable.getReader();
    while (true) {
      const { done, value } = await reader.read();
      if (done) break;

Commit: c611c59

View Full Run

---

Tell us how we did: Give Ito Feedback