Conversation
🦋 Changeset detectedLatest commit: 1b5a05a The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…rameters for toolName and input. Update validation schema and implement tests for new functionality, including rendering input fields and handling edge cases.
…pport-tool-approvals # Conflicts: # packages/agents-work-apps/src/slack/routes/events.ts # packages/agents-work-apps/src/slack/services/blocks/index.ts
…pport-tool-approvals
…ng only the initiating user can approve or deny actions. Added error notification for unauthorized attempts.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…xpected exceptions and malformed input. Implement tests for new error scenarios and ensure proper logging of failures.
…for input. Update tests to reflect changes in input handling, including truncation logic and section structure. Ensure proper rendering of input fields and maintain character limits.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…approval messages when they expire. Introduce new block rendering for expired approvals and update tests to verify the correct behavior of message updates and error handling during streaming. Ensure proper logging for failures in message posting and updating.
…er notifications. Update tests to verify the correct behavior of ephemeral messages in various scenarios, ensuring ownership checks and linking requirements are properly enforced.
…treaming. This change ensures that the timeout is reset, allowing for longer wait times without prematurely closing the stream.
amikofalvy
left a comment
There was a problem hiding this comment.
Needs a changeset entry then you can merge
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review following 17 commits since the last automated APPROVE review (35fe797ccaba).
Delta Analysis ✅
The commits since last review include:
- Changeset added (
069936373) — addresses amikofalvy's merge requirement ✅ - postEphemeral refactor (
7c9bb4b42) — useschat.postEphemeralinstead ofsendResponseUrlMessagefor error notifications - Stream timeout clearing (
b685fd40e) — clears timeout when waiting for human approval - Main merges — Version Packages, channel-based agent authorization, docs updates (infrastructure only)
Implementation Quality ✅
| Change | Assessment |
|---|---|
postEphemeral for errors |
✅ Correct Slack API usage — ephemeral messages work without responseUrl and provide better UX |
clearTimeout on approval |
✅ Safe design — delegates timeout to backend's APPROVAL_TIMEOUT_MS; comment explains the delegation |
replace_original: false |
✅ Critical bug fix — prevents Slack's default behavior from hiding approval buttons when sending ephemeral rejections |
extractApiErrorMessage |
✅ Good UX — surfaces specific backend errors instead of generic messages; safe fallback with try-catch |
Test Coverage ✅
Delta includes comprehensive test updates:
- Ownership checks verified with
mockPostEphemeral - Link requirement checks verified
- API error surfacing tested (lines 329-351 in streaming.test.ts)
- Ephemeral fallback when
responseUrlabsent tested
Prior Feedback Status
All 8 prior issues from earlier reviews have been addressed and verified fixed (ownership check, Zod validation, getInProcessFetch(), error handling, etc.).
✅ APPROVE
Summary: The delta since last APPROVE contains the requested changeset and quality refinements (postEphemeral, timeout clearing, error message extraction, replace_original fix). All changes are correct, well-tested, and improve the feature's reliability and UX. The Slack tool approval feature is ready to ship! 🎉
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| (delta contained refinements only — direct review) | — | — | — | — | — | — | — |
* docs: add tool approvals in Slack section to tool-approvals.mdx * docs: add tool approvals section to Slack commands page (PR #2149) --------- Co-authored-by: inkeep[bot] <257615677+inkeep[bot]@users.noreply.github.com>
No description provided.