Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
git-subtree-dir: lib/slack-cli git-subtree-split: 0e5b91258f5d91aee4f2d17edbc3bab44bf5dc6b
Amp-Thread-ID: https://ampcode.com/threads/T-019d47cc-caf7-740e-a487-be1f0e3443f8 Co-authored-by: Amp <amp@ampcode.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR vendors in the snomiao/slack-cli project as a git subtree and adds tooling to sync subtrees, while also updating the existing Slack bot runtime to use @anthropic-ai/claude-agent-sdk and renaming the persisted inbox file.
Changes:
- Added
scripts/subtree-sync.shand npm scripts to pull/push configured git subtrees. - Added
lib/slack-cli/subtree (Rust Slack CLI + NAPI bindings, build workflow, lockfiles). - Updated Slack bot to use
@anthropic-ai/claude-agent-sdk, renamedmsgs.yaml→inbox.yaml, and changed streaming/update behavior.
Reviewed changes
Copilot reviewed 15 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/subtree-sync.sh | Adds a bash helper to pull/push configured git subtrees. |
| package.json | Adds subtree sync scripts and introduces @anthropic-ai/claude-agent-sdk. |
| bun.lock | Updates lockfile for the new dependency. |
| bot/slack-bot.ts | Refactors agent execution to use the Claude Agent SDK; adjusts inbox filename and message-update loop. |
| inbox.yaml | Adds/initializes the new inbox file tracked by the bot. |
| CLAUDE.md | Expands bot operational notes (PM2 startup/runbook). |
| lib/slack-cli/src/slack.rs | Adds a direct Slack Web API client implementation (reqwest). |
| lib/slack-cli/src/main.rs | Adds the Rust CLI entrypoint and commands. |
| lib/slack-cli/src/lib.rs | Exposes Slack functions to Node via NAPI. |
| lib/slack-cli/Cargo.toml | Defines Rust crate/bin and dependencies. |
| lib/slack-cli/Cargo.lock | Locks Rust dependencies for the subtree. |
| lib/slack-cli/README.md | Documents the Slack CLI usage/config. |
| lib/slack-cli/package.json | Adds NAPI build config for the subtree package. |
| lib/slack-cli/index.js | Loads the correct .node binary for the host platform. |
| lib/slack-cli/index.d.ts | Type declarations for the NAPI bindings. |
| lib/slack-cli/build.rs | Sets up napi-build for compiling the addon. |
| lib/slack-cli/.gitignore | Ignores build outputs for the subtree. |
| lib/slack-cli/LICENSE | Adds license file for the subtree. |
| lib/slack-cli/bun.lock | Adds Bun lockfile for the subtree’s JS tooling. |
| lib/slack-cli/.github/workflows/build.yml | Subtree CI workflow for building bindings/binaries (nested workflow). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Drain taskInputFlow into the SDK agent | ||
| const inputDrainPromise = (async () => { | ||
| const reader = taskInputFlow.readable.getReader(); | ||
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| if (value && agentQuery !== null) { | ||
| const userMsg: SDKUserMessage = { | ||
| type: "user" as const, | ||
| message: { role: "user" as const, content: value }, | ||
| parent_tool_use_id: null, | ||
| session_id: "", | ||
| }; | ||
| await (agentQuery as Query).streamInput( | ||
| (async function* () { | ||
| yield userMsg; | ||
| })(), | ||
| ); | ||
| logger.info(`Injected follow-up message into SDK agent: ${value.slice(0, 100)}`); | ||
| } | ||
| } | ||
| const text = buffer.toString(); | ||
| // Write raw output to dedicated stdout log file | ||
| await appendFile(stdoutLogPath, text).catch(() => {}); | ||
| return text; | ||
| }) | ||
| } catch { | ||
| // taskInputFlow closed | ||
| } | ||
| })(); |
There was a problem hiding this comment.
taskInputFlow is drained into the Claude Agent SDK via getReader(), but there are no writes to taskInputFlow.writable anywhere in this file (and the only TaskInputFlows.get(...) branch is effectively disabled). As a result, follow-up Slack messages will never reach the running agent, and the drain loop can sit blocked forever. Either wire follow-up thread messages into taskInputFlow.writable (and close/cancel it on task completion) or remove the unused stream plumbing; also consider awaiting/cancelling inputDrainPromise in the task cleanup path to avoid leaking readers.
There was a problem hiding this comment.
Fixed in e0abd3e — added abortController.abort() in the finally block to cancel the input drain reader. The variable is intentionally fire-and-forget (the IIFE runs in background), so keeping it as-is without await is correct for this pattern.
bot/slack-bot.ts
Outdated
| // Periodic Slack update interval | ||
| const slackUpdateInterval = setInterval(async () => { | ||
| if (agentOutput === lastSentOutput || !agentOutput) return; | ||
|
|
||
| const rawTerminalOutput = tr.render().split("\n").slice(-80).join("\n"); | ||
| const my_internal_thoughts = cleanTerminalOutput(rawTerminalOutput); | ||
| // const my_internal_thoughts = tr.tail(80); | ||
| logger.debug( | ||
| "Raw terminal output (before cleaning): " + | ||
| yaml.stringify({ preview: rawTerminalOutput.slice(0, 200) }), | ||
| ); | ||
| logger.info( | ||
| "Cleaned output preview: " + | ||
| yaml.stringify({ | ||
| preview: my_internal_thoughts.slice(0, 200), | ||
| news_preview: news.slice(0, 200), | ||
| }), | ||
| ); | ||
| const news = agentOutput.slice(lastSentOutput.length); | ||
| lastSentOutput = agentOutput; | ||
|
|
||
| // send update to slack | ||
| const updateText = sent || "_(no output yet)_"; | ||
| const contexts = { | ||
| my_internal_thoughts, | ||
| news, | ||
| user_original_intent: resp.user_intent, | ||
| my_response_md_original: quickRespondMsg.text || "", | ||
| }; | ||
| const updateResponseResp = (await zChatCompletion({ | ||
| my_response_md_updated: z.string(), | ||
| })` | ||
| const my_internal_thoughts = agentOutput.split("\n").slice(-80).join("\n"); | ||
| logger.info( | ||
| "Agent output preview: " + | ||
| yaml.stringify({ | ||
| preview: my_internal_thoughts.slice(0, 200), | ||
| news_preview: news.slice(0, 200), | ||
| }), | ||
| ); | ||
|
|
||
| // GPT-4o synthesis for Slack update | ||
| const contexts = { | ||
| my_internal_thoughts, | ||
| news, | ||
| user_original_intent: resp.user_intent, | ||
| my_response_md_original: quickRespondMsg.text || "", | ||
| }; | ||
| const updateResponseResp = (await zChatCompletion({ | ||
| my_response_md_updated: z.string(), | ||
| })` | ||
| TASK: Update my my_response_md_original based on agent's my_internal_thoughts findings, and give me my_response_md_updated to post in slack. |
There was a problem hiding this comment.
The periodic Slack update loop runs every 2s and, on each change, calls zChatCompletion(...) and then chat.update. This is very likely to hit Slack rate limits and will also drive significant LLM cost under normal agent output streaming. Consider adding throttling/debouncing (e.g., minimum N seconds between updates, coalescing output), and only triggering the LLM synthesis on meaningful milestones (tool completion, result messages) rather than every output increment.
There was a problem hiding this comment.
Fixed in e0abd3e — added 10s minimum throttle (MIN_SLACK_UPDATE_INTERVAL_MS) between LLM-synthesized updates, and changed interval from 2s to 10s. This prevents both Slack rate limits and excessive LLM cost.
| } finally { | ||
| clearInterval(slackUpdateInterval); | ||
| // Remove loading icon if still showing | ||
| if (isThinking && quickRespondMsg.ts && quickRespondMsg.channel) { | ||
| await slack.reactions | ||
| .remove({ | ||
| name: "loading", | ||
| channel: quickRespondMsg.channel, | ||
| timestamp: quickRespondMsg.ts, | ||
| }) | ||
| .onFlush(() => clearInterval(id)) | ||
| .run(); | ||
| }) | ||
|
|
||
| // show contents in console if needed for debugging | ||
| // .forkTo((e) => e.pipeTo(fromWritable(process.stdout))) | ||
| // .forkTo((e) => e.pipeTo(fromWritable(process.stdout))) | ||
| .run(); | ||
| .catch(() => {}); | ||
| } | ||
| // Trigger one final Slack update with complete output | ||
| lastSentOutput = ""; // force update | ||
| } |
There was a problem hiding this comment.
In the finally block you clear slackUpdateInterval and then set lastSentOutput = "" with a comment about forcing a final Slack update, but the interval has already been stopped so no final update will actually run. If you want to guarantee the last agent output/result is reflected in Slack, invoke the update routine once explicitly (or stop the interval only after the final update has been sent).
There was a problem hiding this comment.
Fixed in e0abd3e — extracted sendSlackUpdate() as a named function. In the finally block, we now reset lastSlackUpdateTime = 0 (to bypass throttle) and call await sendSlackUpdate() explicitly after clearing the interval.
lib/slack-cli/src/slack.rs
Outdated
| fn client() -> Client { | ||
| Client::new() | ||
| } | ||
|
|
||
| async fn get(token: &str, method: &str, params: &[(&str, &str)]) -> Result<Value> { | ||
| let resp = client() | ||
| .get(format!("https://slack.com/api/{method}")) | ||
| .bearer_auth(token) | ||
| .query(params) | ||
| .send() | ||
| .await? | ||
| .json::<Value>() |
There was a problem hiding this comment.
client() constructs a new reqwest::Client for every request. This defeats connection pooling and adds avoidable overhead. Prefer a single shared client (e.g., static OnceLock<Client>/LazyLock) reused by get/post.
There was a problem hiding this comment.
Fixed in ace0f4f — client() now returns a shared &'static Client via LazyLock<Client>, enabling connection pooling across all requests.
lib/slack-cli/src/slack.rs
Outdated
| let mut body = serde_json::json!({ | ||
| "channel": channel, | ||
| "text": text, // fallback for notifications | ||
| "blocks": [{ "type": "markdown", "text": text }] |
There was a problem hiding this comment.
The chat.postMessage payload uses blocks: [{ "type": "markdown", "text": text }], but Slack Block Kit does not support a markdown block type; this will likely fail with invalid_blocks. Use a section block with a text object of type mrkdwn (and keep text as the fallback).
| "blocks": [{ "type": "markdown", "text": text }] | |
| "blocks": [{ | |
| "type": "section", | |
| "text": { | |
| "type": "mrkdwn", | |
| "text": text | |
| } | |
| }] |
There was a problem hiding this comment.
Fixed in cf0707c — changed to section block with mrkdwn text object as suggested.
| // Create dedicated log files for this task | ||
| const taskLogDir = `${botWorkingDir}/.logs`; | ||
| await mkdir(taskLogDir, { recursive: true }); | ||
| const stdoutLogPath = `${taskLogDir}/claude-yes-stdout.log`; | ||
| const stderrLogPath = `${taskLogDir}/claude-yes-stderr.log`; | ||
| const agentLogPath = `${taskLogDir}/agent-output.log`; | ||
| const statusLogPath = `${taskLogDir}/STATUS.txt`; | ||
|
|
||
| // create a user for task | ||
| const exitCodePromise = Promise.withResolvers<number | null>(); | ||
| const sh = (() => { | ||
| // if (process.env.CLI === "amp") { | ||
| // // use amp | ||
| // } | ||
| // const continueArgs: string[] = []; | ||
| // if (botworkingdir/.claude-yes have content) | ||
| // then continueArgs.push('--continue') | ||
| // TODO: maybe use smarter way to detect if need continue | ||
| // if (existsSync(`${botWorkingDir}/.claude-yes`)) { | ||
| // // const stat = Bun.statSync(`${botWorkingDir}/.claude-yes`) | ||
| // // if (stat.isDirectory && stat.size > 0) { | ||
| // // } | ||
| // continueArgs.push('--continue') | ||
| // } | ||
| // const cmd = `bunx claude-yes -i=1d -- ${Bun.$.escape(agentPrompt)}`; | ||
| // const cli = cmd.split(" ")[0]; | ||
| const cli = "claude-yes"; // Use the globally installed claude-yes (via bun) | ||
| // Pass prompt to read PROMPT.txt and TODO.md | ||
| const args = [ | ||
| "--exit-on-idle=1m", | ||
| "--", | ||
| "Please read PROMPT.txt and TODO.md in the current directory and complete all tasks listed there.", | ||
| ]; | ||
| logger.info( | ||
| `Spawning process: ${cli} ${args.join(" ")} in ${botWorkingDir} with env GH_TOKEN_COMFY_PR_BOT=[REDACTED]`, | ||
| ); | ||
| const shell = spawn(cli, args, { | ||
| cwd: botWorkingDir, | ||
| env: { | ||
| ...process.env, | ||
| GH_TOKEN: process.env.GH_TOKEN_COMFY_PR_BOT || DIE("missing GH_TOKEN_COMFY_PR_BOT env"), | ||
| GITHUB_TOKEN: process.env.GH_TOKEN_COMFY_PR_BOT || DIE("missing GH_TOKEN_COMFY_PR_BOT env"), | ||
| }, | ||
| }); | ||
|
|
||
| // check if p spawned successfully | ||
| shell.on("error", (err) => { | ||
| logger.error(`Failed to start ${cli} process for task ${workspaceId}:`, { err }); | ||
| }); | ||
| shell.on("exit", (code, signal) => { | ||
| logger.info(`process for task ${workspaceId} exited with code ${code} and signal ${signal}`); | ||
| exitCodePromise.resolve(code); | ||
| }); | ||
|
|
||
| // Auto-answer the trust prompt (option 1 = "Yes, proceed") | ||
| if (shell.stdin) { | ||
| shell.stdin.write("1\n"); | ||
| } | ||
|
|
||
| // Stream stderr to log file and logger (async operations moved outside) | ||
| shell.stderr?.on("data", (data) => { | ||
| const text = data.toString(); | ||
| appendFile(stderrLogPath, text).catch(() => {}); | ||
| logger.warn(`[${cli} stderr]:`, { data: text }); | ||
| }); | ||
|
|
||
| // Check if stdout/stderr are available | ||
| if (!shell.stdout) { | ||
| logger.error(`Process ${cli} has no stdout stream!`); | ||
| } | ||
| if (!shell.stderr) { | ||
| logger.warn(`Process ${cli} has no stderr stream`); | ||
| } | ||
|
|
||
| return shell; | ||
| })(); | ||
|
|
||
| // Write initial status | ||
| await Bun.write( | ||
| statusLogPath, | ||
| `Started: ${new Date().toISOString()}\nPID: ${sh.pid}\nStatus: Running\nLog: ${stdoutLogPath}\n`, | ||
| ); | ||
|
|
||
| const isDebugMode = process.env.DEBUG === "true" || process.env.DEBUG === "1"; | ||
|
|
||
| logger.info(`Spawned claude-yes process with PID ${sh.pid} for task ${workspaceId}`); | ||
| if (isDebugMode) { | ||
| logger.info(`📝 Real-time logs: tail -f ${stdoutLogPath}`); | ||
| logger.info(`📊 Status file: cat ${statusLogPath}`); | ||
| logger.info(`💡 Debug commands: prbot debug watch ${botWorkingDir}`); | ||
| } | ||
|
|
||
| // Start error collector to monitor workspace for errors | ||
| const errorLogPath = `${taskLogDir}/COLLECTED_ERRORS.md`; | ||
| const errorCollector = new ErrorCollector({ | ||
| workspaceDir: botWorkingDir, | ||
| outputLogPath: errorLogPath, | ||
| onError: isDebugMode | ||
| ? (errorPath, content) => { | ||
| logger.warn(`⚠️ Error detected in workspace: ${errorPath}`); | ||
| ? (errorPath: string, content: string) => { | ||
| logger.warn(`Error detected in workspace: ${errorPath}`); | ||
| logger.warn(`Error content preview: ${content.substring(0, 500)}...`); | ||
| } | ||
| : undefined, | ||
| checkInterval: 10000, // Check every 10 seconds | ||
| checkInterval: 10000, | ||
| }); | ||
| await errorCollector.start(); | ||
| if (isDebugMode) { | ||
| logger.info(`🔍 Error collector started, errors will be logged to: ${errorLogPath}`); | ||
| } | ||
|
|
||
| await sflow( | ||
| [""], // Initial Prompt to start the agent, could be empty | ||
| ) | ||
| .merge( | ||
| // append messages from taskInputFlow | ||
| sflow(taskInputFlow.readable) | ||
| // send original message and then write '\n' after 1s delay to simulate user press Enter | ||
| .map(async (awaitableText) => awaitableText), | ||
| ) | ||
| .by(fromStdio(sh)) | ||
| // convert buffer to string and write to log file | ||
| .map(async (buffer) => { | ||
| if (buffer === undefined || buffer === null) { | ||
| logger.warn(`Received undefined/null buffer from process ${workspaceId}`); | ||
| return ""; | ||
| // --- Claude Agent SDK --- | ||
| logger.info( | ||
| `Spawning agent via SDK in ${botWorkingDir} with env GH_TOKEN_COMFY_PR_BOT=[REDACTED]`, | ||
| ); | ||
|
|
||
| const sdkPrompt = | ||
| "Please read PROMPT.txt and TODO.md in the current directory and complete all tasks listed there."; | ||
|
|
There was a problem hiding this comment.
PR description focuses on adding the slack-cli subtree + subtree sync scripts, but this PR also makes substantial functional changes to the Slack bot (switching from spawning claude-yes to @anthropic-ai/claude-agent-sdk, changing the inbox file name, and altering update behavior). Consider updating the PR description (or splitting into separate PRs) so reviewers can assess the bot/runtime impact independently from the subtree vendor-in.
There was a problem hiding this comment.
Updated PR description to cover all three areas of change: subtree vendor-in, sync tooling, and Slack bot SDK migration.
…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>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Changes
1. Git subtree: slack-cli
snomiao/slack-clias git subtree atlib/slack-cli/LazyLock)markdown→section/mrkdwn)2. Subtree sync tooling
scripts/subtree-sync.shfor pulling/pushing all configured subtreessubtree:pullandsubtree:pushnpm scripts3. Slack bot: migrate to Claude Agent SDK
claude-yesprocess spawning with@anthropic-ai/claude-agent-sdk(query()API)sendSlackUpdate()with 10s throttle to avoid Slack rate limits and LLM costmsgs.yaml→inbox.yaml