Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ logs/
Auto Run Docs/
models
.maestro/

# local PR-review worktree (not part of the project)
.pr52-review/
26 changes: 25 additions & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The kernel speaks only in `IncomingMessage` / `OutgoingMessage` / `ChannelTarget
- Resolves the conversation via `provider.resolveConversation` (returns `{agentId, sessionId, readOnly, persistSession}`)
- Downloads any attachments to the agent's `cwd`
- Calls `maestro.send(agentId, content, sessionId, readOnly)`
- Splits the response and calls `provider.send(target, {text})` for each part
- Normalizes markdown tables in the response (see **Output rendering** below), splits it, and calls `provider.send(target, {text})` for each part
- Posts a usage footer: tokens, cost, context %
- Persists the maestro session id on the first response via `conv.persistSession`
5. Errors are logged to `logs/errors.log` and surfaced as a `⚠️` reply in the channel.
Expand All @@ -42,6 +42,28 @@ Each thread is bound to the user who created it (via mention or `/session new`).
- Messages from other users are silently ignored — no error reply, no forwarding.
- This prevents cross-talk and keeps each conversation scoped to one user.

## Output rendering

Agent responses often contain GitHub-flavored markdown tables, which no chat
platform renders (Discord has no table syntax; Slack's *mrkdwn* has none) — they
arrive as raw `| a | b |` pipes. The kernel normalizes them in one
provider-agnostic place so every provider benefits:

- `src/core/renderTables.ts` (`renderTables`) detects GFM table blocks and
re-emits each as a width-aligned ASCII table wrapped in a ``` code fence,
which every target platform renders in a fixed-width font so columns line up.
Column alignment markers (`:--`, `--:`, `:--:`) are honored. Tables wider than
`MAX_TABLE_WIDTH` have their widest columns truncated with an ellipsis (`…`).
Tables already inside a code fence are left untouched.
- `src/core/splitMessage.ts` is fence-aware: when a long response is split across
the per-message length limit, an open code fence is closed on one chunk and
re-opened on the next, so a rendered table never loses its monospace block.

Both run in the queue just before `provider.send`, so providers receive
already-normalized text. Caveat: inline markdown inside a cell (bold, links)
renders literally, since the cell now lives inside a code fence — alignment is
the deliberate tradeoff.

## Read-only mode

`/agents readonly on` puts an agent channel into read-only mode. In this mode the bridge relays messages from the agent (via the HTTP API) but does **not** forward user messages to the agent. Use `/agents readonly off` to resume normal two-way messaging.
Expand All @@ -68,6 +90,8 @@ The schema upgrades on first start: legacy `agent_channels` (single-PK `channel_
| `src/core/maestro.ts` | `maestro-cli` wrapper |
| `src/core/transcription.ts` | ffmpeg + whisper pipeline |
| `src/core/attachments.ts` | Provider-agnostic attachment download |
| `src/core/renderTables.ts` | GFM tables → fenced, aligned ASCII tables |
| `src/core/splitMessage.ts` | Fence-aware message splitting to the length limit |
| `src/core/errors.ts` | Typed errors (`RateLimitError`, `AgentNotFoundError`) |
| `src/providers/discord/adapter.ts` | DiscordProvider implementing BridgeProvider |
| `src/providers/discord/messageCreate.ts` | Discord message → IncomingMessage |
Expand Down
1 change: 1 addition & 0 deletions docs/discord.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ npm run deploy-commands
- **Read-only mode** via `/agents readonly on` lets the bridge POST agent updates to the channel (via the HTTP API) without forwarding user messages back. Toggle off with `/agents readonly off`.
- **Reactions**: `⏳` while a message is queued, `🎧` while a voice message is being transcribed.
- **Usage stats** are appended below each agent reply (tokens, cost, context %).
- **Markdown tables** in agent replies are rendered as aligned, fenced ASCII tables so they display correctly (Discord has no native table syntax). See [architecture.md → Output rendering](architecture.md#output-rendering).

## Security

Expand Down
1 change: 1 addition & 0 deletions docs/slack.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ The Slack provider deliberately ships a smaller command surface than Discord —
- **Reactions**: `⏳` (`hourglass_flowing_sand`) while a message is queued. The Slack API requires emoji *names*, not Unicode characters; the adapter maps `⏳ 🎧 ✅ ❌` to the corresponding Slack names — pass any of them to `provider.react()` and the mapping happens automatically.
- **Typing indicator**: not exposed by Slack's Web API; `sendTyping` is a no-op on this provider.
- **Usage stats** are appended below each agent reply (tokens, cost, context %).
- **Markdown tables** in agent replies are rendered as aligned, fenced ASCII tables so they display correctly (Slack mrkdwn has no native table syntax). See [architecture.md → Output rendering](architecture.md#output-rendering).
- **Channel naming**: agent channels are named `maestro-<sanitized-agent-name>-<id-prefix>`, where `id-prefix` is the first 8 alphanumeric characters of the agent ID. The agent ID makes the name unique even when two different agents normalize to the same display name. The whole result is capped at 80 characters. Both `/agents new` and the HTTP-API auto-create path (`POST /api/send`) use the same helper. If the channel already exists but is archived, the adapter unarchives it; if unarchive fails, it falls back to creating a fresh channel with a `-<timestamp>` suffix appended to the base name.

## Storage
Expand Down
165 changes: 165 additions & 0 deletions src/__tests__/renderTables.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { renderTables, MAX_TABLE_WIDTH } from '../core/renderTables';

test('renderTables converts a basic table to a fenced ASCII table', () => {
const input = ['| Name | Qty |', '| --- | --- |', '| widget | 12 |', '| gadget | 3 |'].join(
'\n',
);
const out = renderTables(input);

assert.ok(out.startsWith('```\n'), 'wrapped in an opening fence');
assert.ok(out.endsWith('\n```'), 'wrapped in a closing fence');
assert.ok(out.includes('+--------+-----+'), 'has aligned rule lines');
assert.ok(out.includes('| Name | Qty |'), 'header is padded to column width');
assert.ok(out.includes('| widget | 12 |'), 'body cells are padded to column width');
});

