Skip to content

ComfyPR-Bot improvements: webhook retry, faster ack, sectioned progress, image vision, ❌-cancel#204

Merged
snomiao merged 22 commits into
mainfrom
sno-bot
Apr 25, 2026
Merged

ComfyPR-Bot improvements: webhook retry, faster ack, sectioned progress, image vision, ❌-cancel#204
snomiao merged 22 commits into
mainfrom
sno-bot

Conversation

@snomiao

@snomiao snomiao commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

ComfyPR-Bot behavior improvements (16 items total), shipped in phases across Slack webhook, UX, cost, features, and reliability.

Reliability (Phase 1)

UX / response quality (Phase 2)

  • ComfyRegistry PR DASHBOARD #1 Quick Respond sectioned structure (📋 Understanding / 🔍 Progress / 📎 Deliverables / ✅ Done) — drop the "one line per update" constraint so long-running tasks show real progress.
  • Making custom nodes installable via pip #2 Async intent analysis — post the "👀 Received..." placeholder immediately and run the GPT call in parallel, then edit the placeholder (first reply 5-10s → ~1s).
  • fix(sno-checking-prs-failures): done #12 Dynamic update model — diffs <2000 chars use gpt-4o-mini, only large diffs use gpt-4o (~70-80% cost reduction).

Feature additions (Phase 3)

Cost / reliability finalization (Phase 4)

Test plan

  • typecheck (bunx tsc --noEmit)
  • bun test bot/taskInputFlow.spec.ts (4/4 pass)
  • Webhook retry dedup live test — confirmed Ignoring duplicate webhook event log
  • Env prefix log on startup — [env] SLACK_SIGNING_SECRET=f944ab... | ...
  • Bot running under PM2, listening for webhooks on port 3475
  • End-to-end Slack DM check: placeholder → intent analysis → sectioned progress edits
  • Confirm 📎 Deliverables surfaces PR URLs after a PR-creation task
  • Confirm ❌ reaction aborts a running task
  • Confirm Claude vision input works on a DM with image attachments

Design notes in tmp/improvement-plan.md (gitignored).

🤖 Generated with Claude Code

snomiao and others added 18 commits March 20, 2026 13:55
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
git-subtree-dir: lib/slack-cli
git-subtree-split: 0e5b91258f5d91aee4f2d17edbc3bab44bf5dc6b
…put drain cleanup

Addresses Copilot review comments:
- Extract sendSlackUpdate() so it can be called from interval AND finally block
- Add 10s minimum throttle between LLM-synthesized Slack updates (avoids rate limits)
- Call sendSlackUpdate() explicitly in finally block (was broken: interval cleared first)
- Abort controller in finally to cancel inputDrainPromise reader

Amp-Thread-ID: https://ampcode.com/threads/T-019d47cc-caf7-740e-a487-be1f0e3443f8
Co-authored-by: Amp <amp@ampcode.com>
… pooling

Addresses Copilot review: client() was creating a new Client per request,
defeating connection pooling.

Amp-Thread-ID: https://ampcode.com/threads/T-019d47cc-caf7-740e-a487-be1f0e3443f8
Co-authored-by: Amp <amp@ampcode.com>
…nvalid markdown type

Addresses Copilot review: Slack Block Kit does not support 'markdown' block type.
Use 'section' block with 'mrkdwn' text object instead.

Amp-Thread-ID: https://ampcode.com/threads/T-019d47cc-caf7-740e-a487-be1f0e3443f8
Co-authored-by: Amp <amp@ampcode.com>
…ersion, caching, and safe messaging

Standalone utilities ported from lib/slack/ with zero project-specific deps:
- parse-url: parseSlackUrl() + slackMessageUrlParse() with team extraction
- ts-to-iso: slackTsToISO() timestamp conversion
- parse-markdown: sync + async mrkdwn→markdown with optional resolver
- safe-message: truncateSlackText() + truncateSlackBlocks()
- cached-client: createCachedSlack() in-memory Map cache proxy

Amp-Thread-ID: https://ampcode.com/threads/T-019d47cc-caf7-740e-a487-be1f0e3443f8
Co-authored-by: Amp <amp@ampcode.com>
- Add bot/task-user.ts: create/manage task-* system users with comfy-tasks group
- Add bot/spawn-as-user.ts: custom spawnClaudeCodeProcess using sudo -u per task
- Update bot/slack-bot.ts: integrate task user creation before agent spawn, add hourly cleanup, fix --continue IIFE bug (async fn was declared but never called)
- Update lib/zChat.ts: replace require() with async import() for ESM compatibility, add initZChat()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace SocketModeClient with Bun HTTP /slack/events endpoint
- Verify requests with HMAC-SHA256 using SLACK_SIGNING_SECRET
- Handle url_verification challenge for Slack App setup
- Extract handleSlackEvent() for clean event dispatch
- Caddy: add prbot.stukivx.xyz → :3475 route

