fix(harness): address code-review findings (correctness, security honesty, altitude)

Correctness:
- reconciler + PreCompact hook unified into one read-modify-write with a
  single atomic write, and the parsed value is shape-validated: a settings.json
  holding valid-JSON-non-object (null, a scalar, {"env":"bad"}) no longer
  throws a TypeError that bricked the group — it warns and skips, like the old
  hook. Fixes divergent atomic/non-atomic writers and a double read per spawn.
- config update wraps both DB writes in one transaction — a failing scalar
  update can no longer leave the harness override half-applied.
- parseHarnessCapabilitiesArg returns a single key->directive map, so a repeated
  key resolves last-wins (was: clears always beat sets regardless of order).
- validation uses Object.hasOwn, not `key in` — inherited names like
  `constructor`/`toString` are no longer accepted as capabilities.
- createContainerConfig now binds cli_scope + harness_capabilities (were
  silently dropped to the column default).

Security honesty:
- agent-teams=off is documented as spawn-time hygiene, not a hard in-container
  boundary: settings.json is RW-mounted so an agent can rewrite it for the
  container lifetime. workflow=off keeps its disallowedTools backstop. Follow-up
  filed to mount the managed settings source read-only.
- dispatch escalation gate protects canonical field names and normalizes each
  incoming arg key — no spelling-variant bypass.

Altitude / cleanup:
- capabilities carry their host mechanism in the registry; the reconciler
  iterates it, so adding a key is one registry entry (+ one runner map line if
  it blocks a tool). Runner unknown-key lockstep + warning loop deleted (host
  drops unknown keys at resolve; warns once per distinct problem, not per spawn).
- resolved view is structured {state, source}; source is 'override' only for a
  VALID stored override (an invalid value reports 'default', not a lie).
- ReportFindings joins the fixed disallow list (headless has no UI to receive
  it; ~1.9KB/turn). PreToolUse hook built once in the constructor with a Set.
- drift guard: assert the installed SDK version too (SDK now pinned exactly),
  and dual-capture the fixture (tools vs toolsBare). This EMPIRICALLY corrected
  a prior claim: allowedTools is fully inert here (surfaces are byte-identical),
  it does not promote Glob/Grep — comment fixed to match.