test('renderTables honors column alignment markers', () => {
const input = ['| L | R | C |', '| :-- | --: | :-: |', '| a | b | c |'].join('\n');
const out = renderTables(input);
// Column widths are 1; with a width-1 column alignment is a no-op, so widen.
const wide = ['| Left | Right | Mid |', '| :-- | --: | :-: |', '| a | b | c |'].join('\n');
const w = renderTables(wide);

assert.ok(out.includes('| a | b | c |'));
assert.ok(
w.includes('| a | b | c |'),
'left col left-aligned, right col right-aligned, middle centered',
);
});

test('renderTables normalizes ragged rows to the header column count', () => {
const input = ['| A | B | C |', '| - | - | - |', '| 1 |', '| 1 | 2 | 3 | 4 |'].join('\n');
const out = renderTables(input);

// Short row is padded with empty cells; long row is truncated to 3 columns.
assert.ok(out.includes('| 1 | | |'));
assert.ok(out.includes('| 1 | 2 | 3 |'));
assert.ok(!out.includes('| 4 |'));
});

test('renderTables unescapes \\| inside cells', () => {
const input = ['| Expr |', '| --- |', '| a \\| b |'].join('\n');
const out = renderTables(input);
assert.ok(out.includes('a | b'));
});

test('renderTables handles tables without outer pipes', () => {
const input = ['Name | Qty', '--- | ---', 'widget | 12'].join('\n');
const out = renderTables(input);
assert.ok(out.includes('| widget |'));
assert.ok(out.includes('| Name | Qty |'));
});

test('renderTables leaves non-table text untouched', () => {
const input = 'Just a sentence.\nAnother line with a | pipe but no table.';
assert.equal(renderTables(input), input);
});

test('renderTables preserves surrounding prose', () => {
const input = ['Here are results:', '', '| A | B |', '| - | - |', '| 1 | 2 |', '', 'Done.'].join(
'\n',
);
const out = renderTables(input);
assert.ok(out.startsWith('Here are results:\n\n```'));
assert.ok(out.endsWith('```\n\nDone.'));
});

test('renderTables does not touch a table already inside a code fence', () => {
const input = ['```', '| A | B |', '| - | - |', '| 1 | 2 |', '```'].join('\n');
assert.equal(renderTables(input), input);
});

test('renderTables converts multiple tables in one message', () => {
const input = [
'| A | B |',
'| - | - |',
'| 1 | 2 |',
'',
'between',
'',
'| C | D |',
'| - | - |',
'| 3 | 4 |',
].join('\n');
const out = renderTables(input);
const fences = out.split('```').length - 1;
assert.equal(fences, 4, 'two tables → two open + two close fences');
assert.ok(out.includes('between'));
});

test('renderTables keeps body rows that look like a separator (regression: data loss)', () => {
// A dash-only data row must not terminate the table early.
const input = ['| A | B |', '| --- | --- |', '| --- | --- |', '| 1 | 2 |'].join('\n');
const out = renderTables(input);

// All three body rows survive inside one fenced table; nothing leaks as raw markdown.
assert.equal(out.split('```').length - 1, 2, 'exactly one fenced block');
assert.ok(out.includes('| --- | --- |'), 'dash row rendered as data');
assert.ok(out.includes('| 1 | 2 |'), 'trailing row survives');
});

