Skip to content

fix(cli): ncl groups delete cascades + auto-IDs survive OneCLI validation#2540

Open
abarbaccia wants to merge 1 commit into
nanocoai:mainfrom
abarbaccia:fix/ncl-groups-cascade-and-id-prefix
Open

fix(cli): ncl groups delete cascades + auto-IDs survive OneCLI validation#2540
abarbaccia wants to merge 1 commit into
nanocoai:mainfrom
abarbaccia:fix/ncl-groups-cascade-and-id-prefix

Conversation

@abarbaccia
Copy link
Copy Markdown

Summary

Two coupled bugs that surface together the moment you create a brand-new agent group via `ncl groups create` and try to clean it up.

1. `ncl groups delete` always fails with FOREIGN KEY constraint failed

`genericDelete` in `src/cli/crud.ts` ran `DELETE FROM ${table}` directly. That's fine for leaf rows, but `agent_groups` has FK references with no `ON DELETE CASCADE` from `sessions`, `messaging_group_agents`, `agent_group_members`, `user_roles`, `agent_destinations`, and a few pending-approval tables. Result: once a session row exists for the group, `ncl groups delete` errors forever and there's no `ncl` verb to clean it up.

Fix:

  • New optional `customDelete?: (id: string) => number` hook on `ResourceDef`.
  • `groups` resource wires it to `deleteAgentGroup`, which walks every dependent table in a single transaction (delete sessions/wirings/members/roles/destinations/pending sender + channel approvals; null-out `pending_approvals.agent_group_id`; then delete the group).
  • `genericDelete` falls back to the raw SQL when `customDelete` is not set — every other resource is unchanged.

2. Auto-generated agent group IDs are rejected by OneCLI

`genericCreate` generates IDs via `randomUUID()`. UUIDs start with a digit ~62% of the time. OneCLI's `POST /api/agents` validates `identifier` against `^[a-z][a-z0-9-]{0,49}$` and returns 400 Bad Request. The host's first wakeContainer call then fails forever:
```
OneCLIRequestError: [URL=…/api/agents] [StatusCode=400] OneCLI returned 400 Bad Request
```
Repro: `./bin/ncl groups create --name Foo --folder foo` — any UUID starting with a digit. Symptom is silent — the host just keeps retrying on every sweep.

Fix:

  • New optional `idPrefix?: string` on `ResourceDef`. Auto-generated IDs become `${idPrefix}${randomUUID()}`.
  • `groups` resource sets `idPrefix: 'ag-'` so auto-IDs are always OneCLI-safe.
  • `genericCreate` now also honors an explicit `--id` if the caller provides one (previously `generated: true` silently dropped user input), so you can pass `--id ag-shopping` for a readable identifier.

Test plan

  • Added `cascades through sessions and wirings on delete` test — creates an agent group, messaging group, wiring, and session; calls `deleteAgentGroup`; asserts they're all gone.
  • `pnpm exec vitest run src/db/db-v2.test.ts` — 33/33 pass.
  • `pnpm run build` — clean tsc.
  • Manual end-to-end:
    • `ncl groups create --id ag-shopping --name "Homey - Shopping" --folder shopping` → group created with the literal ID
    • Wired a WhatsApp messaging group, host spawned the container on the first message (no more 400 from OneCLI).
    • `ncl groups delete --id ` on a group with active sessions + wiring → clean delete, no FK error.

🤖 Generated with Claude Code

…dation

Two coupled fixes for ncl groups create/delete:

1. `customDelete` hook on ResourceDef. genericDelete previously ran
   `DELETE FROM ${table}` directly, which is fine for leaf resources
   but breaks on agent_groups — sessions, wirings, members, roles,
   destinations, and a few approval tables all hold a FK with no ON
   DELETE CASCADE. The cascade now happens in deleteAgentGroup() inside
   a single transaction. genericDelete falls back to the raw SQL when
   no customDelete is set, so every other resource is unchanged.

2. `idPrefix` on ResourceDef + accept explicit --id on create.
   randomUUID() starts with a digit ~62% of the time. OneCLI rejects
   any agent identifier that doesn't match ^[a-z][a-z0-9-]{0,49}$, so
   every freshly-created group via `ncl groups create` failed forever
   on the first wakeContainer with HTTP 400 Bad Request. Groups now
   carry idPrefix='ag-' so auto-IDs are always OneCLI-safe, and
   callers can pass --id explicitly when they want a readable identifier.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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