Skip to content

feat: verb-based maestro-discord CLI + 5 new slash commands#26

Merged
chr1syy merged 6 commits into
mainfrom
feat/cli
May 5, 2026
Merged

feat: verb-based maestro-discord CLI + 5 new slash commands#26
chr1syy merged 6 commits into
mainfrom
feat/cli

Conversation

@chr1syy
Copy link
Copy Markdown
Collaborator

@chr1syy chr1syy commented May 3, 2026

Summary

Two related pieces of work that surface previously-unused maestro-cli capabilities to the bot and to agents:

  1. maestro-discord CLI — restructured into verb-based commands (send, notify, status) so the agent→Discord surface can grow without flag-explosion.
  2. Five new slash commands/playbook (list/show/run), /agents show, /gist, /notes (synopsis/history), /auto-run start.

What's in the box

CLI (bin/maestro-discord)

  • send — same behavior as the old top-level form, now under a verb
  • notify toast|flash — formats a styled message with a color-coded emoji prefix and posts to the agent's channel
  • status — pulls the agent's stats from maestro-cli show agent --json and posts a status card

All three ride the existing POST /api/send endpoint; no server changes.

Slash commands

Command Notes
/playbook list [agent] Embed of all playbooks; agent autocomplete
/playbook show <id> Description, doc list with task progress
/playbook run <id> Public reply; blocks until complete event; posts cost + elapsed
/agents show <id> Stats, token totals, last 5 history entries
/gist [description] [public] Publishes current channel's agent transcript as a gist
/notes synopsis [days] AI-generated synopsis (2 min LLM timeout)
/notes history [days/limit/filter] Unified history feed across agents
/auto-run start <doc> Bare filenames resolve against agent's autoRunFolderPath; .md autocomplete

Service & wiring

  • src/services/maestro.ts gains showAgent, createGist, directorSynopsis, directorHistory, startAutoRun plus their types.
  • src/index.ts introduces a CommandModule type so the registry Map unifies across SlashCommandBuilder, SlashCommandSubcommandsOnlyBuilder, and SlashCommandOptionsOnlyBuilder.
  • src/deploy-commands.ts registers the four new commands.
  • README.md slash-command table updated.

Verification

  • tsc --noEmit clean
  • eslint src clean (only one pre-existing warning in transcription.ts, untouched)
  • npm test — 110/110 pass
  • npm run build — all new commands emit to dist/commands/

Test plan

  • Run npm run deploy-commands after merge to register the new slash commands with Discord
  • /playbook list shows playbooks; agent filter narrows the list
  • /playbook show <id> renders an embed with description and doc list
  • /playbook run <id> posts a public completion summary with elapsed/cost
  • /agents show <id> renders stats and recent activity
  • /gist in an agent channel returns a clickable gist URL; public:true makes it public
  • /notes synopsis returns an AI summary (allow up to ~2 min)
  • /notes history returns recent entries; filters narrow them
  • /auto-run start autocomplete lists .md files from the agent's Auto Run folder; selecting one launches the run
  • maestro-discord send --agent <id> --message "hi" still posts to Discord (renamed surface)
  • maestro-discord notify toast --agent <id> --title T --message M --color red posts a 🔴 prefixed embed
  • maestro-discord status --agent <id> posts a status card

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • /agents show — view detailed agent info with stats and recent activity; improved channel creation handling with friendly errors
    • /playbook — list, show, and run playbooks (clamped/truncated embed content)
    • /auto-run start — launch Auto Run documents with path resolution and autocomplete
    • /gist — publish session transcripts as GitHub gists
    • /notes — synopsis and history commands
    • Maestro-Discord CLI verbs: send, notify, status (new verb-based CLI)
  • Documentation

    • README and docs updated with expanded slash-commands and CLI usage examples

chr1syy and others added 2 commits May 3, 2026 13:06
Split the agent->Discord CLI into `send`, `notify`, and `status` verbs so the
surface can grow beyond the original single-purpose flag form. Verbs use
Node's built-in parseArgs and live in src/cli/verbs/, with shared HTTP and
exec helpers in src/cli/lib.ts. All three currently ride the existing
/api/send endpoint; no server changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface previously-unused maestro-cli capabilities as Discord slash commands:

- /playbook list|show|run — list playbooks, show details, run with completion
  summary posted to the channel (service layer was already wired)
- /agents show <id> — embed of agent stats and recent activity
- /gist [description] [public] — publish the channel's agent transcript as
  a GitHub gist; returns the URL in-channel
- /notes synopsis|history — Director's Notes (AI synopsis with 2 min LLM
  timeout, or unified history feed across agents)
- /auto-run start <doc> — launch an Auto Run for the channel's agent;
  bare filenames are resolved against the agent's autoRunFolderPath and
  autocomplete reads .md files from that directory

Adds five service methods (showAgent, createGist, directorSynopsis,
directorHistory, startAutoRun) and supporting types. Introduces a
CommandModule type in index.ts so the command registry Map type-unifies
across builders with different shapes (subcommand-only vs options-only).
Updates the README slash-command table.

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

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c4531c27-a514-4ea2-8a04-dd78d7e892ed

