Compare commits

...

15 Commits

Author SHA1 Message Date
Omri Maya 0d841bcd05 fix(ncl): default messaging-groups create instance to channel_type
`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>
2026-06-29 08:38:22 +03:00
gavrielc 2afbd18233 Merge pull request #2859 from cben0ist/fix/migrate-v2-is-main
fix(migrate-v2): don't SELECT is_main from v1 registered_groups
2026-06-26 13:38:42 +03:00
gavrielc 953496dc37 Merge branch 'main' into fix/migrate-v2-is-main 2026-06-26 13:38:27 +03:00
Christophe Benoist 797491d8b3 fix(migrate-v2): don't SELECT is_main from v1 registered_groups
The v2 DB seed queried `is_main` from the v1 `registered_groups` table, but
that column was a later v1 addition — older v1 installs (e.g. 1.1.0) don't have
it, so the migration's `1b-db` step crashes with `no such column: is_main` and
v2.db is never created, cascading into the sessions and tasks steps failing.

`is_main` was selected into the V1Group interface but never read anywhere, so
this just drops it from the SELECT and the interface. The accompanying comment
already states the intent ("Query only the columns we know exist in all v1
installs") — the code now matches it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-25 15:43:19 -04:00
github-actions[bot] 2df754459b chore: bump version to 2.1.21 2026-06-25 18:59:54 +00:00
gavrielc 0896d4089e Merge pull request #2832 from nanocoai/feat/reject-with-reason
feat(approvals): reject with reason
2026-06-25 21:59:42 +03:00
gavrielc d153d91307 Merge branch 'main' into feat/reject-with-reason 2026-06-25 21:45:12 +03:00
gavrielc ce55af12d5 Merge pull request #2843 from robbyczgw-cla/feat/learn-skill
feat: add /learn skill — distill or refine a reusable skill from anything
2026-06-25 21:41:23 +03:00
gavrielc 545800a94e Merge branch 'main' into feat/learn-skill 2026-06-25 21:40:55 +03:00
github-actions[bot] bfb309bd0c chore: bump version to 2.1.20 2026-06-25 18:34:34 +00:00
gavrielc 38d9390eea Merge pull request #2856 from nanocoai/container-limits
feat(container): per-container CPU/memory limits (opt-in)
2026-06-25 21:34:16 +03:00
robbyczgw-cla 520ec44aec feat: add /learn skill — distill or refine a reusable skill from anything
Instruction-only skill that distills a reusable skill from any source
(directory, URL, pasted notes, or the current conversation) or refines an
existing skill in place. Uses existing agent tools (Read/Grep/Glob/WebFetch/
Write) and injects the project's skill-authoring guidelines.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-24 09:41:49 +02:00
gavrielc 8c6a243ffd Merge branch 'main' into feat/reject-with-reason 2026-06-23 15:46:02 +03:00
Moshe Krupper 2ac7809385 feat(agent-to-agent): clarify the a2a gate approval prompt
Replace the terse "Approve delivery?" with a one-line legend that names all
three buttons and notes that "Reject with reason…" prompts the approver to
type a reason relayed back to the sender. The longer line also widens the
card bubble, easing the three-button-row truncation on platforms that size
buttons to the message width.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-22 17:16:03 +03:00
Moshe Krupper e8148bc0a7 feat(approvals): reject-with-reason — relay an optional decline reason to the agent
Add a third "Reject with reason…" button to module approval cards. Plain
Reject stays the instant fast path; the new option holds the row
(status='awaiting_reason'), DM-prompts the approver, and captures their
next DM (≤280 chars, truncated) as a one-line reason relayed to the
requesting agent as a single combined message. A ghosted hold is
finalized as a plain reject by the host sweep after ~5 min — restart-safe
via the durable DB row.

- Generalize the router message-interceptor to a list
  (registerMessageInterceptor) so approvals can capture replies alongside
  the permissions agent-naming flow.
- Share reject finalization across the instant, captured, and swept paths
  via finalizeReject.
- Scope: all module approvals (create_agent, install_packages,
  add_mcp_server); OneCLI credential cards are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 14:55:01 +03:00
18 changed files with 806 additions and 29 deletions
+97
View File
@@ -0,0 +1,97 @@
---
name: learn
description: "Distill a reusable skill from anything — a directory, a URL, pasted notes, or what you just did together — or refine an existing skill with new learnings. Use when the user says '/learn', 'learn this', 'turn this into a skill', 'capture this workflow', 'make a skill from <source>', or 'improve/update the <name> skill'. Produces or updates a .claude/skills/<name>/SKILL.md authored to NanoClaw's skill guidelines. (This CREATES or REFINES a skill from a source; it does not install existing skills from a registry.)"
---
# Learn — Distill a Skill from Anything
Turn a source — a directory, a URL, pasted notes, or the work just done in this conversation — into a clean, reusable NanoClaw skill. The output is a new `.claude/skills/<name>/SKILL.md` (plus optional `scripts/`, `references/`, `templates/`) authored to the project's skill guidelines.
This skill is **instruction-only**: it uses the tools you already have (`Read`, `Grep`, `Glob`, `WebFetch`, `Write`) — there is no separate distillation engine and no reach-ins into core code.
## When to use
Invoke when the user wants to *capture* a workflow as a reusable skill:
- `/learn <path>` — read a project/dir and build a skill for working with it
- `/learn <url>` — read docs / an API page and build a usage skill
- `/learn what we just did` — distill the current conversation's workflow
- `/learn` + pasted notes — turn notes into a structured skill
If the user instead wants to *find and install* an existing community skill, that is a different task — this skill **creates** new skills, it does not import them.
## Workflow
### 1. Identify the source — and whether this is a new skill or a refine
- A **path** → read the code/files.
- A **URL** → fetch and read the page.
- **"what we just did" / "this"** → use the current conversation as the source.
- **Pasted text** → use it directly.
Then check `.claude/skills/` for an existing skill that already covers this topic (the user may name it, e.g. *"update the wow-on-steam-deck skill"*, or the subject may obviously match one). **If one exists, this is a REFINE, not a fresh create** — go to step 4's "Refining" branch.
If it is ambiguous what the skill should *do*, ask one clarifying question before proceeding.
### 2. Gather the material
- **Path:** `Glob` the structure, `Read` the key files, `Grep` for the important entry points. Read enough to understand the *repeatable procedure*, not every line.
- **URL:** `WebFetch` the page; pull out the concrete commands/steps, not the prose.
- **Conversation:** re-read what was actually done — the commands, the gotchas, the decisions — and keep the parts that generalize.
### 3. Distill — find the reusable procedure
Strip the one-off specifics; keep the *repeatable* shape. A good skill answers: *"Next time someone needs to do X, what are the exact steps, files, commands, and gotchas?"* Capture:
- the trigger / when-to-use,
- the step-by-step procedure (commands, file paths, decision points),
- the non-obvious **gotchas** that were hit — usually the most valuable part,
- any scripts or templates worth shipping alongside.
### 4. Author the SKILL.md
**Refining an existing skill?** First `Read` the current `.claude/skills/<name>/SKILL.md`, then *update it in place* — do not blindly overwrite:
- Keep what is still correct; weave the new learnings into the right sections.
- **Dedupe** — don't append a near-duplicate step or a second gotcha that says the same thing.
- Correct anything the new source proves stale (a changed path, command, or flag).
- Preserve the existing `name`/folder and overall structure; the diff should read as a focused improvement, not a rewrite.
**New skill?** Write `.claude/skills/<kebab-name>/SKILL.md`.
**Frontmatter (required):**
```yaml
---
name: <kebab-case, matches the folder>
description: "<what it does + when to use it + likely trigger phrases>"
---
```
`description` is what the agent reads to decide relevance — make it concrete and include the phrases a user would actually say.
**Body:** open with one paragraph on what the skill does, then a `## When to use` section and a `## Workflow` of numbered steps (the actual procedure). Use tables for command/file references, and add a short examples or troubleshooting section when the gotchas warrant it.
**House authoring rules (from `docs/skill-guidelines.md`):**
- **Additive, minimal reach-ins** — prefer adding files; make the *smallest possible* edit to existing code, and only via single-line calls into skill-owned functions.
- **Instruction-only when possible** — if Claude can do it by following prose plus existing tools, ship no code. These are the easiest skills to maintain and to merge.
- If apply leaves anything behind, ship a **`REMOVE.md`** that fully reverses every change (no soft-disabled/commented-out removals).
- If the skill adds an integration point in core code, add a **test that goes red if the wiring is deleted or drifts**.
- Anti-patterns to avoid: separate `VERIFY.md` files, incomplete cleanup, raw SQL against core DBs, branch merges (use additive fetch), hand-maintained duplicate copies.
### 5. Place and verify
- Write into `.claude/skills/<name>/`; confirm the folder name matches the `name` frontmatter and the YAML parses.
- If feasible, dry-run the procedure the skill describes to confirm it is correct.
- Tell the user the skill exists and how to invoke it (`/<name>`).
## Example
`/learn what we just did` after a multi-step setup:
1. Re-read the conversation's commands and gotchas.
2. Distill the repeatable procedure.
3. Write `.claude/skills/<topic>-setup/SKILL.md` with the steps, file paths, and the gotchas hit along the way.
4. Report: *"Created `/<topic>-setup` — invoke it next time to repeat this."*
## Notes
- Keep skills **focused** — one capability per skill (mirrors the project's "one change per PR" rule).
- The most valuable content is the **gotchas**, not the happy path.
- This skill is prose and safe to re-run — use it again to refine an existing skill.
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "nanoclaw",
"version": "2.1.19",
"version": "2.1.21",
"description": "Personal Claude assistant. Lightweight, secure, customizable.",
"type": "module",
"packageManager": "pnpm@10.33.0",
+1 -2
View File
@@ -43,7 +43,6 @@ interface V1Group {
folder: string;
trigger_pattern: string | null;
requires_trigger: number | null;
is_main: number | null;
}
async function main(): Promise<void> {
@@ -65,7 +64,7 @@ async function main(): Promise<void> {
// v1 schema varies — channel_name was a late addition. Query only the
// columns we know exist in all v1 installs.
const v1Groups = v1Db
.prepare('SELECT jid, name, folder, trigger_pattern, requires_trigger, is_main FROM registered_groups')
.prepare('SELECT jid, name, folder, trigger_pattern, requires_trigger FROM registered_groups')
.all() as V1Group[];
v1Db.close();
+4
View File
@@ -30,6 +30,8 @@ export interface ColumnDef {
updatable?: boolean;
/** Default value on create when not provided. */
default?: unknown;
/** Default to another column's resolved value on create when not provided. */
defaultFrom?: string;
/** Allowed values (shown in help). */
enum?: string[];
}
@@ -150,6 +152,8 @@ function genericCreate(def: ResourceDef) {
throw new Error(`--${col.name.replace(/_/g, '-')} is required`);
} else if (col.default !== undefined) {
values[col.name] = col.default;
} else if (col.defaultFrom !== undefined && values[col.defaultFrom] !== undefined) {
values[col.name] = values[col.defaultFrom];
}
}
@@ -0,0 +1,75 @@
/**
* Regression test: `ncl messaging-groups create` must satisfy the NOT NULL
* `instance` column without an operator-supplied `--instance`. The column has
* no CLI flag at the operator's altitude (the default instance IS the channel
* type), so the generic CRUD insert defaults it to `channel_type` — matching
* `createMessagingGroup`'s `instance ?? channel_type` fallback on the router
* path. Delete the `instance` column / `defaultFrom` wiring in
* `messaging-groups.ts` and this goes red: the insert fails the NOT NULL.
*/
import fs from 'fs';
import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest';
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
isContainerRunning: vi.fn().mockReturnValue(false),
getActiveContainerCount: vi.fn().mockReturnValue(0),
killContainer: vi.fn(),
}));
vi.mock('../../config.js', async () => {
const actual = await vi.importActual('../../config.js');
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-cli-msggroups' };
});
const TEST_DIR = '/tmp/nanoclaw-test-cli-msggroups';
import { initTestDb, closeDb, runMigrations } from '../../db/index.js';
import { getMessagingGroupByPlatform } from '../../db/messaging-groups.js';
import { dispatch } from '../dispatch.js';
// Side-effect import: registers the `messaging-groups-create` command.
import './messaging-groups.js';
describe('messaging-groups CLI create defaults instance to channel_type', () => {
beforeEach(() => {
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true });
fs.mkdirSync(TEST_DIR, { recursive: true });
runMigrations(initTestDb());
});
afterEach(() => {
closeDb();
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true });
});
it('create without --instance sets instance = channel_type', async () => {
// caller: 'host' is the post-approval re-entry path for create (approval op).
const resp = await dispatch(
{
id: 'req-1',
command: 'messaging-groups-create',
args: { channel_type: 'telegram', platform_id: '12345' },
},
{ caller: 'host' },
);
expect(resp.ok).toBe(true);
const row = getMessagingGroupByPlatform('telegram', '12345');
expect(row).toBeDefined();
expect(row?.instance).toBe('telegram');
});
it('create with an explicit --instance keeps that value', async () => {
const resp = await dispatch(
{
id: 'req-2',
command: 'messaging-groups-create',
args: { channel_type: 'telegram', platform_id: '67890', instance: 'work' },
},
{ caller: 'host' },
);
expect(resp.ok).toBe(true);
expect(getMessagingGroupByPlatform('telegram', '67890', 'work')?.instance).toBe('work');
});
});
+8
View File
@@ -23,6 +23,14 @@ registerResource({
'Platform-specific chat ID. Format varies: Telegram chat ID, Discord channel snowflake, Slack channel ID, phone number, email address.',
required: true,
},
{
name: 'instance',
type: 'string',
description:
'Adapter instance that owns this chat, when running N adapters of one channel type. Defaults to channel_type (the default instance) when omitted.',
defaultFrom: 'channel_type',
updatable: true,
},
{
name: 'name',
type: 'string',
+22
View File
@@ -185,6 +185,28 @@ export function updatePendingApprovalStatus(approvalId: string, status: PendingA
getDb().prepare('UPDATE pending_approvals SET status = ? WHERE approval_id = ?').run(status, approvalId);
}
/**
* Park an approval in the "rejected, awaiting reason" hold: the admin clicked
* "Reject with reason…" and we're waiting for their one-line reply. `expiresAt`
* is the deadline after which the host sweep finalizes a plain reject (so a
* ghosted hold never strands the requesting agent). Reuses the otherwise-unused
* `expires_at` column on module-initiated rows.
*/
export function markApprovalAwaitingReason(approvalId: string, expiresAt: string): void {
getDb()
.prepare("UPDATE pending_approvals SET status = 'awaiting_reason', expires_at = ? WHERE approval_id = ?")
.run(expiresAt, approvalId);
}
/** Awaiting-reason approvals whose reply window has elapsed — the sweep's ghost set. */
export function getExpiredAwaitingReasonApprovals(nowIso: string): PendingApproval[] {
return getDb()
.prepare(
"SELECT * FROM pending_approvals WHERE status = 'awaiting_reason' AND expires_at IS NOT NULL AND expires_at <= ?",
)
.all(nowIso) as PendingApproval[];
}
export function deletePendingApproval(approvalId: string): void {
getDb().prepare('DELETE FROM pending_approvals WHERE approval_id = ?').run(approvalId);
}
+12
View File
@@ -152,6 +152,18 @@ async function sweep(): Promise<void> {
log.error('Host sweep error', { err });
}
// Finalize any "Reject with reason…" holds whose reply window elapsed (admin
// ghosted, or the host restarted mid-capture). Central-DB scan, once per tick
// — not per session.
// MODULE-HOOK:approvals-reason-sweep:start
try {
const { sweepAwaitingReasonRejects } = await import('./modules/approvals/index.js');
await sweepAwaitingReasonRejects();
} catch (err) {
log.error('Reject-with-reason sweep failed', { err });
}
// MODULE-HOOK:approvals-reason-sweep:end
setTimeout(sweep, SWEEP_INTERVAL_MS);
}
+4 -1
View File
@@ -278,7 +278,10 @@ function buildGateQuestion(sourceName: string, targetName: string, contentStr: s
const body = text.length > GATE_CARD_BODY_MAX ? `${text.slice(0, GATE_CARD_BODY_MAX)}… (truncated)` : text;
const lines = [`Agent "${sourceName}" wants to send a message to "${targetName}":`, '', body];
if (files.length > 0) lines.push('', `Attachments: ${files.join(', ')}`);
lines.push('', 'Approve delivery?');
lines.push(
'',
`Approve, Reject, or "Reject with reason…" to decline and then type a short reason I'll relay to "${sourceName}".`,
);
return lines.join('\n');
}
+59
View File
@@ -0,0 +1,59 @@
/**
* Shared "finalize a rejected approval" path.
*
* Three entry points land here so they relay one message and clean up
* identically:
* 1. The instant Reject button (response-handler.ts)
* 2. A captured Reject-with-reason reply (reason-capture.ts)
* 3. The host-sweep ghost finalizer (reason-capture.ts, via host-sweep)
*
* Kept in its own leaf file so both response-handler.ts and reason-capture.ts
* can import it without an import cycle (finalize → primitive only).
*/
import { wakeContainer } from '../../container-runner.js';
import { deletePendingApproval } from '../../db/sessions.js';
import { log } from '../../log.js';
import { writeSessionMessage } from '../../session-manager.js';
import type { PendingApproval, Session } from '../../types.js';
import { notifyApprovalResolved } from './primitive.js';
/**
* Notify the requesting agent that its action was rejected, drop the pending
* row, fire approval-resolved callbacks, and wake the container.
*
* When `reason` is provided it's appended to the agent-facing note with generic
* attribution — the why, not the who (the rejecting admin may belong to a
* different owner than the requesting agent). Callers are responsible for
* clamping the reason length before passing it in.
*/
export async function finalizeReject(
approval: PendingApproval,
session: Session,
userId: string,
reason?: string,
): Promise<void> {
const text = reason
? `Your ${approval.action} request was rejected by admin: "${reason}"`
: `Your ${approval.action} request was rejected by admin.`;
writeSessionMessage(session.agent_group_id, session.id, {
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
kind: 'chat',
timestamp: new Date().toISOString(),
platformId: session.agent_group_id,
channelType: 'agent',
threadId: null,
content: JSON.stringify({ text, sender: 'system', senderId: 'system' }),
});
log.info('Approval rejected', {
approvalId: approval.approval_id,
action: approval.action,
userId,
withReason: reason !== undefined,
});
deletePendingApproval(approval.approval_id);
await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
await wakeContainer(session);
}
+9
View File
@@ -8,10 +8,16 @@
* - A response handler that claims pending_approvals rows and dispatches
* to whatever module registered for the row's `action` string. Also
* resolves in-memory OneCLI credential approvals.
* - A message-interceptor (via ./reason-capture.js) that captures an admin's
* one-line reply after they click "Reject with reason…".
* - An adapter-ready callback that starts the OneCLI manual-approval handler
* once the delivery adapter is set.
* - A shutdown callback that stops the OneCLI handler cleanly.
*
* Exposes `sweepAwaitingReasonRejects` for the host sweep to finalize ghosted
* reject-with-reason holds (re-exported here, which also loads reason-capture
* so its interceptor registers).
*
* Self-mod flows (install_packages, add_mcp_server) moved out to
* `src/modules/self-mod/` in PR #7 — they now register delivery actions
* + approval handlers via this module's public API.
@@ -24,6 +30,9 @@ import { startOneCLIApprovalHandler, stopOneCLIApprovalHandler } from './onecli-
// Public API re-exports so consumers import from the module root.
export { requestApproval, registerApprovalHandler, notifyAgent } from './primitive.js';
export type { ApprovalHandler, ApprovalHandlerContext, RequestApprovalOptions } from './primitive.js';
// Host-sweep hook for ghosted "Reject with reason…" holds. The re-export also
// loads reason-capture.js, registering its message-interceptor on import.
export { sweepAwaitingReasonRejects } from './reason-capture.js';
registerResponseHandler(handleApprovalsResponse);
+14 -1
View File
@@ -32,10 +32,23 @@ import type { MessagingGroup, PendingApproval, Session } from '../../types.js';
import { getAdminsOfAgentGroup, getGlobalAdmins, getOwners } from '../permissions/db/user-roles.js';
import { ensureUserDm } from '../permissions/user-dm.js';
/** Two-button approval UI — the only options the primitive supports today. */
/**
* Card value for the "Reject with reason…" button. Selecting it doesn't
* finalize the reject — it holds the row and captures the approver's next DM
* as a one-line reason relayed to the requesting agent. See reason-capture.ts.
*/
export const REJECT_WITH_REASON_VALUE = 'reject_with_reason';
/**
* Three-button approval UI. Plain Reject is the instant fast path; "Reject with
* reason…" opts into the reason-capture flow. Shared by every module approval
* (create_agent, install_packages, add_mcp_server); OneCLI credential cards
* keep their own two-button set in onecli-approvals.ts.
*/
const APPROVAL_OPTIONS: RawOption[] = [
{ label: 'Approve', selectedLabel: '✅ Approved', value: 'approve' },
{ label: 'Reject', selectedLabel: '❌ Rejected', value: 'reject' },
{ label: 'Reject with reason…', selectedLabel: '📝 Rejected (awaiting reason)', value: REJECT_WITH_REASON_VALUE },
];
// ── Approval handler registry ──
@@ -0,0 +1,279 @@
/**
* "Reject with reason…" capture flow.
*
* Covers the three entry points end to end against the real central DB:
* - arming (handleApprovalsResponse with the third option) holds the row and
* prompts the admin instead of finalizing;
* - the captured reply relays one combined message, clamped to 280 chars;
* - the host sweep finalizes a ghosted hold as a plain reject.
*
* writeSessionMessage is mocked so the relayed agent-facing text can be read
* back directly; the delivery adapter is a fake that records prompt sends.
*/
import * as fs from 'fs';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type { InboundEvent } from '../../channels/adapter.js';
import { initTestDb, closeDb, runMigrations } from '../../db/index.js';
import { createAgentGroup } from '../../db/agent-groups.js';
import { createMessagingGroup } from '../../db/messaging-groups.js';
import {
createSession,
createPendingApproval,
deletePendingApproval,
getPendingApproval,
markApprovalAwaitingReason,
} from '../../db/sessions.js';
import { setDeliveryAdapter, type ChannelDeliveryAdapter } from '../../delivery.js';
import { writeSessionMessage } from '../../session-manager.js';
import { upsertUser } from '../permissions/db/users.js';
import { upsertUserDm } from '../permissions/db/user-dms.js';
import { grantRole } from '../permissions/db/user-roles.js';
import { REJECT_WITH_REASON_VALUE } from './primitive.js';
vi.mock('../../container-runner.js', () => ({
wakeContainer: vi.fn().mockResolvedValue(undefined),
}));
vi.mock('../../config.js', async () => {
const actual = await vi.importActual('../../config.js');
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-reject-reason' };
});
vi.mock('../../session-manager.js', async () => {
const actual = await vi.importActual<typeof import('../../session-manager.js')>('../../session-manager.js');
return { ...actual, writeSessionMessage: vi.fn() };
});
const TEST_DIR = '/tmp/nanoclaw-test-reject-reason';
const DM_CHANNEL = 'slack';
const DM_PLATFORM = 'D-admin-1';
function now(): string {
return new Date().toISOString();
}
let delivered: Array<{ channelType: string; platformId: string; content: string }>;
const fakeAdapter: ChannelDeliveryAdapter = {
async deliver(channelType, platformId, _threadId, _kind, content) {
delivered.push({ channelType, platformId, content });
return 'pm-1';
},
};
function seedApproval(approvalId: string, action = 'create_agent'): void {
createPendingApproval({
approval_id: approvalId,
session_id: 'sess-1',
request_id: approvalId,
action,
payload: JSON.stringify({ name: 'child' }),
created_at: now(),
title: 'Approval',
options_json: JSON.stringify([]),
});
}
function dmReply(text?: string): InboundEvent {
const content: Record<string, unknown> = { sender: 'admin-1', senderId: 'admin-1' };
if (text !== undefined) content.text = text;
return {
channelType: DM_CHANNEL,
platformId: DM_PLATFORM,
threadId: null,
message: { id: 'm-1', kind: 'chat', content: JSON.stringify(content), timestamp: now() },
};
}
/** Click the "Reject with reason…" button as the seeded admin. */
async function clickRejectWithReason(approvalId: string): Promise<void> {
const { handleApprovalsResponse } = await import('./response-handler.js');
await handleApprovalsResponse({
questionId: approvalId,
value: REJECT_WITH_REASON_VALUE,
userId: 'admin-1',
channelType: DM_CHANNEL,
platformId: '', // not surfaced by the click payload — resolved via ensureUserDm
threadId: null,
});
}
/** The text of the most recent agent-facing note written via writeSessionMessage. */
function lastRelayedText(): string | undefined {
const call = vi.mocked(writeSessionMessage).mock.calls.at(-1);
if (!call) return undefined;
return (JSON.parse(call[2].content) as { text: string }).text;
}
beforeEach(() => {
vi.clearAllMocks();
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
fs.mkdirSync(TEST_DIR, { recursive: true });
const db = initTestDb();
runMigrations(db);
delivered = [];
createAgentGroup({ id: 'ag-1', name: 'Agent', folder: 'agent', agent_provider: null, created_at: now() });
createSession({
id: 'sess-1',
agent_group_id: 'ag-1',
messaging_group_id: null,
thread_id: null,
agent_provider: null,
status: 'active',
container_status: 'stopped',
last_active: now(),
created_at: now(),
});
// Authorized approver + a cached DM so ensureUserDm resolves without a
// platform openDM call.
upsertUser({ id: 'slack:admin-1', kind: 'slack', display_name: 'Admin', created_at: now() });
grantRole({ user_id: 'slack:admin-1', role: 'owner', agent_group_id: null, granted_by: null, granted_at: now() });
createMessagingGroup({
id: 'mg-dm-1',
channel_type: DM_CHANNEL,
platform_id: DM_PLATFORM,
name: 'Admin DM',
is_group: 0,
unknown_sender_policy: 'strict',
created_at: now(),
});
upsertUserDm({
user_id: 'slack:admin-1',
channel_type: DM_CHANNEL,
messaging_group_id: 'mg-dm-1',
resolved_at: now(),
});
setDeliveryAdapter(fakeAdapter);
});
afterEach(() => {
closeDb();
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
});
describe('reject with reason', () => {
it('holds the row and prompts the admin instead of finalizing', async () => {
seedApproval('appr-1');
await clickRejectWithReason('appr-1');
const row = getPendingApproval('appr-1');
expect(row?.status).toBe('awaiting_reason');
expect(row?.expires_at).toBeTruthy();
// Prompt went to the admin's resolved DM, not the (empty) click platformId.
expect(delivered).toHaveLength(1);
expect(delivered[0].channelType).toBe(DM_CHANNEL);
expect(delivered[0].platformId).toBe(DM_PLATFORM);
expect((JSON.parse(delivered[0].content) as { text: string }).text).toMatch(/reason/i);
// Agent is not notified yet — the hold is still open.
expect(vi.mocked(writeSessionMessage)).not.toHaveBeenCalled();
});
it('relays the captured reason as one combined message and clears the row', async () => {
const { captureReasonReply } = await import('./reason-capture.js');
seedApproval('appr-2', 'install_packages');
await clickRejectWithReason('appr-2');
const consumed = await captureReasonReply(dmReply('too risky for prod'));
expect(consumed).toBe(true);
expect(getPendingApproval('appr-2')).toBeUndefined();
expect(lastRelayedText()).toBe('Your install_packages request was rejected by admin: "too risky for prod"');
});
it('truncates an over-long reason to 280 chars with an ellipsis', async () => {
const { captureReasonReply } = await import('./reason-capture.js');
seedApproval('appr-3');
await clickRejectWithReason('appr-3');
await captureReasonReply(dmReply('x'.repeat(400)));
const reason = lastRelayedText()!.match(/: "(.*)"$/)![1];
expect(reason).toHaveLength(280);
expect(reason.endsWith('…')).toBe(true);
});
it('finalizes a plain reject when the captured reply carries no text', async () => {
const { captureReasonReply } = await import('./reason-capture.js');
seedApproval('appr-4');
await clickRejectWithReason('appr-4');
const consumed = await captureReasonReply(dmReply(undefined));
expect(consumed).toBe(true);
expect(getPendingApproval('appr-4')).toBeUndefined();
expect(lastRelayedText()).toBe('Your create_agent request was rejected by admin.');
});
it('does not swallow a later DM once the hold was already finalized', async () => {
const { captureReasonReply } = await import('./reason-capture.js');
seedApproval('appr-5');
await clickRejectWithReason('appr-5');
// Simulate the sweep (or any other path) finalizing first.
deletePendingApproval('appr-5');
const consumed = await captureReasonReply(dmReply('late reason'));
expect(consumed).toBe(false);
});
it('ignores DMs on channels with no armed reason capture', async () => {
const { captureReasonReply } = await import('./reason-capture.js');
const consumed = await captureReasonReply({
channelType: DM_CHANNEL,
platformId: 'D-someone-else',
threadId: null,
message: { id: 'm', kind: 'chat', content: JSON.stringify({ text: 'hi' }), timestamp: now() },
});
expect(consumed).toBe(false);
});
});
describe('reject-with-reason host sweep', () => {
it('finalizes a hold whose window elapsed as a plain reject', async () => {
const { sweepAwaitingReasonRejects } = await import('./reason-capture.js');
seedApproval('appr-ghost', 'add_mcp_server');
markApprovalAwaitingReason('appr-ghost', new Date(Date.now() - 1000).toISOString());
await sweepAwaitingReasonRejects();
expect(getPendingApproval('appr-ghost')).toBeUndefined();
expect(lastRelayedText()).toBe('Your add_mcp_server request was rejected by admin.');
});
it('leaves a still-open hold untouched', async () => {
const { sweepAwaitingReasonRejects } = await import('./reason-capture.js');
seedApproval('appr-open');
markApprovalAwaitingReason('appr-open', new Date(Date.now() + 60_000).toISOString());
await sweepAwaitingReasonRejects();
expect(getPendingApproval('appr-open')?.status).toBe('awaiting_reason');
expect(vi.mocked(writeSessionMessage)).not.toHaveBeenCalled();
});
});
describe('plain reject (regression)', () => {
it('finalizes immediately with no reason and no DM prompt', async () => {
const { handleApprovalsResponse } = await import('./response-handler.js');
seedApproval('appr-plain', 'install_packages');
await handleApprovalsResponse({
questionId: 'appr-plain',
value: 'reject',
userId: 'admin-1',
channelType: DM_CHANNEL,
platformId: '',
threadId: null,
});
expect(getPendingApproval('appr-plain')).toBeUndefined();
expect(delivered).toHaveLength(0);
expect(lastRelayedText()).toBe('Your install_packages request was rejected by admin.');
});
});
+174
View File
@@ -0,0 +1,174 @@
/**
* "Reject with reason…" capture flow.
*
* When an admin clicks the third approval button, the reject is held instead of
* finalized: the row is parked at status='awaiting_reason' and the admin is
* prompted in their DM for a one-line reason. Their next DM (≤ 280 chars) is
* captured by a router message-interceptor and relayed to the requesting agent
* as one combined message — `Your <action> request was rejected by admin:
* "<reason>"`. A plain Reject never arms this, so an unrelated DM is never
* swallowed.
*
* Restart-safety: arming lives in an in-memory map (lost on restart, like the
* agent-naming capture it mirrors), but the hold is a durable DB row. If the
* admin never replies — or the host restarts mid-capture — the host sweep
* (sweepAwaitingReasonRejects, run each tick) finalizes a plain reject once the
* row's window elapses, so the requesting agent is never stranded.
*
* Reuses, not reinvents: the agent-naming prompt-then-capture pattern
* (in-memory map + next-DM interceptor) and the shared finalizeReject path.
*/
import type { InboundEvent } from '../../channels/adapter.js';
import { getDeliveryAdapter } from '../../delivery.js';
import {
deletePendingApproval,
getExpiredAwaitingReasonApprovals,
getPendingApproval,
getSession,
markApprovalAwaitingReason,
} from '../../db/sessions.js';
import { log } from '../../log.js';
import { registerMessageInterceptor } from '../../router.js';
import type { PendingApproval, Session } from '../../types.js';
import { ensureUserDm } from '../permissions/user-dm.js';
import { finalizeReject } from './finalize.js';
/** How long an awaiting-reason hold waits for the admin's reply before the sweep finalizes a plain reject. */
const REASON_CAPTURE_WINDOW_MS = 5 * 60 * 1000;
/** Cap on the relayed reason — one cheap guardrail against a wall of text landing in another team's agent context. */
const MAX_REASON_LEN = 280;
const PROMPT_TEXT =
"Reply with a one-line reason for the rejection — I'll relay it to the agent. " +
'No reply within ~5 min declines it without a reason.';
interface ReasonArming {
approvalId: string;
/** Namespaced id of the admin who clicked, for resolution attribution. */
userId: string;
}
/**
* Approvers waiting to type a rejection reason, keyed by their DM channel
* (`<channelType>:<dmPlatformId>`). A DM's platform id is unique per user, so
* the inbound reply matches by channel alone — no sender re-parsing needed, and
* a group message can never collide with an armed DM. Cleared on receipt,
* staleness, or restart.
*/
const awaitingReason = new Map<string, ReasonArming>();
function dmKey(channelType: string, platformId: string): string {
return `${channelType}:${platformId}`;
}
function clampReason(raw: string): string {
const trimmed = raw.trim();
if (trimmed.length <= MAX_REASON_LEN) return trimmed;
return trimmed.slice(0, MAX_REASON_LEN - 1) + '…';
}
function extractText(event: InboundEvent): string {
try {
const parsed = JSON.parse(event.message.content) as Record<string, unknown>;
return typeof parsed.text === 'string' ? parsed.text : '';
} catch {
return '';
}
}
/**
* Begin the reject-with-reason hold for an approval the admin chose not to
* finalize outright. Prompts the admin's DM, then parks the row and arms
* capture. If we can't reach the admin (no DM, no adapter, delivery throws) we
* finalize a plain reject immediately rather than strand the requesting agent.
*/
export async function armReasonCapture(approval: PendingApproval, session: Session, userId: string): Promise<void> {
const dm = userId ? await ensureUserDm(userId) : null;
const adapter = getDeliveryAdapter();
if (!dm || !adapter) {
log.warn('reject-with-reason: cannot reach approver, finalizing plain reject', {
approvalId: approval.approval_id,
userId,
hasDm: Boolean(dm),
hasAdapter: Boolean(adapter),
});
await finalizeReject(approval, session, userId);
return;
}
try {
await adapter.deliver(dm.channel_type, dm.platform_id, null, 'chat-sdk', JSON.stringify({ text: PROMPT_TEXT }));
} catch (err) {
log.error('reject-with-reason: reason prompt delivery failed, finalizing plain reject', {
approvalId: approval.approval_id,
err,
});
await finalizeReject(approval, session, userId);
return;
}
// Prompt is out — now hold the row and arm capture. Order matters: a reply
// can't arrive before the prompt is read, so there's no lost-message window.
const expiresAt = new Date(Date.now() + REASON_CAPTURE_WINDOW_MS).toISOString();
markApprovalAwaitingReason(approval.approval_id, expiresAt);
awaitingReason.set(dmKey(dm.channel_type, dm.platform_id), { approvalId: approval.approval_id, userId });
log.info('reject-with-reason: awaiting reason reply', { approvalId: approval.approval_id, userId });
}
/**
* Router message-interceptor: capture the next DM from an admin who armed a
* reason. Returns true (consume the message) when this DM is an armed reason
* channel and still holds a live row; false otherwise so normal routing runs.
*
* Exported for tests; registered as the interceptor below.
*/
export async function captureReasonReply(event: InboundEvent): Promise<boolean> {
const arming = awaitingReason.get(dmKey(event.channelType, event.platformId));
if (!arming) return false;
// This DM is an armed reason channel — disarm regardless of outcome.
awaitingReason.delete(dmKey(event.channelType, event.platformId));
const approval = getPendingApproval(arming.approvalId);
if (!approval || approval.status !== 'awaiting_reason') {
// Already finalized (e.g. ghosted by the sweep). The reply is no longer a
// reason — let it route normally instead of swallowing it.
return false;
}
const session = approval.session_id ? getSession(approval.session_id) : null;
if (!session) {
deletePendingApproval(approval.approval_id);
return true;
}
const reason = clampReason(extractText(event));
await finalizeReject(approval, session, arming.userId, reason || undefined);
log.info('reject-with-reason: reason captured and relayed', {
approvalId: approval.approval_id,
hasReason: reason.length > 0,
});
return true;
}
registerMessageInterceptor(captureReasonReply);
/**
* Host-sweep finalizer: any reject-with-reason hold whose window elapsed (admin
* ghosted, or the host restarted mid-capture and lost the in-memory arming) is
* finalized as a plain reject. Restart-safe — the hold is a durable row, so the
* requesting agent always gets its decision. Called once per sweep tick.
*/
export async function sweepAwaitingReasonRejects(): Promise<void> {
const rows = getExpiredAwaitingReasonApprovals(new Date().toISOString());
for (const approval of rows) {
const session = approval.session_id ? getSession(approval.session_id) : null;
if (!session) {
deletePendingApproval(approval.approval_id);
continue;
}
// Plain reject, unknown resolver — the admin opted in but never typed.
await finalizeReject(approval, session, '');
log.info('reject-with-reason: window elapsed, finalized as plain reject', { approvalId: approval.approval_id });
}
}
+22 -12
View File
@@ -5,7 +5,10 @@
* 1. Module-initiated actions — the module called `requestApproval()` with
* some free-form `action` string and registered a handler via
* `registerApprovalHandler(action, handler)`. On approve, we look up the
* handler and call it; on reject, we notify the agent and move on.
* handler and call it; on plain reject we relay a decline to the agent; on
* "Reject with reason…" we hold the row and capture the admin's next DM as
* a one-line reason (see reason-capture.ts). Reject finalization is shared
* via finalizeReject.
* 2. OneCLI credential approvals (`action = 'onecli_credential'`). Resolved
* via an in-memory Promise — see onecli-approvals.ts.
*
@@ -19,8 +22,10 @@ import { log } from '../../log.js';
import { writeSessionMessage } from '../../session-manager.js';
import type { PendingApproval } from '../../types.js';
import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
import { finalizeReject } from './finalize.js';
import { ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
import { getApprovalHandler, notifyApprovalResolved } from './primitive.js';
import { getApprovalHandler, notifyApprovalResolved, REJECT_WITH_REASON_VALUE } from './primitive.js';
import { armReasonCapture } from './reason-capture.js';
export async function handleApprovalsResponse(payload: ResponsePayload): Promise<boolean> {
const approval = getPendingApproval(payload.questionId);
@@ -65,6 +70,21 @@ async function handleRegisteredApproval(
return;
}
// "Reject with reason…" — hold the row and capture the admin's next DM
// instead of finalizing now. The agent is notified exactly once: after the
// reason arrives, or after the sweep's timeout if the admin ghosts.
if (selectedOption === REJECT_WITH_REASON_VALUE) {
await armReasonCapture(approval, session, userId);
return;
}
// Plain Reject (or any other non-approve value) — instant fast path.
if (selectedOption !== 'approve') {
await finalizeReject(approval, session, userId);
return;
}
// Approved — dispatch to the module that registered for this action.
const notify = (text: string): void => {
writeSessionMessage(session.agent_group_id, session.id, {
id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
@@ -77,16 +97,6 @@ async function handleRegisteredApproval(
});
};
if (selectedOption !== 'approve') {
notify(`Your ${approval.action} request was rejected by admin.`);
log.info('Approval rejected', { approvalId: approval.approval_id, action: approval.action, userId });
deletePendingApproval(approval.approval_id);
await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
await wakeContainer(session);
return;
}
// Approved — dispatch to the module that registered for this action.
const handler = getApprovalHandler(approval.action);
if (!handler) {
log.warn('No approval handler registered — row dropped', {
+2 -2
View File
@@ -22,7 +22,7 @@ import {
routeInbound,
setAccessGate,
setChannelRequestGate,
setMessageInterceptor,
registerMessageInterceptor,
setSenderResolver,
setSenderScopeGate,
type AccessGateResult,
@@ -521,7 +521,7 @@ registerResponseHandler(handleChannelApprovalResponse);
// Captures the next DM from an approver who clicked "Create new agent",
// creates the agent immediately, wires the channel, and replays.
setMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
registerMessageInterceptor(async (event: InboundEvent): Promise<boolean> => {
const userId = extractAndUpsertUser(event);
if (!userId) return false;
+17 -9
View File
@@ -110,16 +110,20 @@ export function setSenderScopeGate(fn: SenderScopeGateFn): void {
/**
* Message-interceptor hook. Runs at the very top of routeInbound, before
* messaging-group resolution. When the interceptor returns true the message
* is consumed and routing stops. Used by the permissions module to capture
* free-text replies during multi-step approval flows (e.g. agent naming).
* messaging-group resolution. When an interceptor returns true the message is
* consumed and routing stops. Multiple interceptors may register; they run in
* registration order and the first to claim the message (return true) wins.
*
* Used by modules to capture free-text DM replies during multi-step approval
* flows — the permissions module (agent naming during channel registration)
* and the approvals module (reject-with-reason capture).
*/
export type MessageInterceptorFn = (event: InboundEvent) => Promise<boolean>;
let messageInterceptor: MessageInterceptorFn | null = null;
const messageInterceptors: MessageInterceptorFn[] = [];
export function setMessageInterceptor(fn: MessageInterceptorFn): void {
messageInterceptor = fn;
export function registerMessageInterceptor(fn: MessageInterceptorFn): void {
messageInterceptors.push(fn);
}
/**
@@ -156,9 +160,13 @@ function safeParseContent(raw: string): { text?: string; sender?: string; sender
* Creates messaging group + session if they don't exist yet.
*/
export async function routeInbound(event: InboundEvent): Promise<void> {
// Pre-route interceptor — lets modules consume messages before any routing
// (e.g. free-text replies during multi-step approval flows).
if (messageInterceptor && (await messageInterceptor(event))) return;
// Pre-route interceptors — let modules consume messages before any routing
// (e.g. free-text DM replies during multi-step approval flows). They run in
// registration order; the first to claim the message stops routing. The
// sequential await is intentional — first-to-claim is order-dependent.
for (const intercept of messageInterceptors) {
if (await intercept(event)) return;
}
// 0. Apply the adapter's thread policy. Non-threaded adapters (Telegram,
// WhatsApp, iMessage, email) collapse threads to the channel. Resolved
+6 -1
View File
@@ -200,8 +200,13 @@ export interface PendingApproval {
channel_type: string | null;
platform_id: string | null;
platform_message_id: string | null;
/**
* For OneCLI credential rows, the gateway's request TTL. For a module
* approval held by "Reject with reason…", the deadline after which the
* host sweep finalizes a plain reject (set by markApprovalAwaitingReason).
*/
expires_at: string | null;
status: 'pending' | 'approved' | 'rejected' | 'expired';
status: 'pending' | 'approved' | 'rejected' | 'expired' | 'awaiting_reason';
title: string;
options_json: string;
/** When set, only this exact user may resolve the approval. */