Reviewers on #2933 asked for tests on the button-style plumbing. Two layers:
- src/channels/ask-question.test.ts (new): normalizeOption/normalizeOptions
style whitelist — primary/danger/default pass through; unknown strings,
case variants, and non-string values drop to undefined; string-shorthand
options carry no style; style coexists with the label/selectedLabel/value
defaulting. This is the load-bearing gate: an invalid style reaching
Slack Block Kit fails the whole card with invalid_blocks, which in the
approval flow is an effective auto-deny.
- src/channels/chat-sdk-bridge.test.ts: ask_question delivery passes each
normalized option style into Button() and omits it when unset; an invalid
style in the raw payload is stripped before the card is built.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The dispatcher trimmed exactly one trailing dash-segment to recover the
target id, so any generated id containing dashes (UUIDs, sess-*, appr-*)
never matched a command and failed unknown-command; only --id worked.
Resolve by longest registered prefix instead: split the dash-joined
command, take the longest prefix that lookup() resolves as the command,
and treat the re-joined remainder as args.id. Server-side only, no wire
or client change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add '/start' back to the host FILTERED set (a Telegram fix from 866b791
was silently undone when host gating was added) so it is dropped instead
of reaching the agent as a normal message. Replace the inline isAdmin
query and its hasTable('user_roles') fail-open guard with a call to
hasAdminPrivilege; user_roles always exists (core migration 001-initial),
so the guard only ever masked a missing check. Add a focused command-gate
test covering both.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Approval cards render every button with the same neutral style, so Approve
and Reject read identically at a glance. The chat SDK's Button already
accepts style ('primary' | 'danger' | 'default') and @chat-adapter/slack
maps primary→green and danger→red Block Kit styles (Telegram ignores it),
but the ask_question pipeline dropped the field.
- ask-question.ts: add OptionStyle and an optional style field to
OptionInput/NormalizedOption; normalizeOption whitelists the value.
Bare-string options stay unstyled.
- chat-sdk-bridge.ts: forward opt.style into Button() on ask_question
cards. Persisted options_json tolerates the extra key
(getAskQuestionRender only reads label/selectedLabel/value).
- Producers: module approvals (Approve/Reject), sender approvals
(Allow/Deny), channel approvals (Connect/Reject), and OneCLI credential
cards (Approve/Reject) annotate primary/danger. Multi-choice picker
options stay unstyled — a list of equals.
Purely additive: options without style render exactly as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The container self-registration barrel imported mock.js, so every container
registered a 'mock' provider returning canned text. A typo'd --provider mock
would silently 'work' instead of failing loudly. Drop the import (mock.ts stays
for direct test use) and tidy the now-stale host comments and docs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hosted OneCLI gateway (api.onecli.sh) sends a structured summary field
on ApprovalRequest — { action, details: [{label, value}] } — that the SDK's
TypeScript type doesn't declare yet. When present, render it as the approval
card body (*Action:* plus labeled fields, fencing multi-line values) instead
of the raw METHOD host/path + bodyPreview, so approvers see what the agent
is doing (e.g. To / Subject / Body of an email send) rather than an HTTP
trace.
Rendering is defensive: non-string values are coerced via JSON.stringify (a
render throw would turn into a deny via handleRequest's catch), each value is
capped at 900 chars, and a 2600-char running budget keeps the card under
Slack's 3000-char section block limit — overflow is reported with an
explicit "…N field(s) omitted for length" line instead of a failed
delivery. Without a summary, the old bodyPreview fallback remains, now
capped and with the method/host/path as a footnote.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address review feedback on PR #2795:
- SKILL.md Step 1 now creates tools/ before cp -R (cp does not create
intermediate parents; tools/ is not a standard NanoClaw dir, so the copy
failed on a fresh checkout).
- package.json engines bumped to >=22.5 — server.js statically imports
activity.js -> node:sqlite (DatabaseSync), which lands in Node 22.5, so
the server crashed at module load on Node 20/21 LTS.
- Doc references (SKILL.md, README.md) updated to Node >=22.5 for consistency.
Verified 87/87 tests pass on Node 22.14.
Agent templates: folder-only templates under templates/ (context/instructions.md +
optional context extras, .mcp.json, skills/). Stamping via ncl groups create
--template writes the provider-neutral instructions.prepend.md (inlined at the top
of CLAUDE.md/AGENTS.md every spawn), copies context extras preserving their
template-relative layout, writes MCP servers to container config, and installs the
per-group skills overlay. Includes docs (docs/templates.md, templates/README.md).
Setup-wizard wiring ships separately on top of this.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #2837 added Slack Socket Mode end-to-end ("adapter + guided setup") but
was merged into the `channels` branch, not `main`. As a result the
`setup:auto` Slack flow on main is webhook-only: it always collects a
signing secret and pushes the user toward a public Request URL, with no
Socket Mode option — even though setup/verify.ts already recognizes
SLACK_APP_TOKEN and the channels-branch adapter supports it.
Forward-port the setup-side of #2837 onto current main, re-authored on top
of main's current flow (back-nav, inline agent wiring, operator-role prompt,
welcome DM all preserved):
- setup/channels/slack.ts: add a mode picker (askSlackMode), mode-specific
app-creation steps, collectAppToken() for the xapp- app-level token,
conditional credential collection + env, and a mode-aware post-install
checklist (Socket Mode skips the public-URL guidance).
- setup/add-slack.sh: require/persist either SLACK_APP_TOKEN (Socket Mode)
or SLACK_SIGNING_SECRET (webhook) instead of mandating the signing secret.
Scope is setup-side only: the adapter's socket support already lives on
`channels` (#2837/#2839) and reaches users via /add-slack; the add-slack
SKILL.md doc is handled by #2700.
`ncl messaging-groups create` failed with a NOT NULL violation on the
`instance` column (migration 016). The generic CRUD insert builds its
column list from the resource definition, and `instance` wasn't declared
there — so the INSERT omitted the column entirely. The router path never
hit this because it goes through `createMessagingGroup`, which has its own
`instance ?? channel_type` fallback.
There is no operator-facing reason to require `--instance`: the default
instance IS the channel type (migration 016, `createMessagingGroup`, and
the default-instance resolver all encode this). So rather than force a
flag, default it.
- crud: add `defaultFrom` to ColumnDef — default a column to another
already-resolved column's value on create. Generic, reusable.
- messaging-groups: declare `instance` with `defaultFrom: 'channel_type'`
(placed after channel_type so it resolves first), still overridable via
`--instance` for multi-instance setups.
- test: drive the real dispatch('messaging-groups-create') path; asserts
omitted -> channel_type and explicit --instance preserved. Goes red if
the column/defaultFrom wiring is deleted (insert fails NOT NULL).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extractAttachmentFiles (the channel-inbound attachment path) hardened only
the per-message inbox subdir, not the `inbox` root itself. A compromised
container can write inside its own session dir, so it could replace `inbox`
with a symlink: mkdirSync({recursive}) then followed it, and the realpath
containment check passed because it compared against realpathSync(inboxRoot)
— which had already followed the symlink. A brand-new attachment file (the
`wx` flag only blocks an existing dst) therefore landed outside the session
sandbox. This is the same symlink-follow class fixed for the A2A path in
#2828 (CWE-59), but reachable from ordinary inbound messages.
Extract the guard both inbound paths need into src/inbox-safety.ts
(ensureContainedInboxDir + isPathInside): lstat-reject a pre-placed symlink
or non-dir at the inbox root AND the per-message subdir before mkdir, then a
realpath containment check. forwardAttachedFiles and extractAttachmentFiles
now share it, removing the duplicated guard logic. Callers still write with
an exclusive flag (COPYFILE_EXCL / wx) and skip-with-warn on failure.
Adds src/session-manager.attachments.test.ts (red before this change, green
after) covering the symlinked inbox-root vector on the channel path.
forwardAttachedFiles hardened only the source side of A2A attachment
forwarding; the target side called fs.mkdirSync({recursive}) and
fs.copyFileSync without any symlink or containment checks. A compromised
target agent that can write inside its own session dir could pre-place
`inbox` (or `inbox/<future-msgId>`) as a symlink pointing anywhere
host-writable — mkdirSync silently follows it and copyFileSync lands
attacker-influenced bytes outside the sandbox (CWE-59, GHSA #2828). This
mirrors the existing defensive pattern in src/session-manager.ts
saveAttachments(): lstat-reject a pre-existing symlink/non-dir at the
inbox root and the per-message subdir before mkdir, realpath + isPathInside
containment check, and an exclusive (COPYFILE_EXCL) copy that refuses to
follow or overwrite a pre-placed symlinked destination. Failures log.warn
with structured context and skip rather than throw, so one bad attachment
never kills a batch. Tests cover a symlinked inbox dir, a symlinked
inbox/<msgId> subdir, a pre-existing symlinked destination file, and a
normal end-to-end forward regression.
gavrielc