📥 Commits

Reviewing files that changed from the base of the PR and between 4304946 and 5a22613.

📒 Files selected for processing (6)
  • docs/api.md
  • src/__tests__/auto-run-command.test.ts
  • src/__tests__/cli-lib.test.ts
  • src/__tests__/gist-command.test.ts
  • src/commands/agents.ts
  • src/commands/auto-run.ts
✅ Files skipped from review due to trivial changes (3)
  • docs/api.md
  • src/tests/cli-lib.test.ts
  • src/tests/gist-command.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/commands/auto-run.ts
  • src/commands/agents.ts

📝 Walkthrough

Walkthrough

Adds multiple slash commands (/playbook, /gist, /notes, /auto-run) and an /agents show subcommand; implements CLI verb framework and helpers; introduces embed clamping utilities and expanded maestro service methods; wires new commands into the bot and deployment; and adds comprehensive tests and README/docs updates.

Changes

Commands, CLI, Services, and Embed Utilities

Layer / File(s) Summary
Data Shape / Types
src/utils/embed.ts, src/services/maestro.ts
Adds embed limit constants and clamp helpers; adds MaestroAgentDetail, GistResult, DirectorNotesEntry, DirectorSynopsis, and AutoRunOptions.
Core Implementation
src/utils/embed.ts, src/services/maestro.ts, src/cli/lib.ts
Implements clampText + wrappers; adds maestro methods (showAgent, createGist, directorSynopsis, directorHistory, startAutoRun); adds CLI helpers (postToSendApi, parsePort, runMaestroCli, fail, ok) and related types/constants.
Wiring / Command Logic
src/commands/playbook.ts, src/commands/gist.ts, src/commands/notes.ts, src/commands/auto-run.ts, src/commands/agents.ts
Adds /playbook (list/show/run), /gist, /notes (synopsis/history), /auto-run start, and /agents show; each uses maestro APIs and clamp helpers, includes autocomplete where applicable, and surfaces friendly error replies. Adds sendability check after channel creation in agents new.
CLI Entrypoint & Verbs
src/cli/maestro-discord.ts, src/cli/verbs/send.ts, src/cli/verbs/notify.ts, src/cli/verbs/status.ts
Replaces single-purpose script with verb-based dispatcher and adds send, notify, and status verb implementations using src/cli/lib.ts.
Bot Wiring / Deployment
src/index.ts, src/deploy-commands.ts
Registers new command modules (playbook, gist, notes, autoRun) in the commands map and includes them in the deployed commands list.
Tests
src/__tests__/* (agents-command.test.ts, embed.test.ts, playbook-command.test.ts, auto-run-command.test.ts, cli-lib.test.ts, gist-command.test.ts)
Adds/extends tests for embed clamping, agents rendering and sendability, playbook list/show/run clamping and errors, auto-run path resolution and errors, CLI helpers (parsePort, postToSendApi), and gist publishing error cases and truncation.
Documentation
README.md, docs/api.md
Updates Slash commands table and CLI docs to document new commands and verb-based maestro-discord usage, including examples for send, notify, and status.

Sequence Diagram

sequenceDiagram
    participant User as Discord User
    participant Bot as Discord Bot
    participant Cmd as Command Handler
    participant Maestro as Maestro Service
    participant CLI as maestro-cli / HTTP API

    User->>Bot: /agents show agent:foo
    Bot->>Cmd: ChatInputCommandInteraction
    Cmd->>Maestro: showAgent('foo')
    Maestro->>CLI: maestro-cli show agent foo --json
    CLI-->>Maestro: { id, cwd, stats, recentActivity }
    Maestro-->>Cmd: MaestroAgentDetail
    Cmd->>Cmd: Build embed with clamped fields
    Cmd-->>User: Embed (Stats + Recent activity)

    User->>Bot: /playbook run playbook:pb1
    Bot->>Cmd: ChatInputCommandInteraction
    Cmd->>Maestro: runPlaybook('pb1')
    Maestro->>CLI: maestro-cli playbook run pb1
    CLI-->>Maestro: Running / completion result
    Maestro-->>Cmd: result { completed, elapsed, cost }
    Cmd->>Cmd: Format status with metrics
    Cmd-->>User: Embed (completion summary)

    User->>CLI: maestro-discord status --agent foo
    CLI->>CLI: runMaestroCli(['show','agent','foo','--json'])
    CLI->>Bot: postToSendApi({agentId, message})
    Bot-->>User: Status message (via send API)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I trimmed the fields, I stitched the flow,
New commands hop out where playbooks grow.
Gists and notes and auto-runs on cue,
Maestro and CLI — a carrot rendezvous.
Hop, click, deploy — celebratory chew!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: a verb-based CLI restructuring (maestro-discord) and five new slash commands (/playbook, /agents show, /gist, /notes, /auto-run).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cli

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…nts show

Discord rejects embed payloads with description > 4096 chars or field value
> 1024 chars. Long playbook descriptions, long document path lists, long
cwds, and accumulated recent-activity text could all trip these limits and
fail editReply with a validation error.

Adds src/utils/embed.ts with clampDescription and clampFieldValue helpers
(append a "\n…" marker when truncating) and applies them to:

- /playbook show — description and Documents field
- /agents show — Cwd, Stats, and Recent activity fields

Also adds focused tests:

- embed.test.ts — clamp limits and edge cases
- playbook-command.test.ts — list/show happy paths, oversize-input clamp,
  load-failure error path
- agents-command.test.ts — show happy path, oversize-cwd clamp, load-failure
  error path

Test count: 110 -> 122 (all green).

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/commands/agents.ts (1)

190-196: ⚠️ Potential issue | 🟠 Major

Replace the unsafe type cast with isSendable() type guard and bound the channel name.

The as TextChannel cast bypasses type safety; instead, check isSendable() to narrow the type. Additionally, the channel name is unbounded and can exceed Discord's 100-character limit for channel names.

Suggested fix
  const channelName = `agent-${agent.name.toLowerCase().replace(/[^a-z0-9-]/g, '-')}`;
- const channel = (await guild.channels.create({
+ const newChannel = await guild.channels.create({
    name: channelName,
    type: ChannelType.GuildText,
    parent: category.id,
    topic: `Maestro agent: ${agent.name} (${agent.id}) | ${agent.toolType} | ${agent.cwd}`,
- })) as TextChannel;
+ });
+ if (!newChannel.isSendable()) {
+   throw new Error('Failed to create sendable channel');
+ }
+ const channel = newChannel as TextChannel;

Or more concisely with the name bounded:

- const channelName = `agent-${agent.name.toLowerCase().replace(/[^a-z0-9-]/g, '-')}`;
+ const channelName = `agent-${agent.name.toLowerCase().replace(/[^a-z0-9-]/g, '-')}`.slice(0, 100);
  const channel = (await guild.channels.create({
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/agents.ts` around lines 190 - 196, The current cast "(...) as
TextChannel" is unsafe and the generated channelName can exceed Discord's
100-char limit; instead, truncate/bound channelName (e.g., to 100 chars) before
creating the channel and after creation use the isSendable() type guard to
narrow the returned channel to a TextChannel (handle the non-sendable case by
throwing or logging). Update the code around channelName, guild.channels.create
and the variable channel to apply the length bound on channelName and replace
the explicit "as TextChannel" cast with an isSendable() check to safely access
TextChannel methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/lib.ts`:
- Around line 21-60: postToSendApi currently can hang if the connection is
accepted but the server never responds; add a request timeout (e.g. 3–5s) on the
http.ClientRequest (req) to fail fast: call req.setTimeout(timeoutMs, ...) and
in the timeout handler reject the Promise with a clear timeout Error (e.g.
'Request to bot timed out') and destroy/abort the request (req.abort() or
req.destroy()) to ensure the socket is closed; keep existing error and response
handlers but ensure the timeout handler cannot also cause a second
resolve/reject by only rejecting once (use a flag or cleanup listeners) so
postToSendApi fails quickly on stalled servers.

In `@src/cli/verbs/notify.ts`:
- Around line 79-80: The port parsing currently uses parseInt on
parsed.values.port which allows inputs like "123abc" to be coerced; change the
logic in the port handling so that if parsed.values.port is present it is
strictly validated as an unsigned integer string (e.g. /^\d+$/) before
converting — if the string fails validation call fail('--port must be a
number'); otherwise safely parse with parseInt(parsed.values.port, 10) (fall
back to DEFAULT_PORT when parsed.values.port is absent). Reference
parsed.values.port, DEFAULT_PORT and the fail(...) call when updating the code.

In `@src/cli/verbs/send.ts`:
- Around line 47-48: parsed.values.port currently uses parseInt which accepts
values like "123abc"; change to strict validation: verify parsed.values.port
matches only digits (e.g. /^\d+$/) or convert via Number and ensure
Number.isInteger and not NaN, then check the port is within the allowed range
(1–65535) before assigning to port; if validation fails call fail('--port must
be a valid integer between 1 and 65535') and otherwise set port (default to
DEFAULT_PORT when no value provided). Reference parsed.values.port, DEFAULT_PORT
and fail in your change.

In `@src/cli/verbs/status.ts`:
- Around line 89-94: The current try/catch around runMaestroCli + JSON.parse
conflates execution errors and parse errors; split them so runMaestroCli errors
are handled separately from JSON parsing errors. Call runMaestroCli(['show',
'agent', agentId, '--json']) inside its own try/catch and fail with a message
like "maestro-cli show agent failed" using runMaestroCli error details, then
parse the returned raw string inside a second try/catch that fails with a clear
parse message (e.g., "failed to parse maestro-cli JSON output") and include the
parse Error; reference the runMaestroCli call and the AgentDetail assignment for
locating the changes.

In `@src/commands/agents.ts`:
- Around line 225-236: The embed is using raw detail.name for setTitle and raw
detail.groupName for addFields which can overflow/throw; before calling
EmbedBuilder.setTitle and embed.addFields for Group, run those strings through
the same clamp function (clampFieldValue) or a short-title clamp and use the
clamped values (e.g., clampedName = clampFieldValue(detail.name) then
setTitle(clampedName), and clampedGroup = clampFieldValue(detail.groupName)
before embed.addFields) so both the title and group field are safely
length-limited while keeping existing cwd/stats clamping logic intact.

In `@src/commands/auto-run.ts`:
- Around line 105-110: The current logic only rewrites bare filenames; change
the condition so any non-absolute path is resolved against the agent Auto Run
folder. In the block that sets docPath (variables/functions: docPath, doc,
getAgentFolder, channelInfo.agent_id, path.isAbsolute), replace the check
(currently excluding strings containing '/') with simply !path.isAbsolute(doc),
then if getAgentFolder(...) returns a folder assign docPath = path.join(folder,
doc) so relative paths like "subdir/doc.md" are resolved against the agent
folder.
- Around line 44-53: getAgentFolder currently relies on maestro.listAgents()
which may not include autoRunFolderPath and therefore returns null; change it to
fetch the detailed agent record for the specific agentId (e.g., call
maestro.getAgent(agentId) or maestro.getAgentDetails(agentId) instead of
maestro.listAgents()), then read (agent as { autoRunFolderPath?: unknown
}).autoRunFolderPath and return it if it's a string, otherwise return null; keep
the try/catch behavior to return null on errors and reference the existing
getAgentFolder, maestro.listAgents, agent, and autoRunFolderPath symbols when
making the change.

In `@src/commands/gist.ts`:
- Around line 40-43: The catch block after calling maestro.createGist() uses
(err as Error).message.slice(...) which will throw if err is not an Error;
normalize the caught value first (e.g., convert to a safe string via String(err)
or extract message = err?.message ?? String(err)) before slicing, then pass that
safe truncated string to interaction.editReply so building the reply cannot
itself throw; update the catch block around maestro.createGist() /
interaction.editReply to compute a safeMessage variable and use that.

In `@src/commands/playbook.ts`:
- Around line 157-175: Clamp the embed title and the Agent field value before
adding them to the EmbedBuilder to avoid Discord limits: when building the embed
(EmbedBuilder usage that calls setTitle(detail.name)) pass a truncated/validated
version of detail.name limited to 256 chars, and when adding the Agent field
(the embed.addFields call that uses detail.agentName) pass a truncated value
limited to 1024 chars (or reuse the existing clampFieldValue helper for the
agent field and clampDescription or a new clampTitle helper for the title).
Ensure you still fall back to a safe default string if detail.name or
detail.agentName are missing or empty.

---

Outside diff comments:
In `@src/commands/agents.ts`:
- Around line 190-196: The current cast "(...) as TextChannel" is unsafe and the
generated channelName can exceed Discord's 100-char limit; instead,
truncate/bound channelName (e.g., to 100 chars) before creating the channel and
after creation use the isSendable() type guard to narrow the returned channel to
a TextChannel (handle the non-sendable case by throwing or logging). Update the
code around channelName, guild.channels.create and the variable channel to apply
the length bound on channelName and replace the explicit "as TextChannel" cast
with an isSendable() check to safely access TextChannel methods.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3d883c71-e1c1-426c-9e90-803d0b94ec18

📥 Commits

Reviewing files that changed from the base of the PR and between efbfbdb and 7fe2196.

📒 Files selected for processing (18)
  • README.md
  • src/__tests__/agents-command.test.ts
  • src/__tests__/embed.test.ts
  • src/__tests__/playbook-command.test.ts
  • src/cli/lib.ts
  • src/cli/maestro-discord.ts
  • src/cli/verbs/notify.ts
  • src/cli/verbs/send.ts
  • src/cli/verbs/status.ts
  • src/commands/agents.ts
  • src/commands/auto-run.ts
  • src/commands/gist.ts
  • src/commands/notes.ts
  • src/commands/playbook.ts
  • src/deploy-commands.ts
  • src/index.ts
  • src/services/maestro.ts
  • src/utils/embed.ts

Comment thread src/cli/lib.ts Outdated
Comment thread src/cli/verbs/notify.ts Outdated
Comment thread src/cli/verbs/send.ts Outdated
Comment thread src/cli/verbs/status.ts
Comment thread src/commands/agents.ts
Comment thread src/commands/auto-run.ts
Comment thread src/commands/auto-run.ts Outdated
Comment thread src/commands/gist.ts Outdated
Comment thread src/commands/playbook.ts
- cli/lib: add 5s timeout + single-settle guard to postToSendApi; new
  parsePort() helper that strictly validates --port (digits only, 1-65535)
- cli/verbs/{send,notify,status}: use parsePort()
- cli/verbs/status: split runMaestroCli execution from JSON.parse so each
  surfaces a distinct error
- utils/embed: add EMBED_TITLE_MAX (256) + clampTitle()
- commands/agents: clamp title and groupName in handleShow; bound channel
  name to Discord's 100-char limit; replace `as TextChannel` cast with
  isSendable() guard + friendly error reply
- commands/auto-run: fetch autoRunFolderPath via showAgent (listAgents
  doesn't return it); resolve any non-absolute doc path against the agent
  folder, not only bare filenames
- commands/gist: normalize non-Error rejection values before slicing
- commands/playbook: clamp title and agent name in handleShow

Tests:
- new cli-lib.test.ts: parsePort validation + postToSendApi timeout,
  ECONNREFUSED, invalid-JSON, and double-settle protection
- new auto-run-command.test.ts: bare filename, subdir path regression,
  absolute passthrough, showAgent failure, missing folder
- new gist-command.test.ts: success, Error throw, string throw, object
  throw, long-message truncation
- extend agents/playbook/embed tests with clamp + bound regressions

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/__tests__/playbook-command.test.ts (2)

70-99: ⚡ Quick win

show description/docs clamping test omits agentName from the mock response.

The mock at lines 72–83 does not include an agentName field, unlike both the list mock (line 45) and the title/agent clamping mock (line 111). If showPlaybook always returns an agentName in production, the mock is unrepresentative and the Agent embed field would silently render "undefined" without the test catching it. Consider adding a representative agentName value (e.g., agentName: 'TestAgent') to make this fixture self-consistent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/playbook-command.test.ts` around lines 70 - 99, The
showPlaybook mock in the test 'playbook show clamps oversize description and
document field' omits agentName causing the Agent embed to render "undefined";
update the mock returned by mock.method(maestro, 'showPlaybook') to include a
representative agentName (e.g., agentName: 'TestAgent') so the fixture matches
other mocks (like the list and title/agent clamping mocks) and the embed
rendering is exercised correctly.

36-148: ⚡ Quick win

run subcommand has zero test coverage.

The PR explicitly introduces a run subcommand that blocks until completion and posts cost + elapsed time — arguably the most complex path in this command. None of the five tests here exercise it, and the show/error tests cover only the other two subcommands. At minimum, a happy-path test (mock runPlaybook, assert the embed contains cost/elapsed) and a failure test (mock throws, assert friendly error string) would give meaningful regression protection for this logic.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__tests__/playbook-command.test.ts` around lines 36 - 148, Add two tests
exercising the new "run" subcommand: one happy-path that mocks
maestro.runPlaybook to return a result containing cost and elapsed (and any
playbook id/name), invokes execute with makeInteraction('run', { playbook:
'pb-1' }), and asserts the reply embed (from
i.editReply.mock.calls[0].arguments[0]) includes the cost and elapsed values;
and one failure test that mocks maestro.runPlaybook to throw, invokes execute
the same way, and asserts the reply is a friendly error string (contains "Could
not run playbook" or similar). Use the same test patterns as the existing tests
(importing maestro, mock.method, and inspecting i.editReply) and reuse the
EMBED_* constants and types already in the file for length/assertion checks
where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/__tests__/auto-run-command.test.ts`:
- Line 124: Add callCount assertions before accessing startMock.mock.calls[0] in
the tests that read opts: guard each access with the same pattern used in tests
2 and 3 by inserting assert.equal(startMock.mock.callCount(), 1) immediately
before any use of startMock.mock.calls[0].arguments[0] (these occur around the
blocks that reference startAutoRun and the const opts assignment), and apply the
same change for the other occurrences noted (the blocks at the two additional
locations mentioned).

In `@src/__tests__/cli-lib.test.ts`:
- Around line 85-91: The test using port 1 can be flaky on some platforms;
change the test for postToSendApi so it creates a short-lived TCP listener on
port 0 (let the OS assign a free port), read the assigned port, immediately
close the listener, then call postToSendApi with that assigned port and assert
it rejects with /not running|not started|ECONNREFUSED/i; reference the test name
("postToSendApi reports a friendly message on ECONNREFUSED") and the function
postToSendApi to locate and replace the current hardcoded port approach with the
bind-then-close pattern to reliably provoke ECONNREFUSED across platforms.

In `@src/__tests__/gist-command.test.ts`:
- Around line 154-156: The test currently only checks an upper bound on the
reply length; add a complementary lower-bound assertion to detect
over-truncation by asserting that reply (the value from
i.editReply.mock.calls[0].arguments[0]) is at least 1500 - 50 characters long
and also verify it starts with the expected prefix "❌ Could not publish gist: "
so both missing body and over-truncation regressions fail the test. Ensure you
use the same margin (±50) as the existing check and keep assertions using reply
and i.editReply.mock.calls[0].arguments[0] so the test targets the same mocked
output.

In `@src/commands/agents.ts`:
- Around line 200-207: The unsafe cast "as TextChannel" should be removed after
the isSendable() guard: replace "const channel = newChannel as TextChannel;"
with "const channel = newChannel;" so you keep the channel reference without
violating the repo's channel-safety rule; if TypeScript still complains, add the
more specific runtime guard newChannel.isTextBased() before assignment (or
narrow with a type predicate) to satisfy the compiler, referencing the existing
newChannel and its isSendable()/isTextBased() checks.

---

Nitpick comments:
In `@src/__tests__/playbook-command.test.ts`:
- Around line 70-99: The showPlaybook mock in the test 'playbook show clamps
oversize description and document field' omits agentName causing the Agent embed
to render "undefined"; update the mock returned by mock.method(maestro,
'showPlaybook') to include a representative agentName (e.g., agentName:
'TestAgent') so the fixture matches other mocks (like the list and title/agent
clamping mocks) and the embed rendering is exercised correctly.
- Around line 36-148: Add two tests exercising the new "run" subcommand: one
happy-path that mocks maestro.runPlaybook to return a result containing cost and
elapsed (and any playbook id/name), invokes execute with makeInteraction('run',
{ playbook: 'pb-1' }), and asserts the reply embed (from
i.editReply.mock.calls[0].arguments[0]) includes the cost and elapsed values;
and one failure test that mocks maestro.runPlaybook to throw, invokes execute
the same way, and asserts the reply is a friendly error string (contains "Could
not run playbook" or similar). Use the same test patterns as the existing tests
(importing maestro, mock.method, and inspecting i.editReply) and reuse the
EMBED_* constants and types already in the file for length/assertion checks
where applicable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 671c07f4-5f5c-4168-b96c-2a16be218673

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe2196 and 4304946.

📒 Files selected for processing (15)
  • src/__tests__/agents-command.test.ts
  • src/__tests__/auto-run-command.test.ts
  • src/__tests__/cli-lib.test.ts
  • src/__tests__/embed.test.ts
  • src/__tests__/gist-command.test.ts
  • src/__tests__/playbook-command.test.ts
  • src/cli/lib.ts
  • src/cli/verbs/notify.ts
  • src/cli/verbs/send.ts
  • src/cli/verbs/status.ts
  • src/commands/agents.ts
  • src/commands/auto-run.ts
  • src/commands/gist.ts
  • src/commands/playbook.ts
  • src/utils/embed.ts
✅ Files skipped from review due to trivial changes (1)
  • src/cli/verbs/send.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/tests/embed.test.ts
  • src/commands/auto-run.ts
  • src/utils/embed.ts
  • src/cli/lib.ts
  • src/commands/gist.ts
  • src/commands/playbook.ts
  • src/cli/verbs/notify.ts
  • src/cli/verbs/status.ts

Comment thread src/__tests__/auto-run-command.test.ts
Comment thread src/__tests__/cli-lib.test.ts
Comment thread src/__tests__/gist-command.test.ts
Comment thread src/commands/agents.ts
chr1syy and others added 2 commits May 5, 2026 08:31
- agents: drop the unsafe `as TextChannel` cast (and import) since
  isSendable() already narrows the channel type
- tests/auto-run: add callCount guards before reading mock.calls[0] in
  absolute-path / showAgent-fail / missing-folder tests so a regression
  produces a clean assertion failure rather than a TypeError
- tests/cli-lib: drop the port-1 ECONNREFUSED trick (unreliable on
  Windows). Bind a server to a random free port, close it immediately,
  then connect — the OS keeps the port reserved long enough to refuse
  predictably across platforms
- tests/gist: tighten truncation test with a lower-bound assertion and
  prefix check so over-truncation regressions don't slip through

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

- auto-run: extract resolveContainedDocPath() that resolves a doc input
  (filename, relative subpath, or absolute path) and returns the resolved
  path only when it lives strictly inside the agent's Auto Run folder.
  /auto-run start now rejects absolute paths pointing outside the folder
  and `..` traversal that escapes it, with a friendly error reply. Also
  rejects up front when the agent has no autoRunFolderPath (previously the
  doc fell back to running relative to the bot's cwd).
- auto-run autocomplete: walk subfolders so nested `subdir/plan.md` docs
  are discoverable; emit forward-slash values for cross-platform
  consistency.
- docs/api.md: rewrite the CLI usage section for the verb-based shape
  (`maestro-discord send | notify | status`); the old flat-flag examples
  no longer worked after the verb refactor.

Tests:
- new resolveContainedDocPath unit cases: bare filename, relative subpath,
  absolute inside, absolute outside, traversal escape, exact-folder match,
  sibling-prefix lookalike (/agents/auto vs /agents/auto-evil)
- /auto-run start integration: rejects /etc/passwd, rejects ../../etc/passwd,
  rejects when showAgent fails, rejects when autoRunFolderPath missing
- autocomplete integration: real tmpdir with nested .md files asserts that
  top.md, sub/nested.md, and sub/deeper/plan.md all surface and that
  README.txt does not, all with forward slashes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@chr1syy chr1syy merged commit a9cc725 into main May 5, 2026
3 checks passed
chr1syy added a commit that referenced this pull request May 7, 2026
…#32)

* feat(cli): restructure maestro-discord into verb-based commands

Split the agent->Discord CLI into `send`, `notify`, and `status` verbs so the
surface can grow beyond the original single-purpose flag form. Verbs use
Node's built-in parseArgs and live in src/cli/verbs/, with shared HTTP and
exec helpers in src/cli/lib.ts. All three currently ride the existing
/api/send endpoint; no server changes.

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

* feat(commands): add /playbook, /gist, /notes, /auto-run, /agents show

Surface previously-unused maestro-cli capabilities as Discord slash commands:

- /playbook list|show|run — list playbooks, show details, run with completion
  summary posted to the channel (service layer was already wired)
- /agents show <id> — embed of agent stats and recent activity
- /gist [description] [public] — publish the channel's agent transcript as
  a GitHub gist; returns the URL in-channel
- /notes synopsis|history — Director's Notes (AI synopsis with 2 min LLM
  timeout, or unified history feed across agents)
- /auto-run start <doc> — launch an Auto Run for the channel's agent;
  bare filenames are resolved against the agent's autoRunFolderPath and
  autocomplete reads .md files from that directory

Adds five service methods (showAgent, createGist, directorSynopsis,
directorHistory, startAutoRun) and supporting types. Introduces a
CommandModule type in index.ts so the command registry Map type-unifies
across builders with different shapes (subcommand-only vs options-only).
Updates the README slash-command table.

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

* fix(commands): clamp embed text to Discord limits in /playbook + /agents show

Discord rejects embed payloads with description > 4096 chars or field value
> 1024 chars. Long playbook descriptions, long document path lists, long
cwds, and accumulated recent-activity text could all trip these limits and
fail editReply with a validation error.

Adds src/utils/embed.ts with clampDescription and clampFieldValue helpers
(append a "\n…" marker when truncating) and applies them to:

- /playbook show — description and Documents field
- /agents show — Cwd, Stats, and Recent activity fields

Also adds focused tests:

- embed.test.ts — clamp limits and edge cases
- playbook-command.test.ts — list/show happy paths, oversize-input clamp,
  load-failure error path
- agents-command.test.ts — show happy path, oversize-cwd clamp, load-failure
  error path

Test count: 110 -> 122 (all green).

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

* fix(cli,commands): address PR #26 review feedback

- cli/lib: add 5s timeout + single-settle guard to postToSendApi; new
  parsePort() helper that strictly validates --port (digits only, 1-65535)
- cli/verbs/{send,notify,status}: use parsePort()
- cli/verbs/status: split runMaestroCli execution from JSON.parse so each
  surfaces a distinct error
- utils/embed: add EMBED_TITLE_MAX (256) + clampTitle()
- commands/agents: clamp title and groupName in handleShow; bound channel
  name to Discord's 100-char limit; replace `as TextChannel` cast with
  isSendable() guard + friendly error reply
- commands/auto-run: fetch autoRunFolderPath via showAgent (listAgents
  doesn't return it); resolve any non-absolute doc path against the agent
  folder, not only bare filenames
- commands/gist: normalize non-Error rejection values before slicing
- commands/playbook: clamp title and agent name in handleShow

Tests:
- new cli-lib.test.ts: parsePort validation + postToSendApi timeout,
  ECONNREFUSED, invalid-JSON, and double-settle protection
- new auto-run-command.test.ts: bare filename, subdir path regression,
  absolute passthrough, showAgent failure, missing folder
- new gist-command.test.ts: success, Error throw, string throw, object
  throw, long-message truncation
- extend agents/playbook/embed tests with clamp + bound regressions

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

* fix(cli,commands,tests): address PR #26 follow-up review

- agents: drop the unsafe `as TextChannel` cast (and import) since
  isSendable() already narrows the channel type
- tests/auto-run: add callCount guards before reading mock.calls[0] in
  absolute-path / showAgent-fail / missing-folder tests so a regression
  produces a clean assertion failure rather than a TypeError
- tests/cli-lib: drop the port-1 ECONNREFUSED trick (unreliable on
  Windows). Bind a server to a random free port, close it immediately,
  then connect — the OS keeps the port reserved long enough to refuse
  predictably across platforms
- tests/gist: tighten truncation test with a lower-bound assertion and
  prefix check so over-truncation regressions don't slip through

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

* refactor(core,providers): extract provider-agnostic kernel and Discord adapter

Restructure the codebase into src/core/ (provider-agnostic kernel) and
src/providers/discord/ (adapter implementing a new BridgeProvider
interface). All Discord-specific code now lives behind the adapter; the
kernel speaks only in IncomingMessage / ConversationRecord / ChannelTarget
types. A new provider can be added by dropping src/providers/<name>/
without touching the kernel.

Behavior preserved end-to-end:
- All 7 slash commands, voice transcription, and HTTP /api/send work
  identically from the user's perspective.
- All env vars unchanged (DISCORD_*, API_PORT, WHISPER_*); new optional
  ENABLED_PROVIDERS defaults to 'discord'.
- /api/send accepts an optional `provider` field (defaults to 'discord').
- DB schema upgrades automatically on first start: agent_channels gets
  a `provider` column and composite PK (provider, channel_id);
  agent_threads is renamed to discord_agent_threads with rows preserved.
- CLI binary maestro-discord unchanged.

Tests: 158/158 pass (151 baseline + 7 new). Includes a MockProvider
smoke test exercising the BridgeProvider interface end-to-end without
any Discord code, and a legacy-schema-upgrade migration test.

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

* chore(rename): discord-maestro -> maestro-bridge

Rename the project to reflect the new architecture: a provider-agnostic
bridge between chat platforms and Maestro agents, with Discord as the
first provider.

Local changes only — does NOT rename the GitHub repo or migrate the
installer in RunMaestro/Maestro-Discord; both require manual follow-up.

Renames:
- package name: discord-maestro -> maestro-bridge
- bin: maestro-bridge (primary), maestro-discord kept as alias
  pointing at the same dist/cli/maestro-bridge.js
- src/cli/maestro-discord.ts -> src/cli/maestro-bridge.ts
- npm script: maestro-bridge (alias maestro-discord retained)

User-facing strings in CLI help, README, AGENTS.md, docs/api.md, and
docs/architecture.md updated to "Maestro Bridge". All DISCORD_* env
vars unchanged. .env.example reorganised into Core / Discord sections
with ENABLED_PROVIDERS=discord (default) added.

Migration: existing scripts calling `maestro-discord ...` keep working
unchanged via the alias bin. Tests, typecheck, and lint all green.

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

* chore(installer): rename maestro-discord installer to maestro-bridge

Imports the installer plumbing that lives on main (install.sh,
maestro-discord-ctl, systemd/launchd templates) into this branch and
renames it for Maestro Bridge.

This branch is stacked on feat/rename-to-bridge, so it includes the
provider-agnostic kernel + the package rename + the installer rename
together. Land the upstream branches first.

Renames:
- bin/maestro-discord-ctl.sh -> bin/maestro-bridge-ctl.sh
- templates/maestro-discord.service -> templates/maestro-bridge.service
- templates/sh.maestro.discord.plist -> templates/sh.maestro.bridge.plist
- install dir default: ~/.local/share/maestro-bridge
- config dir default: ~/.config/maestro-bridge
- systemd unit: maestro-bridge.service
- launchd label: sh.maestro.bridge
- repo default: RunMaestro/Maestro-Bridge

Backwards compatibility:
- MAESTRO_DISCORD_* env vars accepted as fallback for every
  MAESTRO_BRIDGE_* var (so v0.0.x `maestro-discord-ctl update` works).
- Legacy ~/.local/share/maestro-discord and ~/.config/maestro-discord
  are auto-detected when the new dirs do not exist.
- install_ctl creates BOTH `maestro-bridge-ctl` and `maestro-discord-ctl`
  symlinks pointing at the same wrapper.
- install.sh removes a legacy maestro-discord systemd unit / launchd
  plist on upgrade so two services do not run side by side.
- uninstall cleans up both legacy and new service files + symlinks.

Bot path fixes:
- ctl deploy now runs `dist/providers/discord/deploy.js` (the post-
  refactor location) instead of the old `dist/deploy-commands.js`.

Note: a matching change to .github/workflows/release.yml (tarball
staging name maestro-discord-<tag> -> maestro-bridge-<tag>) is required
but excluded from this commit because the bot OAuth token lacks
workflow scope. Apply the diff in a follow-up commit pushed with a
PAT that has 'workflow' scope (the file is left unstaged in the
working tree for that purpose).

Manual follow-ups (not in this PR):
- Rename the GitHub repo RunMaestro/Maestro-Discord to
  RunMaestro/Maestro-Bridge (auto-redirects existing curl URLs).
- Push the .github/workflows/release.yml change with workflow scope.
- Cut a v0.0.5+ release once the rename PRs merge.

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

* chore(rename): transition bridge naming to maestro-relay with compat aliases (#30)

* feat: add module-switch scaffold + complete relay rename/doc cleanup (#31)

* chore(rename): transition bridge naming to maestro-relay with compat aliases

* feat(installer,cli): add module switch scaffold and complete relay rename

* docs(readme): keep npm link separate from relay ctl usage

* docs(readme): add one-line install and remove duplicate CLI section

* fix(release): rename release tarball to maestro-relay-${tag}.tar.gz

install.sh downloads maestro-relay-${tag}.tar.gz but the workflow was
still building maestro-discord-${tag}.tar.gz, so a fresh tag would 404
on download. Aligns the release artifact with the installer.

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

* fix(installer): strip surrounding quotes from ENABLED_PROVIDERS before matching

The sed extraction in deploy_commands (install.sh) and cmd_deploy
(maestro-relay-ctl) captured everything that wasn't whitespace or '#',
so a quoted value like ENABLED_PROVIDERS="discord" became "discord"
(quotes included). The subsequent case ",$ep," in *,discord,*) check
then failed against ,"discord", and the installer/ctl falsely reported
Discord as not enabled, skipping slash-command deployment.

Strip a single layer of leading/trailing single or double quotes after
extraction. Reported by Codex review of PR #32.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant