[bassilkhilo-ag2]

Summary

Two new skills:

1. skills/audio-transcribe/ — Gemini 2.5-flash voice transcription for all bridges

• Slack, Discord, and Telegram bridges all call _transcribe_via_skill() after downloading audio
• Voice notes → [Voice transcript: <text>] in task bodies instead of bare file paths
• Fail-open: any failure falls back to [File attached:] unchanged; skill is optional

2. skills/task-progress/ — Mid-task progress updates to Slack, Discord, Telegram

• Agent calls scripts/notify.py at task-start (for >2min tasks) and at checkpoints
• Sends directly to the originating channel using the bridge bot token
• Fail-open: a failed send never blocks the task

Test plan

• python3 tests/audio-transcribe-skill.test.py — 16 tests, all passing
• python3 tests/task-progress-skill.test.py — 14 tests, all passing
• Live test: real Slack .m4a voice note transcribed correctly
• Live test: task-progress notify.py sent a Slack DM successfully (exit 0)
• Manual: send voice note on Discord + Telegram — confirm [Voice transcript:]
• Manual: test Discord + Telegram notify paths

🤖 Generated with Claude Code

[sonichi]

LGTM. Well-designed addition.

Highlights:

• Fail-open throughout: skill absent → None → file-path fallback. subprocess exception → None. Non-zero exit → None. Chain is complete.
• 25s timeout is right for audio transcription — not too short, not unbounded.
• Empty transcript guard (result.stdout.strip() or None) prevents empty [Voice transcript:] lines.
• Test coverage matches the failure modes: absent skill, zero exit, nonzero exit, subprocess exception — all three bridges.
• Coupled correctly to main repo (modifies src/ bridges).

One note for consideration (non-blocking): skill resolution uses Path(file).parent.parent which finds the skill only when installed in the same repo tree. Users who install audio-transcribe to ~/.claude/skills/ separately won't get auto-transcription unless bridges also check that path. Current fallback to [File attached:] is correct behavior — just noting it for future if standalone installs become common.

[sonichi]

Reviewed the added task-progress skill (force-pushed since my last review). Looks good:

• Standalone (stdlib urllib only, no src/ imports) — correct design for an agent-callable skill
• Fail-open: failed sends print to stderr, exit 1, but SKILL.md explicitly instructs callers to continue working regardless
• Thread support for Slack is the right UX for @mention replies
• _token() resolution order (env → channel .env file) matches the audio-transcribe pattern

Note: this skill is agent-invoked (not wired into bridges), so the bridge diff in this PR is still just audio-transcribe. That's correct separation.

Combined LGTM on both skills — prior approval stands.

[sonichi]

Merge-readiness: MERGEABLE / UNSTABLE — mergeable=MERGEABLE, mergeStateStatus=UNSTABLE. The two posted checks (recheck, license/cla) are both SUCCESS, so UNSTABLE here is just GitHub still waiting on the auto-merge-state to settle after CLA recheck. Effectively green.

Verdict: ship-shaped. One async/event-loop concern worth a quick follow-up.

---

1. ⚠️ Event-loop blocking in discord-bridge.py (worth a follow-up commit or issue)

_handle_discord_message is async def (src/discord-bridge.py:2152), and the new _transcribe_via_skill(...) call at line ~2521 runs subprocess.run(..., timeout=25) synchronously. Up to 25 s of frozen event loop per voice note — the bridge can't process other messages (or send acks, or respond to mentions) during that window.

Fix is one line — wrap the subprocess call in await asyncio.get_event_loop().run_in_executor(None, ...) or use asyncio.create_subprocess_exec. Slack and Telegram bridges are sync-polled so they're unaffected.

2. 🪶 Transcript length / message budget

No truncation. A 5-min voice note could become a multi-KB transcript embedded directly in the task body, and downstream relays (Discord 2000-char limit, Telegram 4096) may reject the eventual reply if the agent quotes too much back. Cheap follow-up: cap to ~2k chars + a trailing [transcript truncated; full file: <path>].

3. 🪶 Discord helper called for every attachment, audio or not

The bridge invokes _transcribe_via_skill for every attached file before falling back to [File attached:]. For non-audio (images, PDFs, video), the skill returns None cheaply from its MIME filter — but it's still a python subprocess fork per attachment. Gate on extension at the call site to skip the fork for known-non-audio types.

---

Approve in principle — design is right (fail-open, optional skill, three-bridge surface, real tests with 16+14 cases). The async issue is the only real risk; the other two are sandbox-quality tightening.

Sonichi has already approved (15:27Z); flagging the async detail here so it's captured before merge — happy to open a follow-up PR if useful.

— Echo Act IV (Mini)

[bassilkhilo-ag2]

@sonichi Made some changes. Let me know if it lgty. Thanks!

[sonichi]

LGTM — clean pair of skills, good architecture.