test('renderTables ignores a table-like block inside a longer (4-backtick) fence', () => {
const input = ['````', '| A | B |', '| - | - |', '| 1 | 2 |', '```', 'still inside', '````'].join(
'\n',
);
// The inner ``` does not close the ```` block, so the table stays untouched.
assert.equal(renderTables(input), input);
});

test('renderTables does not convert header/separator column-count mismatches', () => {
const input = ['a | b | c', '--- | ---', '1 | 2'].join('\n');
assert.equal(renderTables(input), input);
});

test('renderTables does not treat a backtick line with backticks in its info as a fence', () => {
// Per CommonMark a backtick fence opener may not have backticks in its info
// string, so this line is plain text and must not suppress the table below.
const input = ['```js `inline`', 'just prose', '', '| A | B |', '| - | - |', '| 1 | 2 |'].join(
'\n',
);
const out = renderTables(input);
assert.ok(out.includes('+---+---+'), 'the following table is still converted');
});

test('renderTables stops the table at a following list/blockquote even with a pipe', () => {
const input = ['| A | B |', '| - | - |', '| 1 | 2 |', '- next | note', '> quote | here'].join(
'\n',
);
const out = renderTables(input);

// The table ends before the list item; list and quote stay as raw markdown.
assert.ok(out.includes('| 1 | 2 |'), 'table body rendered');
assert.ok(out.includes('\n- next | note'), 'list item left intact outside the table');
assert.ok(out.includes('\n> quote | here'), 'blockquote left intact outside the table');
assert.equal(out.split('```').length - 1, 2, 'exactly one fenced block');
});

test('renderTables does not convert a 4-space-indented (indented-code) table', () => {
const input = [' | A | B |', ' | - | - |', ' | 1 | 2 |'].join('\n');
// Indented 4+ spaces is a code block, not a table — leave it verbatim.
assert.equal(renderTables(input), input);
});

test('renderTables still converts a table after an over-indented (non-)fence line', () => {
// 4+ leading spaces is indented code, not a fence, so it must NOT suppress
// table detection for the table that follows.
const input = [' ```', 'indented sample', '', '| A | B |', '| - | - |', '| 1 | 2 |'].join(
'\n',
);
const out = renderTables(input);
assert.ok(out.includes('+---+---+'), 'table after indented line is still rendered');
assert.ok(out.startsWith(' ```'), 'the indented line is left as-is');
});

test('renderTables truncates wide cells with an ellipsis under the width cap', () => {
const long = 'x'.repeat(120);
const input = ['| Col |', '| --- |', `| ${long} |`].join('\n');
const out = renderTables(input);

assert.ok(out.includes('…'), 'truncation marker present');
// No rendered content row should exceed the cap budget plus borders/padding.
const longestRow = Math.max(...out.split('\n').map((l) => l.length));
assert.ok(longestRow <= MAX_TABLE_WIDTH + 4, `row width ${longestRow} within cap`);
});
73 changes: 73 additions & 0 deletions src/__tests__/splitMessage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,76 @@ test('splitMessage hard-splits and trims leading whitespace', () => {
assert.equal(parts[1], 'y');
assert.ok(parts.every((part) => part.length <= MAX_LENGTH));
});

test('splitMessage re-fences a code block split across a boundary', () => {
const before = 'p'.repeat(1500);
const code = 'c'.repeat(1500);
const input = `${before}\n\`\`\`\n${code}\n\`\`\``;
const parts = splitMessage(input);

assert.ok(parts.length >= 2, 'message was split');
// Every part must contain a balanced number of fence delimiters.
for (const part of parts) {
const fences = part.split('```').length - 1;
assert.equal(fences % 2, 0, `part has balanced fences: ${fences}`);
}
// The fenced content survives across the boundary.
assert.ok(parts.some((p) => p.includes(code.slice(0, 100))));
});

test('splitMessage leaves fence-free splits unchanged', () => {
const left = 'a'.repeat(1000);
const right = 'b'.repeat(1200);
const parts = splitMessage(`${left}\n${right}`);
assert.deepEqual(parts, [left, right]);
});

test('splitMessage treats an inner ``` inside a 4-backtick block as content', () => {
const before = 'p'.repeat(1500);
const inner = 'c'.repeat(800) + '\n```\n' + 'd'.repeat(800); // a literal ``` line inside
const input = `${before}\n\`\`\`\`\n${inner}\n\`\`\`\``;
const parts = splitMessage(input);

assert.ok(parts.length >= 2, 'message was split');
// Each chunk must have balanced 4-backtick fences; the inner ``` must not be
// mistaken for a close/open, which would corrupt the block.
for (const part of parts) {
const four = part.split('````').length - 1;
assert.equal(four % 2, 0, `balanced 4-backtick fences: ${four}`);
}
});