Webhook URL: https://prbot.stukivx.xyz/slack/events

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- webhook handler: dedup duplicate events by event_id (1h TTL) so Slack
  retries (x-slack-retry-num) no longer trigger 3x task execution
- bot/index.ts: load .env.local with override semantics + log secret
  prefixes at startup to prevent stale shell env shadowing the file
  (regression from the 2026-04-24 signing-secret mismatch incident)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- fast-ack: post a "👀 受け取りました" placeholder before the intent LLM
  call so users see activity within ~1s instead of waiting 5-10s for
  the GPT-4o synthesis. Intent analysis now edits the placeholder.
- intent model: gpt-4o → gpt-4o-mini (classification-only, same accuracy
  for intent extraction at ~3x throughput / lower cost).
- progress template: switch from "1-line-per-update" constraint to a
  fixed section layout (📋 理解 / 🔍 進捗 / 📎 成果物 / ✅ 完了) so
  users can actually see what the agent is doing on long tasks.
- progress model: route small diffs (<2000 chars) to gpt-4o-mini;
  large diffs stay on gpt-4o. ~70-80% token cost reduction per task.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- PR URL extraction: scan agent output for github.com/.../pull/N URLs
  and pin them to the 📎 成果物 section so freshly-opened PRs can never
  fall off the message as the agent generates new findings.
- ❌ reaction cancel: subscribe reaction_added events; reacting to the
  original user message with ❌ aborts the matching task's
  AbortController. Author-only check prevents bystanders from killing
  someone else's task.
- image vision: download image attachments from the triggering Slack
  message into ./attachments/ at workspace creation, list them in
  PROMPT.txt so the Claude agent can open them with the Read tool
  (vision-capable). Non-image files are skipped.

Slack App config: subscribe to reaction_added (reactions:read scope)
to enable cancellation. Existing app_mention / message.* events remain.

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

- maxTurns by complexity: intent LLM now also classifies complexity
  (simple/medium/complex) and the agent gets 40/100/200 turns
  respectively. Caps runaway cost on simple lookups while leaving room
  for complex multi-repo work.
- content-hash dedup: include sha256(text)[:8] in the dedup key so
  message edits with new intent re-trigger, but identical retries
  within 10s still suppress.
- message_changed support: flatten Slack's nested edit payload so the
  rest of the pipeline can treat edits like fresh messages (paired with
  the content-hash dedup above).
- ErrorCollector event-driven: fs.watch (recursive, per-file 500ms
  debounce) replaces the 10s polling loop. Slow 60s safety-net poll
  still runs in case the FS layer drops events.
- thread context compression: when a thread exceeds 20 messages, keep
  the latest 15 verbatim and let gpt-4o-mini summarize the older ones
  (decisions, open questions, named files/PRs, errors). Slashes intent
  prompt token usage on long threads.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- agent self-updates: instruct the agent in .bot/AGENT.md to call
  prbot slack update directly on milestones (plan / subtask done /
  blocker / 30-60s heartbeat). Agent now owns the user-facing message
  format using the same section structure (📋/🔍/📎/✅).
- synthesizer fallback only: bump the GPT-4o synthesizer interval
  from 10s → 30s. The agent's direct updates are primary; the
  synthesizer is just a safety net for agents that go quiet on Slack.
- follow-up drain test: cover the TransformStream contract slack-bot.ts
  relies on for piping mid-task Slack messages into a running SDK
  session — order preservation, single-reader drain, close semantics,
  release-and-reacquire writers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 24, 2026 23:42
@vercel

vercel Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
comfy-pr Ready Ready Preview, Comment Apr 25, 2026 6:10pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades ComfyPR-Bot’s Slack integration and agent runtime to improve reliability (webhook retries/dedup), responsiveness (fast placeholder + async intent), UX (sectioned progress), and capability (image attachments + ❌ cancel), while also adding a vendored slack-cli subtree and a CLI feedback reporter.

Changes:

  • Replace Socket Mode handling with Slack Events API webhook + signature verification and 1h event_id dedup.
  • Move agent execution to Claude Agent SDK with per-task user isolation, follow-up message streaming, progress section updates, PR URL surfacing, image downloads, and reaction-based cancellation.
  • Add slack-cli subtree tooling + subtree sync scripts, plus a prbot feedback command posting to Slack.