audio-transcribe:

• Fail-open design is correct: non-zero exit → caller passes file path through unchanged, never blocks task delivery.
• Key resolution order is sensible (env → workspace .env → bridge .envs). No secrets logged.
• 20s timeout is a good bound — short voice notes transcribe in 2-5s, long ones stay bounded.
• urllib.request (no third-party dep) is the right call for a skill that must be portable.

task-progress:

• Correct pattern: notify fires first, before any tool calls. The CLAUDE.md addition codifies the voice-message sequence explicitly.
• Fail-open on send: task never blocks on a failed notification.

Bridge integration:

• _transcribe_via_skill() helper is identical across all three bridges — copy-paste is intentional here since each bridge is a standalone module.
• Falls back gracefully when skill directory is absent.

Minor observations (not blockers):

• The CI check shows "recheck" not "tsc + tests" — this PR predates the recent CI matrix changes and only has the CLA check plus a recheck gate. If the test suite was added to CI after this PR opened, it won't be run here. Worth noting for Chi to verify tests pass locally before merge.
• CLAUDE.md change adds notify instructions but doesn't update the "Built-in tools" section pointer — fine as-is since it's a new section, not a replacement.

Ready to merge pending Chi's sign-off.

[liususan091219]

Thanks @bassilkhilo-ag2 — this is a clean, well-scoped contribution. Two genuinely useful skills (voice-note transcription + mid-task progress notifications), both built fail-open and both removable without touching core. The skills architecture is respected: delete either skills/ dir and all three bridges degrade gracefully to their pre-PR behavior. I'm comfortable with the design; the notes below are non-blocking polish, and the one thing I'd most like to see is the Discord/Telegram manual checkboxes ticked before merge.

Things I like

1. skills/audio-transcribe/SKILL.md is exemplary. It documents the fail-open contract, the full key-resolution order, supported formats, and a "Removing the skill" section that states the fallback behavior explicitly (SKILL.md:92-95). That's the optionality-as-contract documentation this repo's architecture rules ask for — core boots without it, and the doc proves it.
2. Correct API-key choice for STT. transcribe.py:136-159 resolves GEMINI_API_KEY/GOOGLE_API_KEY (the paid key), not the free voice key. Exactly right: the free GEMINI_VOICE_API_KEY chain 429-exhausts under per-utterance STT load — a lesson we already paid for in the #1456 per-speaker STT work.
3. Uniform fail-open at both layers. The skill exits 1 with no stdout on every failure path (transcribe.py:209-217), and each bridge helper swallows non-zero exits + subprocess exceptions and returns None (src/slack-bridge.py:530-550, discord-bridge.py:484-503, telegram-bridge.py:577-596). The [File attached:] line always survives, so a transcription failure never loses the user's file.
4. Bounded latency. 20s urlopen timeout in the skill (transcribe.py:200) wrapped by a 25s subprocess timeout in the bridge — inner cap fires first, outer is a backstop. Both documented inline. No unbounded network call in the message-handling path.
5. task-progress/notify.py token resolution is clean (notify.py:375-381): env var first, then per-channel .env. The _post helper correctly distinguishes Slack's {"ok": ...} envelope from Discord/Telegram's message-object response.
6. Real test coverage — 16 + 14 tests, including subprocess CLI exit codes and the bridge-helper absent/success/failure matrix. Extracting just the helper to avoid importing the bridge's heavy deps is a nice touch.

Non-blocking observations

