fix(linear-slack): deliver Slack replies + guard the writeback-scope footgun#53
Conversation
…footgun
linear-slack posted replies into the void. Scope-less `slack: {}` meant the
only Slack mount was the trigger's READ mirror of the display-labelled channel
(/slack/channels/{id}__{name}/...), but slackClient().post() writes to the
canonical bare-id writeback path (/slack/channels/{id}/messages). The draft
landed on unmounted disk and was never flushed — confirmed by recovering the
orphaned draft from the live sandbox. slackClient swallows the missing-receipt
timeout (returns ts:'' without throwing), so the runtime logged handler.ok with
nothing in the channel.
- persona.ts: scope slack to /slack/channels/** so the writeback path mounts
(same fix already shipped in review/vendor-monitor/hn-monitor/repo-hygiene).
- agent.ts: postReply treats an empty receipt ts as a delivery failure and
throws, so a dropped reply surfaces as handler.error instead of a silent
no-op.
- tests: tighten the integration-scope guard — a Slack WRITE now requires a
scope even when a trigger exists. The old trigger-or-scope rule
false-negatived this exact case; github/linear are unaffected (their trigger
and writeback paths share one bare-id form). Proven red→green.
Refs AgentWorkforce/cloud#2029 (framework-level prevention).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces the linear-slack agent persona that processes Slack messages in a configured channel, reconstructs conversation history from memory, invokes Claude with mounted Linear VFS access, and persists bidirectional message exchanges. Tightens test infrastructure to enforce scope requirements for providers with display-labeled triggers that cannot cover bare-id writeback paths. ChangesLinear-Slack Agent Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Slack-native conversational Linear board assistant (linear-slack) that navigates a mounted Linear VFS inside a sandbox to read and update issues. It also updates integration scope tests to enforce that display-labelled providers like Slack require an explicit scope for write operations. Key feedback includes: refining Slack message subtype filtering to avoid ignoring valid human messages (e.g., file shares); rethrowing harness errors so failures are not hidden from monitoring; ensuring replies are always threaded to preserve multi-turn conversation history; and correcting the invalid model identifier claude-sonnet-4-6 to a valid one like claude-3-5-sonnet.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Only fresh human messages — skip edits/deletes/joins/etc. | ||
| if (msg.subtype) { | ||
| logSkip(ctx, event, `slack subtype ${msg.subtype}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Skipping all messages with any subtype will cause the agent to silently ignore valid human messages that have subtypes, such as file_share (when a user uploads an image/file with a comment) or thread_broadcast (when a user cross-posts a thread reply to the channel). Instead of skipping all subtypes, consider skipping only specific system or edit subtypes (e.g., message_changed, message_deleted, channel_join, channel_leave, tombstone).
| // Only fresh human messages — skip edits/deletes/joins/etc. | |
| if (msg.subtype) { | |
| logSkip(ctx, event, `slack subtype ${msg.subtype}`); | |
| return; | |
| } | |
| // Only fresh human messages — skip edits/deletes/joins/etc. | |
| const ignoredSubtypes = new Set(['message_changed', 'message_deleted', 'channel_join', 'channel_leave', 'tombstone']); | |
| if (msg.subtype && ignoredSubtypes.has(msg.subtype)) { | |
| logSkip(ctx, event, 'slack subtype ' + msg.subtype); | |
| return; | |
| } |
| } catch (err) { | ||
| ctx.log?.('warn', 'linear-slack.harness.failed', { error: errorMessage(err) }); | ||
| reply = isTransientLlmError(err) | ||
| ? "I'm getting rate-limited by the model right now — give me a moment and ask again." | ||
| : 'Sorry, I hit an unexpected error working on that. Please try again.'; | ||
| // Best-effort apology — we're already in the error path, so don't let an | ||
| // undelivered apology mask the original failure; just log it loudly. | ||
| try { | ||
| await postReply(slack, msg, reply); | ||
| } catch (postErr) { | ||
| ctx.log?.('error', 'linear-slack.reply.undelivered', { error: errorMessage(postErr) }); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
When the harness fails, the agent catches the error, sends a best-effort apology, and returns successfully. This causes the platform to log handler.ok instead of handler.error, hiding harness failures from automated monitoring and alerting. Rethrow the original error after attempting to send the apology so that failures are loud and visible.
| } catch (err) { | |
| ctx.log?.('warn', 'linear-slack.harness.failed', { error: errorMessage(err) }); | |
| reply = isTransientLlmError(err) | |
| ? "I'm getting rate-limited by the model right now — give me a moment and ask again." | |
| : 'Sorry, I hit an unexpected error working on that. Please try again.'; | |
| // Best-effort apology — we're already in the error path, so don't let an | |
| // undelivered apology mask the original failure; just log it loudly. | |
| try { | |
| await postReply(slack, msg, reply); | |
| } catch (postErr) { | |
| ctx.log?.('error', 'linear-slack.reply.undelivered', { error: errorMessage(postErr) }); | |
| } | |
| return; | |
| } | |
| } catch (err) { | |
| ctx.log?.('warn', 'linear-slack.harness.failed', { error: errorMessage(err) }); | |
| reply = isTransientLlmError(err) | |
| ? "I'm getting rate-limited by the model right now — give me a moment and ask again." | |
| : 'Sorry, I hit an unexpected error working on that. Please try again.'; | |
| // Best-effort apology — we're already in the error path, so don't let an | |
| // undelivered apology mask the original failure; just log it loudly. | |
| try { | |
| await postReply(slack, msg, reply); | |
| } catch (postErr) { | |
| ctx.log?.('error', 'linear-slack.reply.undelivered', { error: errorMessage(postErr) }); | |
| } | |
| throw err; | |
| } |
| const result = msg.threadTs | ||
| ? await slack.reply(msg.channel, msg.threadTs, text) | ||
| : await slack.post(msg.channel, text); | ||
| if (!result?.ts) { |
There was a problem hiding this comment.
If the user sends a top-level message (msg.threadTs is undefined), the bot currently posts its reply as a new top-level message using slack.post. Because the reply is not threaded, any subsequent user messages will also be top-level, resulting in a new ts and a different convKey (${msg.channel}:${msg.ts}). This completely breaks multi-turn conversation history. To preserve history, the bot should always reply in a thread by falling back to msg.ts when msg.threadTs is undefined.
async function postReply(slack: SlackClientLike, msg: SlackMessage, text: string): Promise<void> {
const result = await slack.reply(msg.channel, msg.threadTs ?? msg.ts, text);| }, | ||
|
|
||
| harness: 'claude', | ||
| model: 'claude-sonnet-4-6', |
There was a problem hiding this comment.
The model name claude-sonnet-4-6 is invalid or non-existent (the current latest is Claude 3.5 Sonnet or Claude 3.7 Sonnet). This will cause model resolution failures at runtime. Use a valid model identifier such as claude-3-5-sonnet.
| model: 'claude-sonnet-4-6', | |
| model: 'claude-3-5-sonnet', |
|
pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
…olidated skill
A Slack trigger mirrors the display-labelled path read-only and never covers the
bare-id writeback path, so a Slack WRITE always needs a scope — a trigger is not
enough. Surfaced by the linear-slack silent-drop bug (2026-06; orphaned draft
recovered from the live sandbox).
- starter persona.json: scope slack to /slack/channels/** instead of "slack": {}
(the example previously shipped the footgun and contradicted the real review
agent it models).
- scope warning: rewrite — a Slack trigger does NOT cover a Slack write; explain
the labelled-mirror vs bare-id path mismatch.
- §1: add the labelled-mirror sub-trap, correct the "trigger or scope" rule to
carve out Slack, and add the make-delivery-loud rule (empty ts ⇒ throw).
Refs AgentWorkforce/agents#53, AgentWorkforce/cloud#2029.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Document and vendor creating cloud persona skill refs * chore: apply pr-reviewer fixes for #73 * chore: apply pr-reviewer fixes for #73 * chore: apply pr-reviewer fixes for #73 * chore: apply pr-reviewer fixes for #73 * chore: apply pr-reviewer fixes for #73 * docs(creating-cloud-persona): fix the Slack-write footgun in the consolidated skill A Slack trigger mirrors the display-labelled path read-only and never covers the bare-id writeback path, so a Slack WRITE always needs a scope — a trigger is not enough. Surfaced by the linear-slack silent-drop bug (2026-06; orphaned draft recovered from the live sandbox). - starter persona.json: scope slack to /slack/channels/** instead of "slack": {} (the example previously shipped the footgun and contradicted the real review agent it models). - scope warning: rewrite — a Slack trigger does NOT cover a Slack write; explain the labelled-mirror vs bare-id path mismatch. - §1: add the labelled-mirror sub-trap, correct the "trigger or scope" rule to carve out Slack, and add the make-delivery-loud rule (empty ts ⇒ throw). Refs AgentWorkforce/agents#53, AgentWorkforce/cloud#2029. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: agent-relay-code[bot] <agent-relay-code[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
1 similar comment
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #53 in AgentWorkforce/agents. |
…em) (#54) Asked to "make an issue", the harness hand-wrote a JSON file into the read-only Linear mirror — inventing an AR-NN ref + UUID and copying an existing issue's shape (including read-only fields the writeback rejects). That is not a Linear mutation: nothing was created, yet the reply claimed "Done! Created AR-83". Route board WRITES through the real writeback instead of the harness filesystem: - persona.ts: the harness treats ./linear as READ-ONLY and, when a change is asked for, resolves real ids from the VFS and emits a fenced `linear-actions` block (create_issue / comment). It must not invent ids/refs, must not claim success in prose, and is told a milestone can't be set on create. - agent.ts: the handler parses that block and executes each action through `linearClient()` (draft → issueCreate/comment → receipt), allow-listing IssueCreateInput fields so a stray read-only field can't fail the create. It reports the CONFIRMED Linear url; because relay-helpers `created()` falls back to the draft path on a missing receipt, a non-http url is surfaced as "unconfirmed" rather than claimed done. Malformed/again-missing ids are refused, not guessed. - tests: new tests/linear-slack-agent.test.mjs — confirmed create, unconfirmed (no-receipt) create, missing-id refusal, comment, read-only turn, malformed block. Follow-up to #53 (which fixed delivery). Refs AgentWorkforce/cloud#2029. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ast ack (#55) Live trace of a real run: the issue WAS created (AR-87 in Launch SDK) but no reply ever posted. Two causes, one of them self-inflicted by #53: - The writeback receipt timeout defaults to 3s — too short for the cloud worker's round-trip. `slack.post()` returned `ts:''` and `createIssue()` returned the draft-path fallback even though both writes landed. - #53 made `postReply` THROW on an empty `ts`. With the scope mounted the draft still flushes at cleanup, so an empty `ts` is a slow receipt, not a drop — but the throw crashed the turn and tore the box down before the reply flushed. Issue created, channel silent. Fixes: - Build slack/linear clients with `writebackTimeoutMs: 12s` so receipts have time to arrive (keeps the box alive to flush) and we get real confirmation. - `postReply` no longer throws on a missing receipt — it logs `linear-slack.reply.no-receipt` and lets the draft flush. The genuine no-scope failure is already caught at build time by tests/persona-integration-scopes, not at runtime. - Reframe an unconfirmed create/comment from a scary "never confirmed" to "submitting now — appears on the board shortly" (creates land via the mirror within ~minutes), still logged for triage. - Add a fast 👀 ack: react on the teammate's message the moment the handler picks up the turn (fire-and-forget, doesn't block the harness), so the channel isn't silent for the minutes-long run. Follow-up to #53/#54. Refs AgentWorkforce/cloud#2029. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
What happened
linear-slackwas engaged in Slack and silently dropped its reply — nothing appeared in the channel, yet the run loggedhandler.ok. Traced live via the Daytona CLI into the running sandbox:slackClient().post()doesn't call the Slack API — it writes a draft into the VFS mount and polls for a writeback receipt. The reply draft was recovered, orphaned and undelivered, at/slack/channels/C0B9287EP6Y/messages/messages <uuid>.json.Root cause: the labelled-mirror footgun
slack: {}. The only Slack mount was the trigger's read-only mirror of the display-labelled channel path/slack/channels/{id}__{name}/messages./slack/channels/{id}/messages. They never coincide, so the draft landed on unmounted disk and the writeback worker never flushed it.vfs-client.writeJsonFilereturnsreceipt: undefinedon timeout andslackClient.postreturns{ ts: '' }without throwing → the runtime loggedhandler.okwith nothing delivered.(Bonus finding: the "AR-83" issue the agent claimed to create was also fake — it wrote local JSON into the read-only Linear mirror, not a real Linear mutation. Out of scope for this PR.)
Changes
persona.ts— fix delivery. Scope slack to/slack/channels/**so the writeback path is actually mounted. Same fix already shipped inreview/vendor-monitor/hn-monitor/repo-hygiene;linear-slackwas the last holdout.agent.ts— make failures loud.postReplynow treats an empty receipttsas a delivery failure and throws, so a dropped reply surfaces ashandler.errorinstead of a falsehandler.ok.tests/persona-integration-scopes.test.mjs— tighten the guard. The existing guard treated trigger OR scope as sufficient, which false-negatived this case (linear-slack has a slack trigger). New sub-rule: a write to a display-labelled provider (WRITEBACK_NEEDS_SCOPE={slack}) requires ascopeeven when a trigger exists. github/linear are unaffected — their trigger and writeback paths share one bare-id form. Proven red→green against the original config.This PR also adds the
linear-slackagent itself (it was deployed but never committed). It ships onlyagent.ts+persona.ts; README/graphics to follow if we want it in the showcase set.Preventing this for users
Repo-local guards/docs only protect this repo. Framework-level prevention is tracked in AgentWorkforce/cloud#2029 — chiefly: make
slackClientwrite methods reject on a missing receipt (so every future occurrence is a visible error, not a silent no-op), plus auto-mounting writeback paths and a persona-kit compile-time check. Thewriting-agent-personasskill §1 is updated with the labelled-mirror sub-trap (separate PR in AgentWorkforce/skills).Test
npm run typecheckclean;npm test54/54, including the tightened guard proven red on the broken config and green on the fix.🤖 Generated with Claude Code
Summary by cubic
Fixes undelivered Slack replies in
linear-slackby mounting the writeback path and failing fast on missing delivery receipts. Also adds the agent to the repo and tightens the scope guard to prevent future silent Slack writebacks.Bug Fixes
/slack/channels/**so writebacks use a mounted path and replies deliver.postReplynow throws when Slack returns an emptyts, surfacing dropped replies ashandler.error.scopeeven with a trigger; prevents drafts landing on unmounted disk. GitHub/Linear unaffected.New Features
linear-slackagent and persona (agent.ts,persona.ts).Written for commit cc1b2fc. Summary will update on new commits.