mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-07-12 19:00:58 +08:00
fix(cli): live-refresh destinations on ncl wirings create
Closes the restart-required behavior: wirings-create's postCreate wrote the central agent_destinations row but never projected it into running sessions' inbound.db, so an agent with a live container dropped every reply to the newly-wired chat as "unknown destination" until a group restart (observed on a live instance). `ncl destinations add` already projects for exactly this reason; wirings create now has parity. postCreate is sync and runs inside the central-DB transaction, so it's the wrong place for an async, cross-DB (session inbound.db) side effect. - crud.ts: add a postCommit hook that runs after the create transaction commits; correct the "pure DB writes" comment. - resources/destinations.ts: export projectDestinationsToSessions so wirings.ts reuses the exact same projection as destinations add/remove. - resources/wirings.ts: project the new destination to live sessions in postCommit. - crud.test.ts: cover projection with and without a running session.
This commit is contained in:
@@ -20,7 +20,17 @@ vi.mock('../log.js', () => ({
|
||||
log: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
|
||||
}));
|
||||
|
||||
// `wirings.ts`'s postCommit projects the new destination into every running
|
||||
// session's `inbound.db` via the agent-to-agent module's `writeDestinations`.
|
||||
// That helper opens on-disk session DB files we don't have in a unit test, so
|
||||
// mock it and observe that the projection is invoked per live session.
|
||||
const writeDestinationsSpy = vi.fn();
|
||||
vi.mock('../modules/agent-to-agent/write-destinations.js', () => ({
|
||||
writeDestinations: (...args: unknown[]) => writeDestinationsSpy(...args),
|
||||
}));
|
||||
|
||||
import { initTestDb, closeDb, runMigrations, createAgentGroup, createMessagingGroup } from '../db/index.js';
|
||||
import { createSession } from '../db/sessions.js';
|
||||
import { getContainerConfig } from '../db/container-configs.js';
|
||||
import { getDestinations } from '../modules/agent-to-agent/db/agent-destinations.js';
|
||||
import { lookup } from './registry.js';
|
||||
@@ -37,6 +47,7 @@ beforeEach(() => {
|
||||
const db = initTestDb();
|
||||
runMigrations(db);
|
||||
ensureContainerConfigSpy.mockClear();
|
||||
writeDestinationsSpy.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -95,3 +106,71 @@ describe('genericCreate postCreate hook', () => {
|
||||
expect(destinations[0].target_id).toBe('mg-1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('genericCreate postCommit hook', () => {
|
||||
it('wirings-create projects the new destination into live sessions', async () => {
|
||||
createAgentGroup({
|
||||
id: 'ag-1',
|
||||
name: 'Agent One',
|
||||
folder: 'agent-one',
|
||||
agent_provider: null,
|
||||
created_at: new Date().toISOString(),
|
||||
});
|
||||
createMessagingGroup({
|
||||
id: 'mg-1',
|
||||
channel_type: 'discord',
|
||||
platform_id: 'channel-123',
|
||||
name: 'general',
|
||||
is_group: 1,
|
||||
unknown_sender_policy: 'strict',
|
||||
created_at: new Date().toISOString(),
|
||||
});
|
||||
// A container is already running for this agent — its session inbound.db
|
||||
// holds a stale destination projection.
|
||||
createSession({
|
||||
id: 'sess-1',
|
||||
agent_group_id: 'ag-1',
|
||||
messaging_group_id: null,
|
||||
thread_id: null,
|
||||
agent_provider: null,
|
||||
status: 'active',
|
||||
container_status: 'running',
|
||||
last_active: new Date().toISOString(),
|
||||
created_at: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const cmd = lookup('wirings-create');
|
||||
await cmd!.handler({ messaging_group_id: 'mg-1', agent_group_id: 'ag-1' }, hostCtx);
|
||||
|
||||
// Live-refresh parity with `ncl destinations add`: without postCommit the
|
||||
// running container keeps serving the stale projection and drops replies
|
||||
// to this chat as "unknown destination" until a restart — issue #2389.
|
||||
expect(writeDestinationsSpy).toHaveBeenCalledWith('ag-1', 'sess-1');
|
||||
});
|
||||
|
||||
it('wirings-create projection is a no-op when no sessions are running', async () => {
|
||||
createAgentGroup({
|
||||
id: 'ag-2',
|
||||
name: 'Agent Two',
|
||||
folder: 'agent-two',
|
||||
agent_provider: null,
|
||||
created_at: new Date().toISOString(),
|
||||
});
|
||||
createMessagingGroup({
|
||||
id: 'mg-2',
|
||||
channel_type: 'discord',
|
||||
platform_id: 'channel-456',
|
||||
name: 'ops',
|
||||
is_group: 1,
|
||||
unknown_sender_policy: 'strict',
|
||||
created_at: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const cmd = lookup('wirings-create');
|
||||
await cmd!.handler({ messaging_group_id: 'mg-2', agent_group_id: 'ag-2' }, hostCtx);
|
||||
|
||||
// No live session for ag-2 → nothing to project, and the central
|
||||
// destination row was still written (covered above).
|
||||
expect(writeDestinationsSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
+21
-2
@@ -80,6 +80,22 @@ export interface ResourceDef {
|
||||
* inserted, so generated fields like `id` and `created_at` are populated.
|
||||
*/
|
||||
postCreate?: (row: Record<string, unknown>) => void;
|
||||
/**
|
||||
* Runs AFTER the create transaction has committed, with the row that was
|
||||
* written. Use this — not `postCreate` — for side effects that live
|
||||
* OUTSIDE the central DB (filesystem writes, projecting rows into a
|
||||
* running agent's session `inbound.db`) or that are async: those must not
|
||||
* sit inside the better-sqlite3 transaction, which only covers central-DB
|
||||
* statements and is synchronous.
|
||||
*
|
||||
* The canonical case is live-refresh parity with `ncl destinations add`:
|
||||
* after `ncl wirings create` writes the companion `agent_destinations`
|
||||
* row, the change has to be projected into any running container's session
|
||||
* DB or the agent won't see the new delivery target until its next spawn
|
||||
* (#2389). Runs only if the transaction succeeds, so it never observes a
|
||||
* rolled-back row.
|
||||
*/
|
||||
postCommit?: (row: Record<string, unknown>) => void | Promise<void>;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -170,13 +186,16 @@ function genericCreate(def: ResourceDef) {
|
||||
const placeholders = colNames.map((c) => `@${c}`);
|
||||
// Single transaction so a postCreate throw rolls back the parent INSERT —
|
||||
// closes the partial-state class this PR exists to fix (#2415, #2389).
|
||||
// better-sqlite3 .transaction() is sync, which matches postCreate's
|
||||
// signature (`(row) => void`); current hooks are pure DB writes.
|
||||
// better-sqlite3 .transaction() is sync, so `postCreate` is sync too and
|
||||
// must only touch the central DB (it's the atomic companion-row write).
|
||||
// Anything async or outside the central DB — filesystem, session-DB
|
||||
// projection — belongs in `postCommit`, which runs after commit below.
|
||||
const db = getDb();
|
||||
db.transaction(() => {
|
||||
db.prepare(`INSERT INTO ${def.table} (${colNames.join(', ')}) VALUES (${placeholders.join(', ')})`).run(values);
|
||||
if (def.postCreate) def.postCreate(values);
|
||||
})();
|
||||
if (def.postCommit) await def.postCommit(values);
|
||||
return values;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -9,13 +9,15 @@ import { registerResource } from '../crud.js';
|
||||
* on `hasTable('agent_destinations')` and load `writeDestinations` lazily —
|
||||
* same pattern as container-runner.ts on container wake.
|
||||
*
|
||||
* Called from both `add` and `remove` so the live container picks up the
|
||||
* change without waiting for the next spawn. Without this, send_message to
|
||||
* the new local_name silently drops with "unknown destination" until restart.
|
||||
* Called from every destination-mutating ncl command — `add` and `remove`
|
||||
* here, plus `wirings create` (which writes a companion destination row in
|
||||
* its postCreate hook) — so the live container picks up the change without
|
||||
* waiting for the next spawn. Without this, send_message to the new
|
||||
* local_name silently drops with "unknown destination" until restart.
|
||||
* See the destination-projection invariant in
|
||||
* src/modules/agent-to-agent/db/agent-destinations.ts.
|
||||
*/
|
||||
async function projectDestinationsToSessions(agentGroupId: string): Promise<void> {
|
||||
export async function projectDestinationsToSessions(agentGroupId: string): Promise<void> {
|
||||
if (!hasTable(getDb(), 'agent_destinations')) return;
|
||||
const { writeDestinations } = await import('../../modules/agent-to-agent/write-destinations.js');
|
||||
for (const session of getSessionsByAgentGroup(agentGroupId)) {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { ensureAgentDestinationForWiring } from '../../db/messaging-groups.js';
|
||||
import type { MessagingGroupAgent } from '../../types.js';
|
||||
import { registerResource } from '../crud.js';
|
||||
import { projectDestinationsToSessions } from './destinations.js';
|
||||
|
||||
registerResource({
|
||||
name: 'wiring',
|
||||
@@ -78,4 +79,16 @@ registerResource({
|
||||
// CRUD path doesn't, hence this hook. See issue #2389.
|
||||
ensureAgentDestinationForWiring(row as unknown as MessagingGroupAgent);
|
||||
},
|
||||
postCommit: async (row) => {
|
||||
// Live-refresh parity with `ncl destinations add`: `postCreate` above
|
||||
// only wrote the central `agent_destinations` row. Any container already
|
||||
// running for this agent keeps serving its stale session projection, so
|
||||
// it would drop replies to this chat as "unknown destination" until the
|
||||
// next spawn (the exact symptom operators hit running `ncl wirings
|
||||
// create` against a live instance — it needed a group restart). Project
|
||||
// the new destination into live sessions now so the fix takes effect
|
||||
// without a restart. Runs after commit because it writes to session
|
||||
// `inbound.db` files (outside the central-DB transaction) and is async.
|
||||
await projectDestinationsToSessions((row as unknown as MessagingGroupAgent).agent_group_id);
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user