1. Skill-path resolution doesn't .resolve(). The helpers use Path(__file__).parent.parent / "skills" / ... (e.g. slack-bridge.py:538), but the existing discord-voice path-insert uses Path(__file__).resolve().parent.parent (discord-bridge.py:73). CLAUDE.md's workspace contract explicitly warns about bridges invoked via an app-bundle src/ symlink. Without .resolve(), a symlinked invocation could miss the skill dir and silently fall back to [File attached:]. The SKILL.md even claims the helper locates the script "relative to src/ (or the app bundle)" (audio-transcribe/SKILL.md:86-87) — but the code has no bundle special-case. Either add .resolve() (cheap, matches the existing pattern) or trim the SKILL.md claim. This is the one bug-shaped nit.
2. Three identical copies of _transcribe_via_skill. ~18 lines pasted into all three bridges. Defensible (keeps each bridge standalone, no shared-import coupling), but if it drifts you'll fix bugs three times — and observation #1 is exactly that kind of bug. Just flagging for the next person who touches it.
3. notify.py has no dedup — two calls for the same checkpoint = two messages. Fine given the "best-effort, when in doubt notify" guidance, but it's at-least-N-by-design.
4. Workspace contract — where the audio lands. Discord still writes downloaded audio to a hardcoded /tmp/discord-inbox (discord-bridge.py:398) while Telegram now resolves REPO = resolve_workspace(). Predates this PR and isn't yours to fix — noting the inbox dirs aren't uniformly under $SUTANDO_WORKSPACE per the 3-space contract.
5. Coupled vs standalone — destination correct. These skills modify src/*-bridge.py + CLAUDE.md, so they're coupled and rightly target sonichi/sutando, per the skill-PR routing rule.
6. CLAUDE.md prose addition is large. The ~32-line "Task progress notifications" block duplicates much of task-progress/SKILL.md. Per-session context budget is a real cost here (it's why built-in-tools.md was pulled out of CLAUDE.md). Consider trimming the CLAUDE.md block to trigger + pointer, letting the SKILL.md hold the detail.

Cross-PR linkage

• Per-speaker STT (#1456 / PR #1457) is the live-voice transcription path (paid key, sole-speaker gate). This PR is the orthogonal attachment path — voice notes dropped into Slack/Discord/Telegram, not live VC audio. No code overlap, and the shared lesson (use the paid key) is honored. Worth a one-line PR-body note so reviewers don't think it duplicates #1456.
• No conflict with the existing [File attached:]/[Voice note attached:] task-body convention — you extend it with [Voice transcript:] and keep the old line as fallback, so downstream parsers keying off [File attached: still work.

Tests

30 tests, all green per the body, covering the meaningful branches (MIME filter, missing key, API error, CLI exit codes, bridge absent/success/failure, Slack ok:false, thread_ts). The gaps are the two unchecked manual boxes: Discord + Telegram voice-note → [Voice transcript:] and the Discord/Telegram notify paths. Slack is live-verified; the other two share identical code so risk is low, but please tick those before merge — REST/sendMessage auth header shapes (Bot {token} vs bare) are exactly where a copy-paste skill quietly fails.

Ready to merge?

Design-wise yes — scoped, optional, fail-open, correct key choice, real tests. I'd merge once (a) the Discord + Telegram manual checkboxes are ticked, and (b) you either add .resolve() to the three helper paths or drop the "or the app bundle" claim from the SKILL.md. Both are 5-minute changes. Everything else is optional polish.

— Lucy (Mac Studio bot)

[john-the-dev]

Cold review — LGTM, blocked on #1467 merge.

audio-transcribe skill: clean design. _api_key() cascade (env → workspace .env → channel .envs) is thorough and correct — it picks up the key from wherever the operator configured it. Fail-open (non-zero exit → caller falls back to [File attached:]) is the right default; transcription latency should never block task delivery. from __future__ import annotations is present — 3.9-safe.

task-progress skill: notify.py CLI is simple and right. Token resolution via _token(source, var) follows the same per-channel .env pattern the bridges use, so no new credential locations to configure. _post() correctly distinguishes Slack's {"ok": ...} shape from the Discord/Telegram message-object response. Fail-open here too — exit 1 on send failure, caller continues.

Bridge integration: _transcribe_via_skill() is identical across all three bridges (discord, slack, telegram) — small DRY nit but acceptable since each bridge is otherwise standalone. The 25s timeout on the Gemini subprocess call is reasonable.

One dependency note: build log shows #1467 (inject skill instructions into owner task files) is still OPEN. #1453 depends on that bridge injection mechanism being present. Hold for #1467 merge, then this is ready.

[john-the-dev]

LGTM. Two genuinely useful fail-open skills:

audio-transcribe: The chain is complete — skill absent → None → file-path fallback; non-zero exit → None; subprocess exception → None. The 25s timeout is right for audio transcription. Key resolution order (env → workspace .env → per-channel .env files) is idiomatic. Removing skills/audio-transcribe/ returns all bridges to status quo.

task-progress: Fail-open pattern mirrors audio-transcribe. The CLAUDE.md instructions are clear about when to call (before work, not after) and include the correct per-source channel routing (Slack thread_ts, Telegram chat_id).

Tests cover the 16 audio-transcribe cases and bridge helpers for all 3 surfaces. The TestBridgeHelperDiscord(TestBridgeHelperSlack) inheritance for shared logic is clean.

One minor note: the CLAUDE.md update says to use the skill for tasks >60s — that's a good default threshold. Ship it.

[liususan091219]

Pushed a maintainer commit fixing the #1525-class hardcoded paths in this PR's new code (same fix as #1588 applies to the merged #1467 sites):

• notify.py / transcribe.py: channel .env paths now resolve via CLAUDE_CONFIG_DIR (fallback ~/.claude)
• Both SKILL.md files + this PR's CLAUDE.md section: ~/.claude/… → $CLAUDE_CONFIG_DIR/…

Both skill test files pass unmodified. Note: the branch is BEHIND main and a handful of main-side test fixes aren't absorbed yet — update-branch before merge will pick those up.

[liususan091219]

@cla-assistant check

[github-actions]

@cla-assistant check

[liususan091219]

@cla-assistant check