fix: sanitize OneCLI agent identifiers

Author: CopyPasteFail

src/container-runner.ts

@@ -23,6 +23,7 @@ import { readContainerConfig, writeContainerConfig } from './container-config.js
 import { CONTAINER_RUNTIME_BIN, hostGatewayArgs, readonlyMountArgs, stopContainer } from './container-runtime.js';
 import { composeGroupClaudeMd } from './claude-md-compose.js';
 import { getAgentGroup } from './db/agent-groups.js';
+import { toOneCliAgentIdentifier } from './onecli-agent-identifier.js';
 import { getDb, hasTable } from './db/connection.js';
 import { initGroupFilesystem } from './group-init.js';
 import { stopTypingRefresh } from './modules/typing/index.js';
@@ -134,9 +135,9 @@ async function spawnContainer(session: Session): Promise<void> {
 
   const mounts = buildMounts(agentGroup, session, containerConfig, contribution);
   const containerName = `nanoclaw-v2-${agentGroup.folder}-${Date.now()}`;
-  // OneCLI agent identifier is always the agent group id — stable across
-  // sessions and reversible via getAgentGroup() for approval routing.
-  const agentIdentifier = agentGroup.id;
+  // OneCLI agent identifier is the stable agent group id normalized to
+  // OneCLI's identifier format.
+  const agentIdentifier = toOneCliAgentIdentifier(agentGroup.id);
   const args = await buildContainerArgs(
     mounts,
     containerName,

src/modules/approvals/onecli-approvals.ts

@@ -21,7 +21,7 @@ import { OneCLI, type ApprovalRequest, type ManualApprovalHandle } from '@onecli
 
 import { pickApprovalDelivery, pickApprover } from './primitive.js';
 import { ONECLI_API_KEY, ONECLI_URL } from '../../config.js';
-import { getAgentGroup } from '../../db/agent-groups.js';
+import { resolveAgentGroupByOneCliIdentifier } from '../../onecli-agent-identifier.js';
 import {
   createPendingApproval,
   deletePendingApproval,
@@ -114,9 +114,15 @@ async function handleRequest(request: ApprovalRequest): Promise<Decision> {
   if (!adapterRef) return 'deny';
 
   // Originating agent group is carried on the request via OneCLI's agent
-  // identifier (set by container-runner.ts to agentGroup.id). Use it as
-  // the scope for approver selection: admin @ group → global admin → owner.
-  const originGroup = request.agent.externalId ? getAgentGroup(request.agent.externalId) : undefined;
+  // identifier. Resolve it back to the database id for scoped approver selection:
+  // admin @ group → global admin → owner.
+  const originGroup = request.agent.externalId ? resolveAgentGroupByOneCliIdentifier(request.agent.externalId) : undefined;
+  if (request.agent.externalId && !originGroup) {
+    log.warn('OneCLI approval agent identifier did not resolve to a unique agent group', {
+      id: request.id,
+      agent: request.agent.externalId,
+    });
+  }
   const agentGroupId = originGroup?.id ?? null;
   const approvers = pickApprover(agentGroupId);
   if (approvers.length === 0) {

src/onecli-agent-identifier.test.ts

@@ -0,0 +1,61 @@
+import { beforeEach, describe, expect, it } from 'vitest';
+import { createAgentGroup } from './db/agent-groups.js';
+import { closeDb, initTestDb, runMigrations } from './db/index.js';
+import { resolveAgentGroupByOneCliIdentifier, toOneCliAgentIdentifier } from './onecli-agent-identifier.js';
+
+describe('OneCLI agent identifiers', () => {
+  beforeEach(() => {
+    closeDb();
+    const db = initTestDb();
+    runMigrations(db);
+  });
+
+  it('converts agent group IDs from underscores to hyphens', () => {
+    expect(toOneCliAgentIdentifier('ag_f249a3521081')).toBe('ag-f249a3521081');
+  });
+
+  it('converts all underscores and satisfies OneCLI validation', () => {
+    const identifier = toOneCliAgentIdentifier('ag_foo_bar');
+
+    expect(identifier).toBe('ag-foo-bar');
+    expect(identifier).toMatch(/^[a-z0-9-]+$/);
+  });
+
+  it('resolves a normalized OneCLI identifier back to the database agent group', () => {
+    createAgentGroup({
+      id: 'ag_f249a3521081',
+      name: 'Test Agent',
+      folder: 'test-agent',
+      agent_provider: 'claude',
+      created_at: new Date().toISOString(),
+    });
+
+    expect(resolveAgentGroupByOneCliIdentifier('ag-f249a3521081')?.id).toBe('ag_f249a3521081');
+  });
+
+  it('throws when normalization still produces an invalid OneCLI identifier', () => {
+    expect(() => toOneCliAgentIdentifier('ag_Foo')).toThrow(/invalid OneCLI identifier/);
+    expect(() => toOneCliAgentIdentifier('ag/foo')).toThrow(/invalid OneCLI identifier/);
+    expect(() => toOneCliAgentIdentifier('')).toThrow(/invalid OneCLI identifier/);
+  });
+
+  it('does not resolve ambiguous normalized identifiers', () => {
+    createAgentGroup({
+      id: 'ag_foo',
+      name: 'Underscore Agent',
+      folder: 'underscore-agent',
+      agent_provider: 'claude',
+      created_at: new Date().toISOString(),
+    });
+
+    createAgentGroup({
+      id: 'ag-foo',
+      name: 'Hyphen Agent',
+      folder: 'hyphen-agent',
+      agent_provider: 'claude',
+      created_at: new Date().toISOString(),
+    });
+
+    expect(resolveAgentGroupByOneCliIdentifier('ag-foo')).toBeUndefined();
+  });
+});

src/onecli-agent-identifier.ts

@@ -0,0 +1,18 @@
+import { getAllAgentGroups } from './db/agent-groups.js';
+
+const ONECLI_AGENT_IDENTIFIER_PATTERN = /^[a-z0-9-]+$/;
+
+export function toOneCliAgentIdentifier(agentGroupId: string): string {
+  const identifier = agentGroupId.replace(/_/g, '-');
+
+  if (!ONECLI_AGENT_IDENTIFIER_PATTERN.test(identifier)) {
+    throw new Error(`Agent group ID "${agentGroupId}" produces an invalid OneCLI identifier: "${identifier}"`);
+  }
+
+  return identifier;
+}
+
+export function resolveAgentGroupByOneCliIdentifier(identifier: string) {
+  const matches = getAllAgentGroups().filter((agentGroup) => toOneCliAgentIdentifier(agentGroup.id) === identifier);
+  return matches.length === 1 ? matches[0] : undefined;
+}