Reviewed changes

Copilot reviewed 30 out of 35 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
scripts/subtree-sync.sh Adds a helper to pull/push configured git subtrees.
package.json Adds subtree scripts and Claude Agent SDK dependency.
bun.lock Locks Claude Agent SDK and transitive deps.
inbox.yaml Adds inbox file used for “missed” message replay.
lib/zChat.ts Switches z-chat-completion loading to async/dynamic import wrapper.
lib/slack/feedback.ts Adds Slack feedback posting utility + CLI entry.
lib/slack-cli/ts/ts-to-iso.ts Adds TS helper for Slack timestamp → ISO conversion.
lib/slack-cli/ts/safe-message.ts Adds truncation utilities for Slack text/blocks.
lib/slack-cli/ts/parse-url.ts Adds Slack message URL parsing helpers.
lib/slack-cli/ts/parse-markdown.ts Adds Slack mrkdwn → Markdown conversion helpers.
lib/slack-cli/ts/index.ts Exports slack-cli TS utilities.
lib/slack-cli/ts/cached-client.ts Adds a caching proxy wrapper for Slack WebClient calls.
lib/slack-cli/src/slack.rs Introduces direct Slack Web API client in Rust (reqwest).
lib/slack-cli/src/main.rs Adds Rust slack CLI (msgs/news/search/send).
lib/slack-cli/src/lib.rs Adds NAPI bindings for Slack operations.
lib/slack-cli/package.json Adds NAPI build scripts/targets for slack-cli addon.
lib/slack-cli/index.js Adds platform-aware loader for the NAPI binary.
lib/slack-cli/index.d.ts Adds TS declarations for the NAPI exports.
lib/slack-cli/bun.lock Adds lockfile for slack-cli subproject.
lib/slack-cli/build.rs Sets up napi-build in Rust build step.
lib/slack-cli/README.md Documents slack-cli usage/configuration.
lib/slack-cli/LICENSE Adds MIT license for slack-cli subtree.
lib/slack-cli/Cargo.toml Defines Rust crate/bin/deps for slack-cli + NAPI.
lib/slack-cli/.gitignore Ignores build artifacts for slack-cli.
lib/slack-cli/.github/workflows/build.yml Adds CI workflow to build/release multi-target artifacts.
bot/taskInputFlow.spec.ts Adds unit tests for TransformStream “drain” contract used for follow-ups.
bot/task-user.ts Adds per-task Linux user lifecycle management + stale cleanup.
bot/spawn-as-user.ts Adds sudo-based spawner to run agent subprocesses as task users.
bot/slack-bot.ts Major refactor: webhook mode, dedup, placeholder ack, sectioned progress, SDK agent run, cancel, image download, thread summarization, dynamic model selection.
bot/index.ts Loads .env.local with override semantics and logs secret prefixes.
bot/error-collector.ts Changes to fs.watch-driven error detection with slow polling fallback.
bot/cli.ts Adds prbot feedback command to post to Slack feedback channel.
CLAUDE.md Adds PM2 operational docs for running the Slack bot.
.bot/AGENT.md Updates agent guidance for live Slack progress updates and section structure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bot/slack-bot.ts Outdated
Comment thread bot/slack-bot.ts
Comment thread bot/spawn-as-user.ts
Comment thread lib/slack/feedback.ts Outdated
Comment thread lib/zChat.ts
Comment thread bot/slack-bot.ts
Comment thread bot/slack-bot.ts Outdated
@snomiao
snomiao requested a review from Copilot April 24, 2026 23:53
@snomiao

snomiao commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/copilot review

- image download: prefix saved filename with file.id (or array index) to
  prevent collision when two attachments share a name; prefer
  url_private_download over url_private; cap downloads at 25MB so an
  oversized upload can't blow up the workspace.
- dedup TTL: webhook-event keys already had a 1h TTL, but the per-message
  msg-{ts}-{hash} entries were written without one and would accumulate
  forever in Nedb+Mongo. Match the 1h TTL.
- task-user cleanup: build activeIds from (thread_ts || ts) so it matches
  workspaceId. Threaded long-running tasks no longer get marked stale and
  have their isolated Linux user deleted out from under them.
- input drain: release the TransformStream reader lock in a finally block
  so cleanup is deterministic and the stream isn't left half-locked.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes ComfyPR-Bot’s Slack integration and agent execution model, focusing on reliability (webhook retries/dedup), faster user feedback, and operational safety/cost controls while adding supporting tooling (feedback command and Slack CLI subtree).