test('splitMessage does not split a fenced message that already fits', () => {
// Length between (maxLength - reserve) and maxLength must still be one chunk.
const body = 'x'.repeat(DEFAULT_MAX_LENGTH - 10);
const input = '```\n' + body + '\n```';
assert.ok(input.length <= DEFAULT_MAX_LENGTH, 'precondition: fits in one message');
assert.deepEqual(splitMessage(input), [input]);
});

test('splitMessage reserves for fence info strings (language labels)', () => {
// A re-opened ```typescript line is longer than a bare ``` — the reserve must
// be sized from the actual fence, not a fixed constant.
const input = '```typescript\n' + 'x'.repeat(5000) + '\n```';
const parts = splitMessage(input);

assert.ok(parts.length > 1, 'split into multiple chunks');
for (const part of parts) {
assert.ok(part.length <= MAX_LENGTH, `chunk length ${part.length} <= ${MAX_LENGTH}`);
}
// The language label is preserved on every re-opened chunk (slice(1) so the
// assertion is non-vacuous even for a two-part split).
assert.ok(parts.slice(1).every((p) => p.startsWith('```typescript')));
});

test('splitMessage honors a custom maxLength even when re-fencing', () => {
const code = 'c'.repeat(120);
const input = `\`\`\`\n${code}\n\`\`\``;
const max = 40;
const parts = splitMessage(input, max);

assert.ok(parts.length > 1, 'split into multiple chunks');
for (const part of parts) {
assert.ok(part.length <= max, `chunk length ${part.length} <= ${max}`);
}
});
68 changes: 68 additions & 0 deletions src/core/fences.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Minimal, CommonMark-aware fenced-code-block tracking, shared by
* `renderTables` (skip tables inside fences) and `splitMessage` (re-fence
* across chunk boundaries).
*
* The important subtlety: fences may be longer than three characters (e.g. a
* ```` ```` block deliberately wraps content that itself contains ```), and a
* closing fence must use the *same character*, be *at least as long* as the
* opener, and carry *no info string*. Collapsing every marker to a 3-char token
* would mistake an inner ``` line for a close.
*/

export interface Fence {
char: '`' | '~';
len: number;
/** The info string (e.g. language) that follows an opening fence; '' for a bare/closing fence. */
info: string;
}

/**
* Parse a line as a fenced-code delimiter, or return null if it isn't one.
* Per CommonMark a fence may be indented at most three spaces; four or more
* makes it an indented-code line, not a fence, so we must not treat it as one.
*/
export function parseFenceLine(line: string): Fence | null {
const m = line.match(/^ {0,3}(`{3,}|~{3,})\s*(.*)$/);
if (!m) return null;
const char = m[1][0] as '`' | '~';
const info = m[2].trim();
// CommonMark: a backtick fence's info string may not contain a backtick
// (it would be ambiguous with inline code), so such a line is not a fence.
if (char === '`' && info.includes('`')) return null;
return { char, len: m[1].length, info };
}

/** Whether `fence` can close an open block started by `open`. */
export function closesFence(open: Fence, fence: Fence): boolean {
return fence.char === open.char && fence.len >= open.len && fence.info === '';
}

/** The literal line that re-opens a block, preserving the original info string. */
export function openLine(f: Fence): string {
return f.char.repeat(f.len) + (f.info ? f.info : '');
}

/** The literal line that closes a block (bare marker, no info string). */
export function closeLine(f: Fence): string {
return f.char.repeat(f.len);
}

/**
* Scan `text` and return the fence left open at the end, or null if balanced.
* A fence-looking line that can't close the current block is treated as content.
*/
export function danglingFence(text: string): Fence | null {
let open: Fence | null = null;
for (const line of text.split('\n')) {
const fence = parseFenceLine(line);
if (!fence) continue;
if (open) {
if (closesFence(open, fence)) open = null;
// else: shorter/different/info-bearing fence inside the block → content
} else {
open = fence;
}
}
return open;
}
3 changes: 2 additions & 1 deletion src/core/queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
ReactionHandle,
} from './types';
import { splitMessage as defaultSplitMessage } from './splitMessage';
import { renderTables } from './renderTables';
import { downloadAttachments as defaultDownload, formatAttachmentRefs } from './attachments';

interface QueueEntry {
Expand Down Expand Up @@ -191,7 +192,7 @@ export function createQueue(deps: QueueDeps) {
`agent=${conv.agentId} session=${conv.sessionId ?? 'new'} channel=${message.channelId} error=${result.error}`,
);
}
const parts = split(result.response);
const parts = split(renderTables(result.response));
for (const part of parts) {
await provider.send(target, { text: part });
}
Expand Down
Loading