Host 663 + container 125 green; wire re-probed (default drops
Workflow/DesignSync/ReportFindings; workflow=on restores only Workflow).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Gabi Simons
2026-07-09 10:35:29 +03:00
parent 7a8bd34b41
commit 49ef2ef24e
11 changed files with 341 additions and 274 deletions
@@ -1,13 +1,12 @@
/**
* Harness-capability mapping in the Claude provider: the disallow list is the
* fixed set plus capability-driven entries (fail closed), the PreToolUse hook
* blocks exactly that list, and unknown resolved keys warn once at
* construction. Pure — no SDK, no DB (the hook's container_state write is
* try/caught by design).
* fixed set plus capability-driven entries (fail closed), and the PreToolUse
* hook blocks exactly that list. Pure — no SDK, no DB (the hook's
* container_state write is try/caught by design).
*/
import { afterEach, describe, expect, it, spyOn } from 'bun:test';
import { describe, expect, it } from 'bun:test';
import { ClaudeProvider, SDK_DISALLOWED_TOOLS, buildDisallowedTools, createPreToolUseHook } from './claude.js';
import { SDK_DISALLOWED_TOOLS, buildDisallowedTools, createPreToolUseHook } from './claude.js';
type LooseHook = (input: unknown) => Promise<Record<string, unknown>>;
@@ -18,6 +17,7 @@ describe('buildDisallowedTools', () => {
for (const fixed of SDK_DISALLOWED_TOOLS) expect(list).toContain(fixed);
expect(list).toContain('Workflow');
expect(list).toContain('DesignSync');
expect(list).toContain('ReportFindings');
}
});
@@ -47,19 +47,3 @@ describe('createPreToolUseHook', () => {
expect(res.continue).toBe(true);
});
});
describe('ClaudeProvider constructor capability handling', () => {
const errSpy = spyOn(console, 'error');
afterEach(() => {
errSpy.mockClear();
});
it('warns once per unknown key, never for the known pair', () => {
new ClaudeProvider({ harnessCapabilities: { 'agent-teams': 'off', workflow: 'on', monitor: 'off' } });
const calls = errSpy.mock.calls.map((c) => String(c[0]));
expect(calls.filter((m) => m.includes('Unknown harness capability'))).toHaveLength(1);
expect(calls.some((m) => m.includes('monitor'))).toBe(true);
expect(calls.some((m) => m.includes("'agent-teams'") || m.includes("'workflow'"))).toBe(false);
});
});
+33 -17
View File
@@ -26,6 +26,8 @@ function log(msg: string): void {
// Code UI affordances; in a headless container they'd appear stuck.
// - DesignSync: desktop design-tool integration — nothing to sync with in a
// headless container, and a ~9.3KB/turn schema (wire-measured).
// - ReportFindings: code-review-reporting UI affordance with no headless host
// surface to receive it (~1.9KB/turn schema).
export const SDK_DISALLOWED_TOOLS = [
'CronCreate',
'CronDelete',
@@ -37,20 +39,33 @@ export const SDK_DISALLOWED_TOOLS = [
'EnterWorktree',
'ExitWorktree',
'DesignSync',
'ReportFindings',
];
/**
* Effective disallow list for a provider instance: the fixed set plus
* capability-driven entries from the group's resolved harness-capability map
* (container.json, resolved host-side — see src/harness-capabilities.ts on
* the host). `workflow` off — the default — adds `Workflow` as
* defense-in-depth: the primary OFF mechanism is the host-reconciled
* `disableWorkflows` settings key, and this backstop covers a malformed or
* hand-reverted settings.json. Absent or unknown state fails closed.
* Configurable capabilities that block a harness tool when OFF, mapped to the
* tool name. This is the runner half of the capability contract (the host half
* is the settings reconciler); it is genuinely runner-specific because
* `disallowedTools` is a Claude concept the host tree can't name. The block is
* defense-in-depth the primary OFF mechanism is the host-reconciled settings
* key — covering a malformed or hand-reverted settings.json.
*/
const CAPABILITY_DISALLOWS: Record<string, string> = {
workflow: 'Workflow',
};
/**
* Effective disallow list for a provider instance: the fixed set plus, for each
* capability in CAPABILITY_DISALLOWS whose resolved state is not explicitly
* `on`, its tool. Absent or unexpected state fails closed (tool stays blocked).
* `caps` is the resolved map from container.json (host-resolved: only known
* keys, valid values).
*/
export function buildDisallowedTools(caps?: Record<string, string>): string[] {
const list = [...SDK_DISALLOWED_TOOLS];
if (caps?.workflow !== 'on') list.push('Workflow');
for (const [key, tool] of Object.entries(CAPABILITY_DISALLOWS)) {
if (caps?.[key] !== 'on') list.push(tool);
}
return list;
}
@@ -182,11 +197,12 @@ function formatTranscriptMarkdown(messages: ParsedMessage[], title?: string | nu
* (like createPreCompactHook) because the blocklist is per-instance — it
* depends on the group's resolved harness capabilities.
*/
export function createPreToolUseHook(blockedTools: string[]): HookCallback {
export function createPreToolUseHook(blockedTools: Iterable<string>): HookCallback {
const blocked = new Set(blockedTools);
return async (input) => {
const i = input as { tool_name?: string; tool_input?: Record<string, unknown> };
const toolName = i.tool_name ?? '';
if (blockedTools.includes(toolName)) {
if (blocked.has(toolName)) {
return {
decision: 'block',
stopReason: `Tool '${toolName}' is not available in this environment — use the nanoclaw equivalent.`,
@@ -366,6 +382,7 @@ export class ClaudeProvider implements AgentProvider {
private model?: string;
private effort?: string;
private disallowedTools: string[];
private preToolUseHook: HookCallback;
constructor(options: ProviderOptions = {}) {
this.assistantName = options.assistantName;
@@ -377,13 +394,12 @@ export class ClaudeProvider implements AgentProvider {
...(options.env ?? {}),
CLAUDE_CODE_AUTO_COMPACT_WINDOW,
};
// The resolved harness-capability map in container.json is host-resolved:
// only known keys with valid values reach here, so the runner never has to
// reason about unknown keys. Blocklist + hook are per-instance but immutable
// after construction — build both once.
this.disallowedTools = buildDisallowedTools(options.harnessCapabilities);
// 'agent-teams' is host-settings-managed — no runner mechanism, but it is
// a known key; warning on it would fire for every group on every boot.
const knownKeys = new Set(['agent-teams', 'workflow']);
for (const key of Object.keys(options.harnessCapabilities ?? {})) {
if (!knownKeys.has(key)) log(`Unknown harness capability key (no claude-provider mechanism): ${key}`);
}
this.preToolUseHook = createPreToolUseHook(this.disallowedTools);
}
isSessionInvalid(err: unknown): boolean {
@@ -454,7 +470,7 @@ export class ClaudeProvider implements AgentProvider {
settingSources: ['project', 'user', 'local'],
mcpServers: this.mcpServers,
hooks: {
PreToolUse: [{ hooks: [createPreToolUseHook(this.disallowedTools)] }],
PreToolUse: [{ hooks: [this.preToolUseHook] }],
PostToolUse: [{ hooks: [postToolUseHook] }],
PostToolUseFailure: [{ hooks: [postToolUseHook] }],
PreCompact: [{ hooks: [createPreCompactHook(this.assistantName)] }],
+6 -1
View File
@@ -23,7 +23,12 @@ ncl groups config update --id <group-id> --harness-capabilities 'workflow=on,age
ncl groups restart --id <group-id> # apply
```
`default` clears the per-group override (it is never stored). Changes from inside a container are rejected — like `cli_scope`, this is operator-only.
`default` clears the per-group override (it is never stored). Config changes through `ncl` from inside a container are rejected — like `cli_scope`, the sanctioned/persistent path is operator-only.
### Enforcement strength (be precise about the boundary)
- **`workflow` off** has two independent locks: the reconciled `disableWorkflows` settings key *and* a runner-side `disallowedTools` block. The tool cannot come back inside a running container even if the settings file is edited.
- **`agent-teams` off** has only one mechanism: the absence of the env key from the group's `settings.json`. That file is mounted **read-write** into the container (the CLI needs to write transcripts there), and `settingSources` also loads project/local settings from the agent-writable workspace. So an agent that actively rewrites its own settings can re-enable teams **for the current container lifetime**, until the next spawn re-reconciles it. Treat `agent-teams=off` as **configuration hygiene enforced at spawn**, not a hard adversarial boundary inside a live container — the real trust boundary remains the container sandbox + OneCLI. A follow-up ([nanocoai/nanoclaw#TBD](https://github.com/nanocoai/nanoclaw/issues)) will mount the managed settings source read-only to close this.
## [BREAKING] Agent teams default off
+1 -1
View File
@@ -222,7 +222,7 @@ function syncSymlink(linkPath: string, target: string): void {
fs.symlinkSync(target, linkPath);
}
function writeAtomic(filePath: string, content: string): void {
export function writeAtomic(filePath: string, content: string): void {
const tmp = `${filePath}.tmp-${process.pid}`;
fs.writeFileSync(tmp, content);
fs.renameSync(tmp, filePath);
+11 -10
View File
@@ -87,16 +87,17 @@ export async function dispatch(
return err(req.id, 'forbidden', 'CLI access is scoped to this agent group.');
}
// Block cli_scope changes from group-scoped agents (privilege escalation)
if (req.args.cli_scope !== undefined || req.args['cli-scope'] !== undefined) {
return err(req.id, 'forbidden', 'Cannot change cli_scope from a group-scoped agent.');
}
// Same escalation shape for harness capabilities: an agent must not be
// able to re-enable harness features (agent teams, Workflow) that the
// operator turned off for its group.
if (req.args.harness_capabilities !== undefined || req.args['harness-capabilities'] !== undefined) {
return err(req.id, 'forbidden', 'Cannot change harness_capabilities from a group-scoped agent.');
// Operator-only config fields — block changes from group-scoped agents
// (privilege escalation: cli_scope widens CLI access, harness
// capabilities re-enable harness features the operator turned off). This
// gate runs on RAW args before normalizeArgs, so we normalize each
// incoming key to canonical snake_case and compare — one spelling per
// protected field, no spelling-variant bypass. Add future fields HERE.
const OPERATOR_ONLY_ARGS = new Set(['cli_scope', 'harness_capabilities']);
for (const key of Object.keys(req.args)) {
if (OPERATOR_ONLY_ARGS.has(key.replace(/-/g, '_'))) {
return err(req.id, 'forbidden', `Cannot change ${key.replace(/-/g, '_')} from a group-scoped agent.`);
}
}
// Auto-fill agent-group-related args so the agent doesn't need
+25 -11
View File
@@ -23,7 +23,7 @@ vi.mock('../../config.js', async () => {
const TEST_DIR = '/tmp/nanoclaw-test-cli-groups-config';
import { initTestDb, closeDb, runMigrations, createAgentGroup } from '../../db/index.js';
import { ensureContainerConfig, getContainerConfig } from '../../db/container-configs.js';
import { ensureContainerConfig, getContainerConfig, updateContainerConfigJson } from '../../db/container-configs.js';
import { dispatch } from '../dispatch.js';
// Side-effect import: registers the `groups-*` commands.
import './groups.js';
@@ -58,30 +58,32 @@ afterEach(() => {
});
describe('groups config update --harness-capabilities', () => {
it('sets an override and renders it as (override) in the resolved view', async () => {
type Resolved = Record<string, { state: string; source: string }>;
it('sets an override and marks it as an override in the resolved view', async () => {
const resp = await configUpdate('agent-teams=on');
expect(resp.ok).toBe(true);
if (resp.ok) {
const data = resp.data as {
harness_capabilities: Record<string, string>;
harness_capabilities_resolved: Record<string, string>;
harness_capabilities_resolved: Resolved;
};
expect(data.harness_capabilities).toEqual({ 'agent-teams': 'on' });
expect(data.harness_capabilities_resolved['agent-teams']).toBe('on (override)');
expect(data.harness_capabilities_resolved.workflow).toBe('off (default)');
expect(data.harness_capabilities_resolved['agent-teams']).toEqual({ state: 'on', source: 'override' });
expect(data.harness_capabilities_resolved.workflow).toEqual({ state: 'off', source: 'default' });
}
expect(JSON.parse(getContainerConfig(GID)!.harness_capabilities)).toEqual({ 'agent-teams': 'on' });
});
it('`default` clears the override and the resolved view returns to (default)', async () => {
it('`default` clears the override and the resolved view returns to default', async () => {
await configUpdate('agent-teams=on');
const resp = await configUpdate('agent-teams=default');
expect(resp.ok).toBe(true);
if (resp.ok) {
const data = resp.data as { harness_capabilities_resolved: Record<string, string> };
expect(data.harness_capabilities_resolved['agent-teams']).toBe('off (default)');
const data = resp.data as { harness_capabilities_resolved: Resolved };
expect(data.harness_capabilities_resolved['agent-teams']).toEqual({ state: 'off', source: 'default' });
}
expect(JSON.parse(getContainerConfig(GID)!.harness_capabilities)).toEqual({});
});
@@ -109,13 +111,25 @@ describe('groups config update --harness-capabilities', () => {
if (resp.ok) {
const data = resp.data as {
harness_capabilities: Record<string, string>;
harness_capabilities_resolved: Record<string, string>;
harness_capabilities_resolved: Resolved;
};
expect(data.harness_capabilities).toEqual({ workflow: 'on' });
expect(data.harness_capabilities_resolved).toEqual({
'agent-teams': 'off (default)',
workflow: 'on (override)',
'agent-teams': { state: 'off', source: 'default' },
workflow: { state: 'on', source: 'override' },
});
}
});
it('a stored invalid value reports source=default, not a lying override', async () => {
// Direct DB write bypassing validation (version skew / manual edit).
updateContainerConfigJson(GID, 'harness_capabilities', { workflow: 'sideways' });
const resp = await dispatch({ id: 't', command: 'groups-config-get', args: { id: GID } }, hostCtx);
expect(resp.ok).toBe(true);
if (resp.ok) {
const data = resp.data as { harness_capabilities_resolved: Resolved };
expect(data.harness_capabilities_resolved.workflow).toEqual({ state: 'off', source: 'default' });
}
});
});
+35 -29
View File
@@ -14,25 +14,19 @@ import {
} from '../../db/container-configs.js';
import { initGroupFilesystem } from '../../group-init.js';
import { createAgentFromTemplate } from '../../templates/create-agent.js';
import { parseHarnessCapabilitiesArg, resolveHarnessCapabilities } from '../../harness-capabilities.js';
import {
HARNESS_CAPABILITY_KEYS,
parseHarnessCapabilitiesArg,
parseHarnessOverrides,
resolveHarnessCapabilities,
} from '../../harness-capabilities.js';
import type { AgentGroup, ContainerConfigRow } from '../../types.js';
import { registerResource } from '../crud.js';
/** Raw override map from the stored JSON; malformed degrades to {} (resolve warns). */
function harnessOverrides(row: ContainerConfigRow): Record<string, unknown> {
try {
const parsed: unknown = JSON.parse(row.harness_capabilities);
return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? (parsed as Record<string, unknown>) : {};
} catch (err) {
if (!(err instanceof SyntaxError)) throw err;
return {};
}
}
/** Deserialize JSON columns for display. */
function presentConfig(row: ContainerConfigRow): Record<string, unknown> {
const overrides = harnessOverrides(row);
const resolved = resolveHarnessCapabilities(row.harness_capabilities);
const overrides = parseHarnessOverrides(row.harness_capabilities);
const resolved = resolveHarnessCapabilities(overrides);
return {
agent_group_id: row.agent_group_id,
provider: row.provider,
@@ -48,8 +42,15 @@ function presentConfig(row: ContainerConfigRow): Record<string, unknown> {
additional_mounts: JSON.parse(row.additional_mounts),
cli_scope: row.cli_scope,
harness_capabilities: overrides,
// Structured {state, source} per key — machine-readable and honest: source
// is 'override' only when a VALID override is stored (an invalid stored
// value resolves to the default, so it reports 'default', not a lie).
harness_capabilities_resolved: Object.fromEntries(
Object.entries(resolved).map(([k, v]) => [k, `${v} (${k in overrides ? 'override' : 'default'})`]),
Object.entries(resolved).map(([k, state]) => {
const ov = (overrides as Record<string, unknown>)[k];
const source = ov === 'on' || ov === 'off' ? 'override' : 'default';
return [k, { state, source }];
}),
),
updated_at: row.updated_at,
};
@@ -272,7 +273,7 @@ registerResource({
description:
'Update container config fields. Changes are saved but do NOT take effect until you run `ncl groups restart`. ' +
'Use --id <group-id> and any of: --provider, --model, --effort, --image-tag, --assistant-name, --max-messages-per-prompt, --cli-scope, ' +
"--harness-capabilities 'key=on|off|default[,key=...]' (keys: agent-teams, workflow; `default` clears the per-group override).",
`--harness-capabilities 'key=on|off|default[,key=...]' (keys: ${HARNESS_CAPABILITY_KEYS.join(', ')}; \`default\` clears the per-group override).`,
handler: async (args) => {
const id = args.id as string;
if (!id) throw new Error('--id is required');
@@ -300,26 +301,31 @@ registerResource({
updates.cli_scope = scope;
}
// Harness capabilities are a JSON column of sparse overrides — apply
// set/clear ops on the stored map (`default` clears; never stored).
let harnessChanged = false;
const capsArg = (args['harness-capabilities'] ?? args.harness_capabilities) as string | undefined;
if (capsArg !== undefined) {
const ops = parseHarnessCapabilitiesArg(String(capsArg));
const overrides = harnessOverrides(row);
for (const key of ops.clear) delete overrides[key];
Object.assign(overrides, ops.set);
updateContainerConfigJson(id, 'harness_capabilities', overrides);
harnessChanged = true;
// Harness capabilities are a sparse-override JSON column. parse returns
// a key→directive map (last-wins per key); apply each: `default` clears
// the override, on/off set it.
let nextOverrides: Record<string, unknown> | undefined;
if (args.harness_capabilities !== undefined) {
const directives = parseHarnessCapabilitiesArg(String(args.harness_capabilities));
nextOverrides = parseHarnessOverrides(row.harness_capabilities);
for (const [key, directive] of Object.entries(directives)) {
if (directive === 'default') delete nextOverrides[key];
else nextOverrides[key] = directive;
}
}
if (Object.keys(updates).length === 0 && !harnessChanged) {
if (Object.keys(updates).length === 0 && nextOverrides === undefined) {
throw new Error(
'Nothing to update — provide at least one of: --provider, --model, --effort, --image-tag, --assistant-name, --max-messages-per-prompt, --cli-scope, --harness-capabilities',
);
}
if (Object.keys(updates).length > 0) updateContainerConfigScalars(id, updates);
// Single transaction: a failure in either write (e.g. a bad scalar
// value) rolls back both, so the command can never half-apply.
getDb().transaction(() => {
if (nextOverrides !== undefined) updateContainerConfigJson(id, 'harness_capabilities', nextOverrides);
if (Object.keys(updates).length > 0) updateContainerConfigScalars(id, updates);
})();
const updated = getContainerConfig(id)!;
return presentConfig(updated);
+2 -2
View File
@@ -36,11 +36,11 @@ export function createContainerConfig(config: ContainerConfigRow): void {
`INSERT INTO container_configs (
agent_group_id, provider, model, effort, image_tag, assistant_name,
max_messages_per_prompt, skills, mcp_servers, packages_apt, packages_npm,
additional_mounts, updated_at
additional_mounts, cli_scope, harness_capabilities, updated_at
) VALUES (
@agent_group_id, @provider, @model, @effort, @image_tag, @assistant_name,
@max_messages_per_prompt, @skills, @mcp_servers, @packages_apt, @packages_npm,
@additional_mounts, @updated_at
@additional_mounts, @cli_scope, @harness_capabilities, @updated_at
)`,
)
.run(config);
+97 -110
View File
@@ -1,39 +1,30 @@
import fs from 'fs';
import path from 'path';
import { writeAtomic } from './claude-md-compose.js';
import { DATA_DIR, GROUPS_DIR } from './config.js';
import { ensureContainerConfig } from './db/container-configs.js';
import { HARNESS_CAPABILITIES } from './harness-capabilities.js';
import { log } from './log.js';
import { providerProvidesAgentSurfaces } from './providers/provider-container-registry.js';
import type { HarnessCapabilityState } from './harness-capabilities.js';
import type { AgentGroup } from './types.js';
// Managed harness keys (the teams env key, disableWorkflows) are deliberately
// NOT in the static default — they enter settings.json exclusively through
// reconcileHarnessSettings() from the group's resolved capability state.
const DEFAULT_SETTINGS_JSON =
JSON.stringify(
{
env: {
CLAUDE_CODE_ADDITIONAL_DIRECTORIES_CLAUDE_MD: '1',
CLAUDE_CODE_DISABLE_AUTO_MEMORY: '0',
},
hooks: {
PreCompact: [
{
hooks: [
{
type: 'command',
command: 'bun /app/src/compact-instructions.ts',
},
],
},
],
},
},
null,
2,
) + '\n';
const PRE_COMPACT_COMMAND = 'bun /app/src/compact-instructions.ts';
// Base settings for a brand-new group. Managed harness keys (the teams env key,
// disableWorkflows) are deliberately NOT here — they enter settings.json
// exclusively through the reconciler from the group's resolved capability state,
// applied on top of this base in the same write.
const DEFAULT_SETTINGS = {
env: {
CLAUDE_CODE_ADDITIONAL_DIRECTORIES_CLAUDE_MD: '1',
CLAUDE_CODE_DISABLE_AUTO_MEMORY: '0',
},
hooks: {
PreCompact: [{ hooks: [{ type: 'command', command: PRE_COMPACT_COMMAND }] }],
},
};
/**
* Initialize the on-disk filesystem state for an agent group. Idempotent
@@ -127,19 +118,7 @@ export function initGroupFilesystem(
}
const settingsFile = path.join(claudeDir, 'settings.json');
if (!fs.existsSync(settingsFile)) {
fs.writeFileSync(settingsFile, DEFAULT_SETTINGS_JSON);
initialized.push('settings.json');
} else {
ensurePreCompactHook(settingsFile, initialized);
}
// Runs in BOTH branches: a freshly written default must be reconciled in
// the same call, or the group's first container lifetime ships without
// its capability state (the default file deliberately omits managed keys).
if (opts?.harnessCapabilities) {
reconcileHarnessSettings(settingsFile, opts.harnessCapabilities, initialized);
}
applyGroupSettings(settingsFile, opts?.harnessCapabilities, initialized);
// Skills directory — created empty here; symlinks are synced at spawn
// time by container-runner.ts based on container.json skills selection.
@@ -160,87 +139,95 @@ export function initGroupFilesystem(
}
}
const PRE_COMPACT_COMMAND = 'bun /app/src/compact-instructions.ts';
type SettingsObject = Record<string, unknown>;
/**
* Patch an existing settings.json to add the PreCompact hook if missing.
* Runs on every group init so pre-existing groups pick up the hook.
*/
function ensurePreCompactHook(settingsFile: string, initialized: string[]): void {
try {
const raw = fs.readFileSync(settingsFile, 'utf-8');
const settings = JSON.parse(raw);
// Check if there's already a PreCompact hook with our command.
const existing = settings.hooks?.PreCompact as unknown[] | undefined;
if (existing && JSON.stringify(existing).includes(PRE_COMPACT_COMMAND)) return;
// Add the hook, preserving existing hooks.
if (!settings.hooks) settings.hooks = {};
if (!settings.hooks.PreCompact) settings.hooks.PreCompact = [];
settings.hooks.PreCompact.push({
hooks: [{ type: 'command', command: PRE_COMPACT_COMMAND }],
});
fs.writeFileSync(settingsFile, JSON.stringify(settings, null, 2) + '\n');
initialized.push('settings.json (added PreCompact hook)');
} catch {
// Don't break init if settings.json is malformed — it'll use whatever's there.
}
function asObject(value: unknown): SettingsObject | null {
return value && typeof value === 'object' && !Array.isArray(value) ? (value as SettingsObject) : null;
}
const TEAMS_ENV_KEY = 'CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS';
/**
* Reconcile the managed harness-capability keys in settings.json to the
* group's resolved capability state. Manages exactly two keys the
* agent-teams env key and `disableWorkflows` adding AND removing them;
* everything else in the file (operator hand-edits, the PreCompact hook,
* unmanaged env keys) is preserved verbatim. Runs on every spawn, so
* pre-existing groups converge to the configured state, including removal
* of the legacy always-on teams key. See docs/harness-capabilities.md.
* Bring a group's settings.json to its desired state in a single read-modify-
* write: ensure the PreCompact hook is present and reconcile the managed
* harness-capability keys to `caps`. A fresh file is composed from
* DEFAULT_SETTINGS + the capability keys in one atomic write; an existing file
* is mutated in memory and rewritten only if it changed. Any content that isn't
* a JSON object (malformed, `null`, a scalar, an array) is left untouched with
* a warning settings trouble must never break group init or block a spawn.
*/
function reconcileHarnessSettings(
function applyGroupSettings(
settingsFile: string,
caps: Record<string, HarnessCapabilityState>,
caps: Record<string, HarnessCapabilityState> | undefined,
initialized: string[],
): void {
if (!fs.existsSync(settingsFile)) {
const settings = JSON.parse(JSON.stringify(DEFAULT_SETTINGS)) as SettingsObject;
reconcileHarnessKeys(settings, caps);
writeAtomic(settingsFile, JSON.stringify(settings, null, 2) + '\n');
initialized.push('settings.json');
return;
}
let raw: string;
let parsed: unknown;
try {
const raw = fs.readFileSync(settingsFile, 'utf-8');
const settings = JSON.parse(raw) as Record<string, unknown> & { env?: Record<string, unknown> };
// agent-teams: the settings env key is the ONLY working switch on the
// pinned CLI — settings env strictly beats SDK options env, so presence
// in this file IS the state.
if (caps['agent-teams'] === 'on') {
if (!settings.env) settings.env = {};
settings.env[TEAMS_ENV_KEY] = '1';
} else if (settings.env && TEAMS_ENV_KEY in settings.env) {
delete settings.env[TEAMS_ENV_KEY];
}
// workflow: disableWorkflows removes the Workflow tool AND its agent-types
// catalog block from every request. The runner's disallowedTools backstop
// covers a malformed or hand-reverted file — that backstop is what makes
// the warn-and-continue below acceptable.
if (caps.workflow === 'off') {
settings.disableWorkflows = true;
} else if ('disableWorkflows' in settings) {
delete settings.disableWorkflows;
}
const next = JSON.stringify(settings, null, 2) + '\n';
if (next === raw) return; // no churn on the every-spawn path
// tmp+rename: two sessions of one group can spawn concurrently; both
// compute identical output from the same DB row, so last-rename-wins is
// harmless — but a torn write mid-read by the CLI would not be.
const tmp = `${settingsFile}.tmp-${process.pid}`;
fs.writeFileSync(tmp, next);
fs.renameSync(tmp, settingsFile);
initialized.push('settings.json (reconciled harness capabilities)');
raw = fs.readFileSync(settingsFile, 'utf-8');
parsed = JSON.parse(raw);
} catch (err) {
if (!(err instanceof SyntaxError)) throw err;
log.warn('settings.json is malformed — skipping harness-capability reconcile', { settingsFile });
log.warn('settings.json is malformed — leaving it untouched', { settingsFile });
return;
}
const settings = asObject(parsed);
if (!settings) {
log.warn('settings.json is not a JSON object — leaving it untouched', { settingsFile });
return;
}
ensurePreCompactHook(settings);
reconcileHarnessKeys(settings, caps);
const next = JSON.stringify(settings, null, 2) + '\n';
if (next === raw) return; // no churn on the every-spawn path
writeAtomic(settingsFile, next);
initialized.push('settings.json (updated)');
}
/** Ensure the PreCompact archiving hook is present, preserving existing hooks. */
function ensurePreCompactHook(settings: SettingsObject): void {
const hooks = asObject(settings.hooks) ?? (settings.hooks = {});
const existing = Array.isArray(hooks.PreCompact) ? (hooks.PreCompact as unknown[]) : (hooks.PreCompact = []);
if (JSON.stringify(existing).includes(PRE_COMPACT_COMMAND)) return;
existing.push({ hooks: [{ type: 'command', command: PRE_COMPACT_COMMAND }] });
}
/**
* Reconcile the managed harness-capability keys to `caps`, iterating the
* registry's host mechanisms adding AND removing each key so pre-existing
* groups converge (including removal of the legacy always-on teams key).
* Everything unmanaged in the file is preserved. `caps` undefined (non-spawn
* callers) leaves capability keys untouched.
*/
function reconcileHarnessKeys(
settings: SettingsObject,
caps: Record<string, HarnessCapabilityState> | undefined,
): void {
if (!caps) return;
for (const [key, def] of Object.entries(HARNESS_CAPABILITIES)) {
const state = caps[key] ?? def.default;
const m = def.host;
if (m.kind === 'env') {
// Presence of the env key IS the on-state.
if (state === 'on') {
const env = asObject(settings.env) ?? (settings.env = {});
env[m.key] = '1';
} else {
const env = asObject(settings.env);
if (env && m.key in env) delete env[m.key];
}
} else {
// disableFlag: the flag is set true when the capability is OFF.
if (state === 'off') settings[m.key] = true;
else if (m.key in settings) delete settings[m.key];
}
}
}
+28 -11
View File
@@ -28,30 +28,47 @@ describe('resolveHarnessCapabilities', () => {
expect(resolveHarnessCapabilities('"off"')).toEqual(HARNESS_CAPABILITY_DEFAULTS);
});
it('passes unknown keys through and keeps defaults for garbage values on known keys', () => {
const resolved = resolveHarnessCapabilities('{"monitor":"off","workflow":"sideways"}');
expect(resolved.monitor).toBe('off');
it('drops unknown keys and garbage values back to defaults (never leaks into the resolved map)', () => {
const resolved = resolveHarnessCapabilities('{"monitor":"off","workflow":"sideways","future":{"nested":1}}');
expect(Object.hasOwn(resolved, 'monitor')).toBe(false); // unknown key dropped
expect(Object.hasOwn(resolved, 'future')).toBe(false); // unknown non-string dropped
expect(resolved.workflow).toBe('off'); // garbage value → default, not 'sideways'
expect(resolved['agent-teams']).toBe('off');
});
it('accepts an already-parsed override map', () => {
expect(resolveHarnessCapabilities({ workflow: 'on' })).toEqual({ 'agent-teams': 'off', workflow: 'on' });
});
it('does not treat inherited Object.prototype names as known keys', () => {
// `key in obj` would be true for 'constructor'/'toString'; Object.hasOwn is not.
const resolved = resolveHarnessCapabilities('{"constructor":"on","toString":"off"}');
expect(resolved).toEqual({ 'agent-teams': 'off', workflow: 'off' });
});
});
describe('parseHarnessCapabilitiesArg', () => {
it('parses set and clear operations', () => {
expect(parseHarnessCapabilitiesArg('agent-teams=on')).toEqual({ set: { 'agent-teams': 'on' }, clear: [] });
expect(parseHarnessCapabilitiesArg('workflow=default')).toEqual({ set: {}, clear: ['workflow'] });
it('parses on/off/default directives into a key→directive map', () => {
expect(parseHarnessCapabilitiesArg('agent-teams=on')).toEqual({ 'agent-teams': 'on' });
expect(parseHarnessCapabilitiesArg('workflow=default')).toEqual({ workflow: 'default' });
expect(parseHarnessCapabilitiesArg('agent-teams=on, workflow=default')).toEqual({
set: { 'agent-teams': 'on' },
clear: ['workflow'],
'agent-teams': 'on',
workflow: 'default',
});
});
it('normalizes key case and underscores', () => {
expect(parseHarnessCapabilitiesArg('AGENT_TEAMS=ON')).toEqual({ set: { 'agent-teams': 'on' }, clear: [] });
it('resolves a repeated key last-wins', () => {
expect(parseHarnessCapabilitiesArg('workflow=on,workflow=default')).toEqual({ workflow: 'default' });
expect(parseHarnessCapabilitiesArg('workflow=default,workflow=on')).toEqual({ workflow: 'on' });
});
it('rejects unknown keys with the allowed set in the message', () => {
it('normalizes key case and underscores', () => {
expect(parseHarnessCapabilitiesArg('AGENT_TEAMS=ON')).toEqual({ 'agent-teams': 'on' });
});
it('rejects unknown keys, including inherited prototype names', () => {
expect(() => parseHarnessCapabilitiesArg('web=off')).toThrow(/unknown harness capability "web".*agent-teams/);
expect(() => parseHarnessCapabilitiesArg('constructor=on')).toThrow(/unknown harness capability "constructor"/);
});
it('rejects bad values and malformed pairs', () => {
+97 -60
View File
@@ -1,70 +1,113 @@
/**
* Harness-capability registry: which harness-native features NanoClaw exposes
* as per-group toggles, and their code defaults.
* as per-group toggles, their code defaults, and how the host applies each one
* to a group's settings.json.
*
* Policy lives here on the host; mechanism lives provider-side in the agent
* runner. Per-group overrides are a sparse JSON map in
* `container_configs.harness_capabilities`; the RESOLVED map (defaults
* overrides) is what gets materialized into container.json and applied to the
* group's settings.json by the reconciler in group-init.ts. Fixed-off
* capabilities (scheduling, ask-user, plan/worktree, DesignSync) are not keys
* here by design a toggle nobody can meaningfully use is surface without
* value. See docs/harness-capabilities.md.
* This is the single host-side source of truth. Per-group overrides are a
* sparse JSON map in `container_configs.harness_capabilities`; the RESOLVED map
* (defaults overrides) is materialized into container.json and applied to the
* group's settings.json by the reconciler in group-init.ts, which iterates the
* `host` mechanism of every entry below so adding a capability is one entry
* here plus (if it also needs a runtime tool block) one line in the runner's
* CAPABILITY_DISALLOWS map. Fixed-off capabilities (scheduling, ask-user,
* plan/worktree, DesignSync) are not keys here by design a toggle nobody can
* meaningfully use is surface without value. See docs/harness-capabilities.md.
*/
import { log } from './log.js';
export type HarnessCapabilityState = 'on' | 'off';
/**
* Configurable keys and their defaults. Adding a key is additive no schema
* change. Keys are lowercase kebab-case; both toggles default OFF because
* NanoClaw's own systems (a2a messaging, host-side orchestration) are the
* authoritative equivalents.
* How the settings reconciler expresses a capability in settings.json.
* - `env`: the presence of an env key IS the on-state (the key is written when
* on, deleted when off). Verified on the pinned CLI: settings env strictly
* beats SDK options env, so this file is the only working switch.
* - `disableFlag`: a top-level boolean set `true` when the capability is OFF
* (disable-style flag), deleted when on.
*/
export const HARNESS_CAPABILITY_DEFAULTS: Record<string, HarnessCapabilityState> = {
'agent-teams': 'off',
workflow: 'off',
export type HostMechanism = { kind: 'env'; key: string } | { kind: 'disableFlag'; key: string };
export interface CapabilityDef {
default: HarnessCapabilityState;
host: HostMechanism;
}
/**
* Configurable capabilities. Both default OFF because NanoClaw's own systems
* (a2a messaging, host-side orchestration) are the authoritative equivalents.
*/
export const HARNESS_CAPABILITIES: Record<string, CapabilityDef> = {
'agent-teams': { default: 'off', host: { kind: 'env', key: 'CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS' } },
workflow: { default: 'off', host: { kind: 'disableFlag', key: 'disableWorkflows' } },
};
export const HARNESS_CAPABILITY_KEYS = Object.keys(HARNESS_CAPABILITIES);
/** Derived key→default map, for callers that only need the defaults. */
export const HARNESS_CAPABILITY_DEFAULTS: Record<string, HarnessCapabilityState> = Object.fromEntries(
Object.entries(HARNESS_CAPABILITIES).map(([key, def]) => [key, def.default]),
);
const VALID_STATES = new Set<string>(['on', 'off']);
function isKnownKey(key: string): boolean {
return Object.hasOwn(HARNESS_CAPABILITIES, key);
}
// Durable bad state (a stale key or garbage value persisted in the DB) would
// otherwise log on every spawn — under scheduled-task wakes, forever. Warn once
// per distinct problem per host process instead, so real warnings aren't buried.
const warnedOnce = new Set<string>();
function warnOnce(signature: string, message: string, data: Record<string, unknown>): void {
if (warnedOnce.has(signature)) return;
warnedOnce.add(signature);
log.warn(message, data);
}
/**
* Resolve a group's stored override JSON against the code defaults.
*
* Malformed JSON degrades to defaults with a warning rather than throwing:
* unlike structural config (mcp_servers, mounts), capabilities have a safe
* fallback, and the only sanctioned write path (ncl) validates its input.
* Unknown keys pass through untouched (with a warning) so the runner's own
* unknown-key diagnostics stay exercised; known keys with garbage values
* fall back to their default.
* Parse a group's stored override JSON into the raw sparse map. Malformed or
* non-object JSON degrades to {} with a warning rather than throwing: unlike
* structural config (mcp_servers, mounts), capabilities have a safe fallback,
* and the only sanctioned write path (ncl) validates its input.
*/
export function parseHarnessOverrides(overridesJson: string | null | undefined): Record<string, unknown> {
if (!overridesJson) return {};
try {
const parsed: unknown = JSON.parse(overridesJson);
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
return parsed as Record<string, unknown>;
}
warnOnce(`shape:${overridesJson}`, 'harness_capabilities is not a JSON object — using defaults', {
value: overridesJson,
});
} catch (err) {
if (!(err instanceof SyntaxError)) throw err;
warnOnce(`syntax:${overridesJson}`, 'harness_capabilities is malformed JSON — using defaults', {
value: overridesJson,
});
}
return {};
}
/**
* Resolve stored overrides (JSON string, or an already-parsed map) against the
* code defaults. Unknown keys and invalid values are dropped (they fall back to
* defaults) so the resolved map only ever contains known keys with valid
* states the runner therefore never has to reason about unknown keys.
*/
export function resolveHarnessCapabilities(
overridesJson: string | null | undefined,
overrides: string | Record<string, unknown> | null | undefined,
): Record<string, HarnessCapabilityState> {
let overrides: Record<string, unknown> = {};
if (overridesJson) {
try {
const parsed: unknown = JSON.parse(overridesJson);
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
overrides = parsed as Record<string, unknown>;
} else {
log.warn('harness_capabilities is not a JSON object — using defaults', { value: overridesJson });
}
} catch (err) {
if (!(err instanceof SyntaxError)) throw err;
log.warn('harness_capabilities is malformed JSON — using defaults', { value: overridesJson });
}
}
const raw = typeof overrides === 'string' || overrides == null ? parseHarnessOverrides(overrides) : overrides;
const resolved: Record<string, HarnessCapabilityState> = { ...HARNESS_CAPABILITY_DEFAULTS };
for (const [key, value] of Object.entries(overrides)) {
if (!(key in HARNESS_CAPABILITY_DEFAULTS)) {
log.warn('Unknown harness capability key in overrides — passing through', { key });
resolved[key] = value as HarnessCapabilityState;
for (const [key, value] of Object.entries(raw)) {
if (!isKnownKey(key)) {
warnOnce(`unknown:${key}`, 'Unknown harness capability key in overrides — dropping', { key });
continue;
}
if (typeof value !== 'string' || !VALID_STATES.has(value)) {
log.warn('Invalid harness capability value — using default', { key, value });
warnOnce(`value:${key}:${String(value)}`, 'Invalid harness capability value — using default', { key, value });
continue;
}
resolved[key] = value as HarnessCapabilityState;
@@ -72,21 +115,17 @@ export function resolveHarnessCapabilities(
return resolved;
}
export interface HarnessCapabilityOps {
set: Record<string, HarnessCapabilityState>;
clear: string[];
}
/**
* Parse the ncl `--harness-capabilities` flag value: comma-separated `k=v`
* pairs where v is `on`, `off`, or `default` (`default` clears the override
* it is never stored). Keys are normalized (trim, lowercase, `_``-`) and
* validated against the registry. Throws with an actionable message on the
* first problem.
* validated against the registry. Returns a plain keydirective map, so a
* repeated key resolves last-wins by ordinary assignment. Throws with an
* actionable message on the first problem.
*/
export function parseHarnessCapabilitiesArg(input: string): HarnessCapabilityOps {
const allowed = Object.keys(HARNESS_CAPABILITY_DEFAULTS).join(', ');
const ops: HarnessCapabilityOps = { set: {}, clear: [] };
export function parseHarnessCapabilitiesArg(input: string): Record<string, HarnessCapabilityState | 'default'> {
const allowed = HARNESS_CAPABILITY_KEYS.join(', ');
const out: Record<string, HarnessCapabilityState | 'default'> = {};
const pairs = input
.split(',')
.map((p) => p.trim())
@@ -104,16 +143,14 @@ export function parseHarnessCapabilitiesArg(input: string): HarnessCapabilityOps
.slice(eq + 1)
.trim()
.toLowerCase();
if (!(key in HARNESS_CAPABILITY_DEFAULTS)) {
if (!isKnownKey(key)) {
throw new Error(`unknown harness capability "${key}" — configurable keys: ${allowed}`);
}
if (value === 'default') {
ops.clear.push(key);
} else if (VALID_STATES.has(value)) {
ops.set[key] = value as HarnessCapabilityState;
if (value === 'default' || VALID_STATES.has(value)) {
out[key] = value as HarnessCapabilityState | 'default';
} else {
throw new Error(`harness capability "${key}" must be on, off, or default — got "${value}"`);
}
}
return ops;
return out;
}