Changes:

  • Switch Slack bot to webhook-based Events API with signature verification and 1h event_id dedup; add faster placeholder ack and sectioned progress updates.
  • Migrate agent execution to Anthropic Claude Agent SDK with per-task Linux user isolation, abort-by-❌ reaction, image attachment download for vision, and dynamic maxTurns/model selection.
  • Add CLI feedback submission to Slack, plus vendored lib/slack-cli subtree utilities and build workflows.

Reviewed changes

Copilot reviewed 30 out of 35 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
scripts/subtree-sync.sh Add git subtree pull/push helper for syncing vendored deps.
package.json Add subtree scripts and Claude Agent SDK dependency.
bun.lock Lockfile update for new dependency set.
lib/zChat.ts Change z-chat-completion loading to async dynamic import + init hook.
lib/slack/feedback.ts New Slack feedback poster + channel resolver/auto-create logic.
lib/slack-cli/ts/ts-to-iso.ts Utility for converting Slack ts to ISO.
lib/slack-cli/ts/safe-message.ts Helpers to truncate Slack text/blocks.
lib/slack-cli/ts/parse-url.ts Parse Slack message URLs into channel/ts tuples.
lib/slack-cli/ts/parse-markdown.ts Convert Slack mrkdwn to Markdown (sync/async).
lib/slack-cli/ts/index.ts Barrel exports for TS slack-cli utilities.
lib/slack-cli/ts/cached-client.ts Proxy-based caching wrapper for Slack WebClient calls.
lib/slack-cli/src/slack.rs Rust Slack Web API client via reqwest.
lib/slack-cli/src/main.rs Rust CLI entrypoint implementing msgs/news/search/send.
lib/slack-cli/src/lib.rs NAPI bindings exposing Slack functions to Node.
lib/slack-cli/package.json NAPI package metadata + build scripts/targets.
lib/slack-cli/index.js Runtime loader for platform-specific .node binary.
lib/slack-cli/index.d.ts Generated TS typings for the NAPI module.
lib/slack-cli/bun.lock Nested lockfile for slack-cli subproject.
lib/slack-cli/build.rs NAPI build setup hook.
lib/slack-cli/README.md Documentation for the Slack CLI subproject.
lib/slack-cli/LICENSE MIT license for the Slack CLI subproject.
lib/slack-cli/Cargo.toml Rust crate config (bin + cdylib + deps).
lib/slack-cli/.gitignore Ignore Rust/node build outputs for subproject.
lib/slack-cli/.github/workflows/build.yml CI workflow to build/upload multi-target binaries.
inbox.yaml Rename/initialize inbox tracking file for missed messages.
bot/taskInputFlow.spec.ts Add unit tests for TransformStream drain contract.
bot/task-user.ts Implement per-task Linux user creation/activity tracking/cleanup.
bot/spawn-as-user.ts Spawn Claude Code CLI as per-task user via sudo.
bot/slack-bot.ts Major bot refactor: webhook handler, dedup, quick ack, SDK agent, cancel, vision, summaries, cost controls.
bot/index.ts Load .env.local with override semantics + log secret prefixes.
bot/error-collector.ts Switch error collection to fs.watch + slow polling safety net.
bot/cli.ts Add prbot feedback command posting to Slack.
CLAUDE.md Add PM2 startup/runbook notes for bot operation.
.bot/AGENT.md Instruct agents to drive Slack progress updates via prbot slack update.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bot/index.ts Outdated
Comment thread bot/slack-bot.ts Outdated
Comment thread bot/slack-bot.ts
Comment thread bot/slack-bot.ts
Comment thread bot/slack-bot.ts
Comment thread bot/spawn-as-user.ts
Comment thread lib/zChat.ts
Comment thread lib/slack/feedback.ts Outdated
Comment thread lib/slack/feedback.ts Outdated
Comment thread lib/slack-cli/ts/cached-client.ts
Round 2 of review fixes (codex P0 + 2nd Copilot pass on PR #204).

Critical
- webhook team_id: Slack's Events API places the workspace id on the
  envelope (payload.team_id), not on the inner event in some payload
  shapes. zAppMentionEvent and the message-event guard both require
  team, so app_mentions/DMs delivered via webhook were getting silently
  dropped — bot 200'd but never processed. Forward team_id onto event.

Hardening
- env allowlist: don't spread process.env into the Claude agent
  subprocess. Pass an explicit set (HOME/PATH/locale, GH/OPENAI/NOTION
  tokens, SLACK_BOT_TOKEN for prbot slack update, MONGODB_URI, log
  flags). Slack signing/socket secrets and other unrelated creds no
  longer leak into bash-tool reach.
- follow-up drain buffer: hold values that arrive before agentQuery is
  ready in earlyBuffer, then flush in order once the SDK is up. Sets
  agentReady right after query() so we don't drop early follow-ups.
- env log prefix: 12 → 6 chars (still enough for an operator to verify
  the right secret loaded, fewer characters available if the log leaks).

Feedback (lib/slack/feedback.ts) — codex P2 + Copilot
- Lazy env read: feedbackChannel() resolves PRBOT_FEEDBACK_CHANNEL on
  call so .env.local override applied inside the CLI handler is honored.
- Channel ID regex: accept C/G/D prefixes (was C-only), so private
  channel IDs and DM IDs pass through without an unnecessary list/create.
- Pagination: walk response_metadata.next_cursor to find channels past
  the first 1000.
- Fail closed: never auto-create the feedback channel. Earlier code
  created it as is_private:false, which would leak the very first
  submission (stack traces, command output) into a brand new public
  channel if the env var was misconfigured. Now we throw a clear setup
  error instead.

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

snomiao commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in commits 1e89d22 and 3d0cbdf:

Critical (codex P0)

  • bot/slack-bot.ts:306 — webhook now forwards payload.team_id onto event.team so zod-required team field is present for Events API payloads (was silently dropping mentions/DMs from webhooks). Both Copilot and codex flagged this.

Copilot 1st-pass fixes

  • bot/slack-bot.ts:1031 (image download) — prefixed filename with file.id (or array index) to avoid collisions; preferred url_private_download; added 25MB size cap.
  • bot/slack-bot.ts:1210 (reader lock) — added try/finally with reader.releaseLock().
  • bot/slack-bot.ts:533 (dedup TTL) — msg-{ts}-{hash} keys now have 1h TTL to match webhook-event-*.
  • bot/slack-bot.ts:393 (cleanupStaleTaskUsers) — activeIds now built from thread_ts || ts so threaded long-running tasks don't get marked stale.

Copilot 2nd-pass fixes

  • bot/index.ts:44 (token prefix) — reduced to 6 chars (was 12) for SLACK_BOT_TOKEN.
  • bot/slack-bot.ts:1206 (early follow-up drop) — added earlyBuffer + agentReady flag; values that arrive before the SDK is up are queued and flushed in order once agentQuery is created.
  • bot/slack-bot.ts:1403 (env spread) — replaced ...process.env with an explicit allowlist (HOME/PATH/locale, GH/OPENAI/NOTION tokens, SLACK_BOT_TOKEN for prbot slack update, MONGODB_URI, log flags). Slack signing/socket secrets no longer reach the agent's bash tool.

Feedback safety (codex P2 + Copilot)

  • lib/slack/feedback.ts: lazy env read, ID regex accepts C/G/D prefixes, paginated channel lookup, and fail-closed (no auto-create as public). The previous code would leak the first submission's stack traces if PRBOT_FEEDBACK_CHANNEL was mistyped.

Deferred (out of scope for this PR / pre-existing)

  • spawn-as-user.ts hardcoded paths — works in this single-tenant deployment; can revisit when multi-host.
  • lib/zChat.ts hardcoded path — pre-existing, separate issue.
  • lib/slack-cli/ts/cached-client.ts cache key — pre-existing in subtree, separate issue.

- prbot slack read @snomiao → resolve user via users.list pagination,
  open DM via conversations.open, then read recent messages.
- gitignore: add local tool artifacts (.codex, .playwright-mcp/,
  .playwright-cli-multi-tab/) so rech/codex output doesn't show up as
  untracked noise on every git status.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rebase was tried first but failed because the slack-cli subtree commits
on this branch are duplicated by PR #187 (already on main); a merge is
much safer and produces the same end state for PR #204.

Conflict resolutions
- CLAUDE.md: keep main's createIndex security note alongside our PM2
  bot startup section.
- bot/slack-bot.ts: keep ours. Our version is a superset — webhook
  mode, sectioned progress, dedup, image vision, ❌-cancel, follow-up
  drain, dynamic maxTurns — already includes the throttle/extract
  cleanup that main's e0abd3e introduced.
- lib/slack/feedback.ts: keep ours. Our version has the privacy fix
  (fail-closed instead of auto-creating a public channel) and lazy env
  read for PRBOT_FEEDBACK_CHANNEL.
- bun.lock: take main's; lockfile is downstream of package.json which
  has only additive merges.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@snomiao
snomiao merged commit 99d4473 into main Apr 25, 2026
5 checks passed
@snomiao
snomiao deleted the sno-bot branch April 25, 2026 18:32
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants