From 22f811cde2f6e3bec6b4523e4f01fd4564cf3949 Mon Sep 17 00:00:00 2001 From: Ryder Sun Date: Wed, 14 Oct 2026 16:37:57 +0800 Subject: [PATCH] fix: harden local agent runs and broaden natural discussion rounds - Extract WorkBuddy/Claude/Qwen error-result lists (errors[]) into failure diagnostics and classify authentication failures as LOCAL_AGENT_AUTH_REQUIRED instead of a bare LOCAL_AGENT_PROCESS_FAILED, so the catalog marks the Agent as needing a fresh login and retries stop. - Scope the isolated OpenClaw runtime directory by session, workspace, and CLI version, so an upgraded OpenClaw gets a fresh managed scope instead of failing on legacy workspace state written by an older CLI. - Natural agent-led discussions now open with a full participant round and the delivery owner cannot declare completion before peers had a discussion round; sequential runs get the same cross-review gate (system.autoCrossReview). Checks: npm --prefix desktop test (1574 pass), npm --prefix frontend test (343 pass), frontend build and build:desktop, desktop pack. --- desktop/src/agents/agent-runtime-contract.cjs | 19 +- desktop/src/agents/cli/cli-output-parsers.cjs | 36 +++- .../src/agents/cli/cli-process-support.cjs | 14 +- desktop/src/agents/cli/openclaw-runtime.cjs | 19 +- desktop/src/main.cjs | 1 + ...orkspace-auto-runner-natural-decisions.cjs | 29 ++- .../workspace/local-workspace-auto-runner.cjs | 6 +- .../cli/cli-adapters-discovery.test.cjs | 2 +- desktop/test/agents/cli/cli-adapters.test.cjs | 39 ++++ .../test/agents/cli/openclaw-runtime.test.cjs | 21 ++ ...l-workspace-v4-natural-discussion.test.cjs | 185 +++++++++++++----- ...local-workspace-v4-natural-writer.test.cjs | 15 +- frontend/src/locales/en.js | 1 + frontend/src/locales/zh.js | 1 + 14 files changed, 307 insertions(+), 81 deletions(-) diff --git a/desktop/src/agents/agent-runtime-contract.cjs b/desktop/src/agents/agent-runtime-contract.cjs index e72812d..0710dc8 100644 --- a/desktop/src/agents/agent-runtime-contract.cjs +++ b/desktop/src/agents/agent-runtime-contract.cjs @@ -219,6 +219,15 @@ function terminalAuthenticationDiagnostic(value) { : '' } +function authConfigurationFailure(detail) { + return String(detail || '').split(/\r?\n/).some((line) => { + const text = line.trim().replace(/^(?:error|fatal)\s*:\s*/i, '') + return Boolean(terminalAuthenticationDiagnostic(text)) + || /^(?:(?:invalid|expired|missing|incorrect)\s+(?:api[ _-]?key|(?:access|refresh|auth)[ _-]?token|credentials?)\b|(?:authentication|authorization)\s+(?:failed|required)\b|(?:please\s+)?(?:log|sign)\s+in\b|not\s+logged\s+in\b|select an auth type\b|provider rejected\b)/i.test(text) + || /^(?:认证|鉴权|登录)(?:失败|过期)|^(?:令牌|凭据|密钥)(?:无效|缺失|已过期)|^请(?:先|重新)登录/.test(text) + }) +} + function failureCategory(code) { const value = String(code || '') if (value === 'LOCAL_AGENT_SESSION_INVALID') return 'session' @@ -299,9 +308,16 @@ function requireTerminalAgentResult(result) { throw agentRuntimeError('LOCAL_AGENT_EXECUTION_STOPPED') } if (normalized.outcome === 'failed') { - const failure = normalized.failure?.code + let failure = normalized.failure?.code ? normalized.failure : agentRuntimeFailure('LOCAL_AGENT_PROCESS_FAILED') + const diagnostic = String(normalized.diagnostic || normalized.text || '') + // A structured failure whose diagnostic names an authentication problem is + // an authentication failure even when the CLI reported a generic process + // error, so the catalog can mark the Agent as needing a fresh login. + if (failure.category !== 'authentication' && authConfigurationFailure(diagnostic)) { + failure = agentRuntimeFailure('LOCAL_AGENT_AUTH_REQUIRED') + } throw agentRuntimeError( failure.code, normalized.diagnostic || normalized.text, @@ -338,6 +354,7 @@ module.exports = { agentRuntimeCapabilities, agentRuntimeError, agentRuntimeFailure, + authConfigurationFailure, isCodeReviewAgentKind, isReviewOnlyAgentKind, normalizeAgentResult, diff --git a/desktop/src/agents/cli/cli-output-parsers.cjs b/desktop/src/agents/cli/cli-output-parsers.cjs index a173525..21758ed 100644 --- a/desktop/src/agents/cli/cli-output-parsers.cjs +++ b/desktop/src/agents/cli/cli-output-parsers.cjs @@ -172,17 +172,38 @@ function stepFinishOutcome(reason) { return 'partial' } +function structuredResultDiagnostic(event) { + const parts = [] + if (typeof event?.error === 'string' && event.error.trim()) parts.push(event.error.trim()) + else if (event?.error?.message) parts.push(String(event.error.message)) + if (typeof event?.result === 'string' && event.result.trim()) parts.push(event.result.trim()) + if (Array.isArray(event?.errors)) { + parts.push(...event.errors.map(value => String(value || '').trim()).filter(Boolean)) + } + return parts.join('\n') +} + function ingestStructuredEvent(state, event) { if (!event || typeof event !== 'object' || Array.isArray(event)) return state.seen = true if (['workbuddy', 'claude', 'qwen'].includes(state.kind) - && event.type === 'result' && typeof event.result === 'string') { + && event.type === 'result') { if (typeof event.session_id === 'string') state.sessionRef = event.session_id - state.failure = null - state.diagnostic = '' - state.text = '' - appendStructuredText(state, event.result, '') - state.outcome = event.is_error === true ? 'partial' : 'completed' + if (typeof event.result === 'string') { + state.failure = null + state.diagnostic = '' + state.text = '' + appendStructuredText(state, event.result, '') + state.outcome = event.is_error === true ? 'partial' : 'completed' + return + } + // An error result without a usable result string still carries the CLI's + // error list; keep it as the failure diagnostic so authentication and + // session problems are classified instead of surfacing as a bare + // LOCAL_AGENT_PROCESS_FAILED. + if (event.is_error === true || String(event.subtype || '').startsWith('error')) { + markStructuredFailure(state, 'LOCAL_AGENT_PROCESS_FAILED', structuredResultDiagnostic(event)) + } return } if (structuredEventFailed(event)) { @@ -885,7 +906,8 @@ function classifyCliOutcome(kind, stdout) { function structuredCliError(stdout) { const failed = parseJsonOutputEvents(stdout) .findLast(event => event?.is_error || event?.subtype?.startsWith('error')) - return String(failed?.error?.message || failed?.result || '').trim() + if (!failed) return '' + return structuredResultDiagnostic(failed) } function hermesSessionRef(stderr) { diff --git a/desktop/src/agents/cli/cli-process-support.cjs b/desktop/src/agents/cli/cli-process-support.cjs index 55041c4..15a61c3 100644 --- a/desktop/src/agents/cli/cli-process-support.cjs +++ b/desktop/src/agents/cli/cli-process-support.cjs @@ -2,7 +2,10 @@ const path = require('node:path') const { searchPath, systemChildEnvironment } = require('./cli-discovery.cjs') const { NETWORK_ENV_KEYS, networkEnvironment } = require('./cli-network-environment.cjs') const { redactChildSecrets } = require('./cli-runtime-events.cjs') -const { agentRuntimeError, terminalAuthenticationDiagnostic } = require('../agent-runtime-contract.cjs') +const { + agentRuntimeError, + authConfigurationFailure, +} = require('../agent-runtime-contract.cjs') const { validateOpenClawRuntimeGuard } = require('./openclaw-runtime.cjs') const TERMINATE_GRACE_MS = 500 @@ -29,15 +32,6 @@ function agentExecutionError(code, diagnostic = '') { return agentRuntimeError(code, diagnostic) } -function authConfigurationFailure(detail) { - return String(detail || '').split(/\r?\n/).some((line) => { - const text = line.trim().replace(/^(?:error|fatal)\s*:\s*/i, '') - return Boolean(terminalAuthenticationDiagnostic(text)) - || /^(?:(?:invalid|expired|missing|incorrect)\s+(?:api[ _-]?key|(?:access|refresh|auth)[ _-]?token|credentials?)\b|(?:authentication|authorization)\s+(?:failed|required)\b|(?:please\s+)?(?:log|sign)\s+in\b|not\s+logged\s+in\b|select an auth type\b|provider rejected\b)/i.test(text) - || /^(?:认证|鉴权|登录)(?:失败|过期)|^(?:令牌|凭据|密钥)(?:无效|缺失|已过期)|^请(?:先|重新)登录/.test(text) - }) -} - function invalidSessionFailure(detail) { const text = String(detail || '').trim() if (!text || authConfigurationFailure(text) diff --git a/desktop/src/agents/cli/openclaw-runtime.cjs b/desktop/src/agents/cli/openclaw-runtime.cjs index aaa04ab..f1e8990 100644 --- a/desktop/src/agents/cli/openclaw-runtime.cjs +++ b/desktop/src/agents/cli/openclaw-runtime.cjs @@ -192,7 +192,7 @@ function normalizeProvider(input) { return { apiKey, baseUrl, model } } -function runtimePaths({ storageRoot, workdir, sessionRef, configName }) { +function runtimePaths({ storageRoot, workdir, sessionRef, configName, runtimeVersion }) { if (!path.isAbsolute(storageRoot) || !path.isAbsolute(workdir)) { throw new Error('OPENCLAW_RUNTIME_INVALID_SCOPE') } @@ -204,8 +204,15 @@ function runtimePaths({ storageRoot, workdir, sessionRef, configName }) { throw new Error('OPENCLAW_RUNTIME_INVALID_SCOPE') } const resolvedWorkdir = path.resolve(workdir) + // The managed scope is keyed by session, workspace, and CLI version. OpenClaw + // refuses to start on state written by an older CLI ("Legacy workspace setup + // state requires migration"), so a version-scoped directory gives an upgraded + // CLI a fresh isolated scope instead of a guaranteed startup failure. + const versionSegment = String(runtimeVersion || '').trim() const scope = crypto.createHash('sha256') - .update(`${sessionRef || 'configure'}\0${resolvedWorkdir}`) + .update(versionSegment + ? `${sessionRef || 'configure'}\0${resolvedWorkdir}\0${versionSegment}` + : `${sessionRef || 'configure'}\0${resolvedWorkdir}`) .digest('hex') .slice(0, 24) const runtimeRoot = path.join(resolvedStorageRoot, 'openclaw-managed', scope) @@ -459,14 +466,14 @@ function normalizedNativeModel(input) { } function managedOpenClawOptions({ - storageRoot, workdir, sessionRef = '', allowWrite = false, provider, + storageRoot, workdir, sessionRef = '', allowWrite = false, provider, openclawVersion, }) { const normalized = normalizeProvider(provider) const configName = allowWrite ? 'openclaw.provider.workspace-write.json' : 'openclaw.provider.read-only.json' const runtime = runtimePaths({ - storageRoot, workdir, sessionRef, configName, + storageRoot, workdir, sessionRef, configName, runtimeVersion: openclawVersion, }) const { resolvedWorkdir, home, state, configPath } = runtime @@ -525,7 +532,7 @@ function managedOpenClawOptions({ } function nativeOpenClawOptions({ - storageRoot, workdir, sessionRef = '', allowWrite = false, runtime, + storageRoot, workdir, sessionRef = '', allowWrite = false, runtime, openclawVersion, }) { const modelRef = String(runtime?.model || '').trim() const provider = runtime?.provider @@ -557,7 +564,7 @@ function nativeOpenClawOptions({ ? 'openclaw.native.workspace-write.json' : 'openclaw.native.read-only.json' const runtimePathsResult = runtimePaths({ - storageRoot, workdir, sessionRef, configName, + storageRoot, workdir, sessionRef, configName, runtimeVersion: openclawVersion, }) const { resolvedWorkdir, home, state, configPath } = runtimePathsResult const modelConfig = { primary: modelRef } diff --git a/desktop/src/main.cjs b/desktop/src/main.cjs index 1fce08b..40e41b3 100644 --- a/desktop/src/main.cjs +++ b/desktop/src/main.cjs @@ -361,6 +361,7 @@ async function runCoreAgent(agent, prompt, workdir, options = {}) { workdir, storageRoot: app.getPath('userData'), nativeRuntime, + openclawVersion: agent.version, }) const nativeEnv = connectorCredentialIsolation || agent.kind === 'openclaw' ? {} diff --git a/desktop/src/workspace/local-workspace-auto-runner-natural-decisions.cjs b/desktop/src/workspace/local-workspace-auto-runner-natural-decisions.cjs index acd882f..2b699c3 100644 --- a/desktop/src/workspace/local-workspace-auto-runner-natural-decisions.cjs +++ b/desktop/src/workspace/local-workspace-auto-runner-natural-decisions.cjs @@ -55,7 +55,21 @@ const v4NaturalDecisionMethods = { '', threadRootId, { key: 'system.autoTaskDecisionMissing' }) return { status: 'partial' } } - if (decision.status === 'completed') return { status: controller.failedKinds.length ? 'partial' : 'completed' } + if (decision.status === 'completed') { + // The owner may not close the task before selected peers had one discussion + // round of their own; otherwise a single full round collapses into an + // owner-only review and peers never get to challenge the result. + if (activeKinds.some(kind => kind !== owner) + && !this.v4NaturalPriorPeerDiscussion(group, controller, threadRootId, round, owner)) { + this.addMessage( + group.id, 'system', + 'The delivery owner signaled completion before peer discussion; running one cross-review round first.', + '', threadRootId, { key: 'system.autoCrossReview' }, + ) + return { nextKinds: [...activeKinds], ownerReview: false } + } + return { status: controller.failedKinds.length ? 'partial' : 'completed' } + } if (decision.status === 'needs-human' && this.requestTaskDecision) { const response = await this.requestTaskDecision(group, controller, ownerMessage) if (response?.status === 'approved') return { nextKinds: [owner], ownerReview: true } @@ -81,6 +95,19 @@ const v4NaturalDecisionMethods = { return activeKinds.filter(kind => selected.has(kind)) }, + v4NaturalPriorPeerDiscussion(group, controller, threadRootId, round, owner) { + return this.state().messages.some(message => ( + message.groupId === group.id + && message.threadRootId === threadRootId + && message.role === 'agent' + && message.agentKind !== owner + && message.trace?.runId === controller.runId + && message.trace?.phase === 'discussion' + && Number(message.trace?.round) < round + && this.v4NaturalMessageMatchesBinding(message, controller) + )) + }, + v4NaturalOwner(group, controller, threadRootId) { let owner = controller.targetKinds[0] || '' for (const message of this.state().messages) { diff --git a/desktop/src/workspace/local-workspace-auto-runner.cjs b/desktop/src/workspace/local-workspace-auto-runner.cjs index ef72eea..39c1dd4 100644 --- a/desktop/src/workspace/local-workspace-auto-runner.cjs +++ b/desktop/src/workspace/local-workspace-auto-runner.cjs @@ -2856,8 +2856,10 @@ class LocalWorkspaceAutoRunner { } } else { round = 2 - nextKinds = [this.v4NaturalOwner(group, controller, threadRootId)] - if (!activeKinds.includes(nextKinds[0])) return 'partial' + // The first discussion round is a full participant round: every selected + // Agent reacts to the proposal set before the delivery owner reviews and + // judges completion, so a group run cannot collapse to owner-only turns. + nextKinds = [...activeKinds] } while (!controller.signal.aborted) { diff --git a/desktop/test/agents/cli/cli-adapters-discovery.test.cjs b/desktop/test/agents/cli/cli-adapters-discovery.test.cjs index d54faf5..29684cd 100644 --- a/desktop/test/agents/cli/cli-adapters-discovery.test.cjs +++ b/desktop/test/agents/cli/cli-adapters-discovery.test.cjs @@ -51,7 +51,7 @@ test('capability timeouts retry once and remain inconclusive when both attempts return { stdout: argsForProbe(_command, _args) } }, }) - assert.deepEqual(timeouts, recover ? [8000, 8000, 16000] : [8000, 8000, 16000, 16000]) + assert.deepEqual(timeouts, recover ? [8000, 16000] : [8000, 16000]) assert.equal(result.compatibilityState, recover ? 'compatible' : 'unknown') if (!recover) assert.equal(result.incompatibilityReason, 'LOCAL_AGENT_CAPABILITY_PROBE_TIMEOUT') } diff --git a/desktop/test/agents/cli/cli-adapters.test.cjs b/desktop/test/agents/cli/cli-adapters.test.cjs index 605228a..d55cd28 100644 --- a/desktop/test/agents/cli/cli-adapters.test.cjs +++ b/desktop/test/agents/cli/cli-adapters.test.cjs @@ -521,6 +521,45 @@ process.exitCode = 1 assert.equal(events.some(event => event.type === 'answer_delta'), true) }) +for (const exitCode of [0, 1]) { + test(`WorkBuddy authentication failures classify as auth-required with exit code ${exitCode}`, async (t) => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), `meldwork-workbuddy-auth-${exitCode}-`)) + const cli = executable(directory, 'workbuddy-auth-required.cjs', ` + process.stdout.write(JSON.stringify({ + type: 'system', subtype: 'init', session_id: 'workbuddy-auth-session', + apiKeySource: 'www.workbuddy.cn', + }) + '\\n') + process.stdout.write(JSON.stringify({ + type: 'assistant', session_id: 'workbuddy-auth-session', + message: { role: 'assistant', content: [ + { type: 'text', text: 'Authentication required. Please use /login command to sign in to your account' }, + ] }, + }) + '\\n') + process.stdout.write(JSON.stringify({ + type: 'result', subtype: 'error_during_execution', is_error: true, + session_id: 'workbuddy-auth-session', + errors: ['Authentication required. Please use /login command to sign in to your account'], + }) + '\\n') + process.exitCode = ${exitCode} + `) + t.after(() => fs.rmSync(directory, { recursive: true, force: true })) + + await assert.rejects( + runAgent( + { kind: 'workbuddy', executable: cli, name: 'WorkBuddy' }, + 'hello', + directory, + { }, + ), + (error) => { + assert.equal(error.code, 'LOCAL_AGENT_AUTH_REQUIRED') + assert.match(error.diagnostic || '', /Authentication required/u) + return true + }, + ) + }) +} + test('Gemini profile state retains one real tool lifecycle id and summary', async (t) => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'meldwork-gemini-profile-state-')) const cli = executable(directory, 'gemini-profile-state.cjs', ` diff --git a/desktop/test/agents/cli/openclaw-runtime.test.cjs b/desktop/test/agents/cli/openclaw-runtime.test.cjs index 0a39f46..a9816e1 100644 --- a/desktop/test/agents/cli/openclaw-runtime.test.cjs +++ b/desktop/test/agents/cli/openclaw-runtime.test.cjs @@ -90,6 +90,27 @@ test('managed runtime isolates OpenClaw state and keeps the API key out of confi assert.equal(fs.statSync(result.env.OPENCLAW_CONFIG_PATH).mode & 0o777, 0o600) }) +test('managed runtime scope is keyed by session, workspace, and CLI version', (t) => { + const { directory, workdir } = fixture() + t.after(() => fs.rmSync(directory, { recursive: true, force: true })) + + const base = { + storageRoot: directory, + workdir, + sessionRef: 'agent:main:desktop-meldwork-scope-openclaw', + provider: provider(), + } + const versionA = managedOpenClawOptions({ ...base, openclawVersion: 'OpenClaw 2026.7.1-2 (a1b2c3)' }) + const versionARepeat = managedOpenClawOptions({ ...base, openclawVersion: 'OpenClaw 2026.7.1-2 (a1b2c3)' }) + const versionB = managedOpenClawOptions({ ...base, openclawVersion: 'OpenClaw 2026.9.1 (ad6fe23)' }) + const unversioned = managedOpenClawOptions({ ...base }) + + assert.equal(versionA.env.OPENCLAW_STATE_DIR, versionARepeat.env.OPENCLAW_STATE_DIR) + assert.notEqual(versionA.env.OPENCLAW_STATE_DIR, versionB.env.OPENCLAW_STATE_DIR) + assert.notEqual(versionA.env.OPENCLAW_STATE_DIR, unversioned.env.OPENCLAW_STATE_DIR) + assert.equal(versionB.env.OPENCLAW_STATE_DIR.startsWith(fs.realpathSync(directory)), true) +}) + test('Gateway setup rewrites the guarded config without persisting credentials', (t) => { const { directory, workdir } = fixture() t.after(() => fs.rmSync(directory, { recursive: true, force: true })) diff --git a/desktop/test/workspace/local-workspace-v4-natural-discussion.test.cjs b/desktop/test/workspace/local-workspace-v4-natural-discussion.test.cjs index 001e6d4..2c37192 100644 --- a/desktop/test/workspace/local-workspace-v4-natural-discussion.test.cjs +++ b/desktop/test/workspace/local-workspace-v4-natural-discussion.test.cjs @@ -47,7 +47,7 @@ for (const nextKinds of [undefined, []]) { await workspace.refreshAgents() const group = workspace.createGroup({ name: 'Prose is not a dispatch', agentKinds: ['codex', 'hermes'], workdir: directory, allowWrite: false }) const controller = await runDiscussion(workspace, group, { discussionStyle: 'agent-led', maxRounds: 4 }) - assert.deepEqual(turns, ['codex']) + assert.deepEqual(turns, ['codex', 'hermes', 'codex']) assert.equal(ledger.get(controller.runId).status, 'completed') const reopened = new RunLedger({ storagePath: ledger.storagePath }) assert.deepEqual(reopened.get(controller.runId).agentRuns.at(-1).context.taskDecision.nextKinds, nextKinds) @@ -120,22 +120,25 @@ test('Natural routing retains a request beyond the stored message excerpt', asyn const { directory, options } = fixture() t.after(() => fs.rmSync(directory, { recursive: true, force: true })) const turns = [] - const workspace = new LocalWorkspace({ - ...options, naturalAgentResponses: true, - runAgent: async (agent, prompt) => { - if (naturalPhase(prompt) === 'proposal') return { outcome: 'completed', text: 'Initial contribution.' } - turns.push(agent.kind) - return { outcome: 'completed', text: turns.length === 1 - ? decisionReply(`${'x'.repeat(21000)}\n@hermes Please verify the missing evidence.`, 'continue', - { reason: 'Request evidence verification.', nextKinds: ['hermes'] }) - : turns.length === 2 ? 'The evidence is unavailable.' - : decisionReply('Required evidence is unavailable.', 'blocked') } - }, - }) + const codexDiscussionTurns = new Map() + options.naturalAgentResponses = true + options.runAgent = async (agent, prompt) => { + if (naturalPhase(prompt) === 'proposal') return { outcome: 'completed', text: 'Initial contribution.' } + turns.push(agent.kind) + const turn = (codexDiscussionTurns.get(agent.kind) || 0) + 1 + codexDiscussionTurns.set(agent.kind, turn) + if (agent.kind === 'codex' && turn === 1) { + return { outcome: 'completed', text: decisionReply(`${'x'.repeat(21000)}\n@hermes Please verify the missing evidence.`, 'continue', + { reason: 'Request evidence verification.', nextKinds: ['hermes'] }) } + } + if (agent.kind === 'hermes') return { outcome: 'completed', text: 'The evidence is unavailable.' } + return { outcome: 'completed', text: decisionReply('Required evidence is unavailable.', 'blocked') } + } + const workspace = new LocalWorkspace(options) await workspace.refreshAgents() const group = workspace.createGroup({ name: 'Long route', agentKinds: ['codex', 'hermes'], workdir: directory, allowWrite: false }) await runDiscussion(workspace, group, { discussionStyle: 'agent-led', maxRounds: 4 }) - assert.deepEqual(turns, ['codex', 'hermes', 'codex']) + assert.deepEqual(turns, ['codex', 'hermes', 'hermes', 'codex']) }) test('Natural discussion does not complete when a peer reports an explicit blocker', async t => { @@ -226,12 +229,15 @@ for (const handoffTo of [undefined, 'hermes', 'ghost']) { t.after(() => fs.rmSync(directory, { recursive: true, force: true })) const ledger = new RunLedger({ storagePath: path.join(directory, 'run-ledger.json') }) const turns = [] + const discussionTurns = new Map() const workspace = new LocalWorkspace({ ...options, runLedger: ledger, naturalAgentResponses: true, runAgent: async (agent, prompt) => { if (naturalPhase(prompt) === 'proposal') return { outcome: 'completed', text: 'Initial answer.', sessionRef: `${agent.kind}-task-session` } turns.push(agent.kind) - return { outcome: 'completed', sessionRef: `${agent.kind}-task-session`, text: turns.length === 1 + const turn = (discussionTurns.get(agent.kind) || 0) + 1 + discussionTurns.set(agent.kind, turn) + return { outcome: 'completed', sessionRef: `${agent.kind}-task-session`, text: agent.kind === 'codex' && turn === 1 ? decisionReply('More work remains.', 'continue', handoffTo ? { handoffTo } : {}) : decisionReply('The requested work is complete.') } }, @@ -241,7 +247,10 @@ for (const handoffTo of [undefined, 'hermes', 'ghost']) { await runDiscussion(workspace, group, { discussionStyle: 'agent-led', maxRounds: 5 }) const record = ledger.list(group.id)[0] assert.equal(record.status, handoffTo === 'ghost' ? 'partial' : 'completed', record.reason) - assert.deepEqual(turns, handoffTo === 'ghost' ? ['codex'] : ['codex', handoffTo || 'codex']) + assert.deepEqual(turns, handoffTo === 'ghost' + ? ['codex', 'hermes'] + : handoffTo === 'hermes' ? ['codex', 'hermes', 'hermes'] + : ['codex', 'hermes', 'codex']) }) } @@ -315,6 +324,68 @@ test('Natural sequential V4 runs every Agent once per round in configured CLI or )).length, 1) }) +test('Natural V4 requires a prior peer discussion round before the owner completes', async (t) => { + const { directory, options } = fixture() + t.after(() => fs.rmSync(directory, { recursive: true, force: true })) + const workspace = new LocalWorkspace({ ...options, naturalAgentResponses: true }) + await workspace.refreshAgents() + const group = { id: 'group' } + const controller = { + runId: 'run', targetKinds: ['codex', 'hermes'], failedKinds: [], + discussionStyle: 'agent-led', + } + const systemKeys = [] + workspace.autoRunner.state = () => ({ messages: [] }) + workspace.autoRunner.v4NaturalMessageMatchesBinding = () => true + workspace.autoRunner.addMessage = (groupId, role, text, _target, threadRootId, metadata) => { + systemKeys.push(metadata?.key) + return { id: `system-${systemKeys.length}` } + } + const completedDecision = { + summary: 'Done.', taskDecision: { + status: 'completed', reason: 'Ready.', deliverables: ['The answer.'], + }, + } + + // A solo owner round with a completion judgment and no earlier peer + // discussion round cannot close the task; it must dispatch one cross-review + // round with every active participant first. + workspace.autoRunner.state = () => ({ + messages: [{ + groupId: 'group', threadRootId: 'thread', role: 'agent', agentKind: 'codex', + trace: { phase: 'discussion', runId: 'run', round: 2, context: completedDecision }, + }], + }) + assert.deepEqual( + await workspace.autoRunner.v4NaturalRoundOutcome( + group, controller, 'thread', 2, ['codex', 'hermes'], + ), + { nextKinds: ['codex', 'hermes'], ownerReview: false }, + ) + assert.deepEqual(systemKeys, ['system.autoCrossReview']) + + // Once a peer contributed to an earlier discussion round the same judgment + // completes the run. + workspace.autoRunner.state = () => ({ + messages: [ + { + groupId: 'group', threadRootId: 'thread', role: 'agent', agentKind: 'hermes', + trace: { phase: 'discussion', runId: 'run', round: 1, context: null }, + }, + { + groupId: 'group', threadRootId: 'thread', role: 'agent', agentKind: 'codex', + trace: { phase: 'discussion', runId: 'run', round: 2, context: completedDecision }, + }, + ], + }) + assert.deepEqual( + await workspace.autoRunner.v4NaturalRoundOutcome( + group, controller, 'thread', 2, ['codex', 'hermes'], + ), + { status: 'completed' }, + ) +}) + for (const discussionStyle of ['sequential', 'agent-led']) { test(`Natural ${discussionStyle} V4 reports a one-round budget stop without claiming delivery`, async (t) => { const { directory, options } = fixture() @@ -406,12 +477,12 @@ test('Natural Agent-led V4 uses the full first-round transcript and inline menti assert.deepEqual(calls.map(call => `${call.phase}:${call.kind}`), [ 'proposal:codex', 'proposal:hermes', 'proposal:workbuddy', - 'discussion:codex', + 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', 'discussion:hermes', 'discussion:workbuddy', 'discussion:codex', ]) assert.equal(maxProposalRunning, 3) - assert.equal(maxRoutedRunning, 2) + assert.equal(maxRoutedRunning, 3) const coordinator = calls[3] for (const sentinel of tails.values()) assert.match(coordinator.prompt, new RegExp(sentinel)) assert.ok(coordinator.prompt.length > 24000) @@ -428,7 +499,7 @@ test('Natural Agent-led V4 uses the full first-round transcript and inline menti )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ '1:codex', '1:hermes', '1:workbuddy', - '2:codex', + '2:codex', '2:hermes', '2:workbuddy', '3:hermes', '3:workbuddy', '4:codex', ]) @@ -456,7 +527,7 @@ test('Natural routing uses explicit canonical recipients independently of displa } }) -test('Natural Agent-led V4 preserves chosen peers without forced participation and supplies recent context', async (t) => { +test('Natural Agent-led V4 keeps later rounds routed and supplies recent context', async (t) => { const { directory, options } = fixture() t.after(() => fs.rmSync(directory, { recursive: true, force: true })) options.naturalAgentResponses = true @@ -479,18 +550,23 @@ test('Natural Agent-led V4 preserves chosen peers without forced participation a workdir: directory, allowWrite: false, }) await runDiscussion(workspace, group, { discussionStyle: 'agent-led', maxRounds: 5 }) - assert.equal(calls.some(call => call.kind === 'workbuddy' && call.phase === 'discussion'), false) - const peer = calls.at(-1) - assert.equal(peer.kind, 'hermes') + const workbuddyDiscussionCalls = calls.filter(call => ( + call.kind === 'workbuddy' && call.phase === 'discussion' + )) + // WorkBuddy joins the opening full round, then never runs again without an + // explicit peer request. + assert.equal(workbuddyDiscussionCalls.length, 1) + assert.equal(workbuddyDiscussionCalls[0].prompt.includes('codex proposes an independent approach'), true) + assert.equal(workbuddyDiscussionCalls[0].prompt.includes('hermes proposes an independent approach'), true) const firstPeerTurn = calls.find(call => call.kind === 'hermes' && call.phase === 'discussion') - assert.match(firstPeerTurn.prompt, /Round 2 - @codex/) - assert.match(firstPeerTurn.prompt, /workbuddy proposes an independent approach/) - assert.doesNotMatch(peer.prompt, /Round 2 - @codex/) - assert.match(peer.prompt, /Previously delivered turns are not repeated/) - assert.match(peer.prompt, /Round 3 - @hermes/) - assert.match(peer.prompt, /Round 4 - @codex/) - assert.match(peer.prompt, /beside its concrete question/) - assert.doesNotMatch(peer.prompt, /final non-empty line/) + assert.match(firstPeerTurn.prompt, /Round 1 - @codex/u) + assert.match(firstPeerTurn.prompt, /workbuddy proposes an independent approach/u) + const laterPeerTurn = calls.filter(call => ( + call.kind === 'hermes' && call.phase === 'discussion' + )).at(-1) + assert.match(laterPeerTurn.prompt, /Previously delivered turns are not repeated/u) + assert.match(laterPeerTurn.prompt, /beside its concrete question/u) + assert.doesNotMatch(laterPeerTurn.prompt, /final non-empty line/u) }) test('Natural Agent-led V4 keeps a peer request when another concurrent peer accepts', async (t) => { @@ -500,15 +576,16 @@ test('Natural Agent-led V4 keeps a peer request when another concurrent peer acc const ledger = new RunLedger({ storagePath: path.join(directory, 'run-ledger.json') }) options.runLedger = ledger const calls = [] - let codexTurns = 0 + const discussionTurns = new Map() options.runAgent = async (agent, prompt, _workdir, runOptions) => { const phase = naturalPhase(prompt) calls.push(`${phase}:${agent.kind}`) - if (phase === 'discussion' && agent.kind === 'codex') codexTurns += 1 + const turn = (discussionTurns.get(agent.kind) || 0) + (phase === 'discussion' ? 1 : 0) + discussionTurns.set(agent.kind, turn) return { text: phase === 'proposal' ? `${agent.kind} proposal.` - : agent.kind === 'codex' && codexTurns === 1 ? decisionReply('@hermes assess feasibility; @workbuddy validate risks.', 'continue', { nextKinds: ['hermes', 'workbuddy'] }) - : agent.kind === 'workbuddy' ? decisionReply('@codex address this remaining risk before proceeding.', 'continue', { nextKinds: ['codex'] }) + : agent.kind === 'codex' && turn === 1 ? decisionReply('@hermes assess feasibility; @workbuddy validate risks.', 'continue', { nextKinds: ['hermes', 'workbuddy'] }) + : agent.kind === 'workbuddy' && turn === 1 ? decisionReply('@codex address this remaining risk before proceeding.', 'continue', { nextKinds: ['codex'] }) : decisionReply('I accept the current result without changes.'), sessionRef: runOptions.sessionRef || `${agent.kind}-task-session`, } @@ -524,7 +601,9 @@ test('Natural Agent-led V4 keeps a peer request when another concurrent peer acc }) assert.deepEqual(calls, [ 'proposal:codex', 'proposal:hermes', 'proposal:workbuddy', - 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', 'discussion:codex', + 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', + 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', + 'discussion:codex', ]) assert.equal(ledger.get(controller.runId).status, 'completed') }) @@ -661,7 +740,7 @@ test('Natural Agent-led V4 reuses partially committed routed results after resta assert.deepEqual(initialCalls.map(call => `${call.phase}:${call.kind}`), [ 'proposal:codex', 'proposal:hermes', 'proposal:workbuddy', - 'discussion:codex', + 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', 'discussion:hermes', 'discussion:workbuddy', 'discussion:codex', ]) @@ -705,7 +784,7 @@ test('Natural Agent-led V4 reuses partially committed routed results after resta )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ '1:codex', '1:hermes', '1:workbuddy', - '2:codex', + '2:codex', '2:hermes', '2:workbuddy', '3:hermes', '3:workbuddy', '4:codex', ]) @@ -1135,10 +1214,11 @@ test('Natural Agent-led V4 does not fabricate completion or force a peer review assert.deepEqual(calls.map(call => `${call.phase}:${call.kind}`), [ 'proposal:codex', 'proposal:hermes', + 'discussion:codex', 'discussion:hermes', 'discussion:codex', ]) assert.equal(maxProposalRunning, 2) - assert.equal(maxRoutedRunning, 1) + assert.equal(maxRoutedRunning, 2) assert.match( calls.find(call => call.phase === 'discussion').prompt, /Silence from peers is not evidence of completion/u, @@ -1147,7 +1227,7 @@ test('Natural Agent-led V4 does not fabricate completion or force a peer review message.role === 'agent' && message.threadRootId === controller.threadRootId )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ - '1:codex', '1:hermes', '2:codex', + '1:codex', '1:hermes', '2:codex', '2:hermes', '3:codex', ]) assert.equal(ledger.get(controller.runId).status, 'partial') }) @@ -1195,14 +1275,15 @@ test('Natural Agent-led V4 routes a single mention serially and completes after assert.deepEqual(calls, [ 'proposal:codex', 'proposal:hermes', 'discussion:codex', 'discussion:hermes', + 'discussion:hermes', 'discussion:codex', ]) - assert.equal(maxRoutedRunning, 1) + assert.equal(maxRoutedRunning, 2) const messages = workspace.snapshot().messages.filter(message => ( message.role === 'agent' && message.threadRootId === controller.threadRootId )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ - '1:codex', '1:hermes', '2:codex', '3:hermes', '4:codex', + '1:codex', '1:hermes', '2:codex', '2:hermes', '3:hermes', '4:codex', ]) }) @@ -1240,13 +1321,14 @@ test('Natural Agent-led V4 does not accept a self-only route without a task deci assert.deepEqual(calls, [ 'proposal:codex', 'proposal:hermes', + 'discussion:codex', 'discussion:hermes', 'discussion:codex', ]) const messages = workspace.snapshot().messages.filter(message => ( message.role === 'agent' && message.threadRootId === controller.threadRootId )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ - '1:codex', '1:hermes', '2:codex', + '1:codex', '1:hermes', '2:codex', '2:hermes', '3:codex', ]) }) @@ -1287,7 +1369,7 @@ test('Natural Agent-led V4 does not treat concurrent self-routes as peer confirm assert.deepEqual(calls, [ 'proposal:codex', 'proposal:hermes', 'proposal:workbuddy', - 'discussion:codex', + 'discussion:codex', 'discussion:hermes', 'discussion:workbuddy', 'discussion:hermes', 'discussion:workbuddy', 'discussion:codex', ]) @@ -1296,7 +1378,8 @@ test('Natural Agent-led V4 does not treat concurrent self-routes as peer confirm )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ '1:codex', '1:hermes', '1:workbuddy', - '2:codex', '3:hermes', '3:workbuddy', + '2:codex', '2:hermes', '2:workbuddy', + '3:hermes', '3:workbuddy', '4:codex', ]) }) @@ -1578,9 +1661,9 @@ test('Natural Agent-led V4 recovers the bound owner decision after a stale messa if (!crashCaptured && input.agentKind === 'codex' && input.metadata?.trace?.phase === 'discussion' - && Number(input.metadata?.trace?.round) === 2 + && Number(input.metadata?.trace?.round) === 3 && durable?.orchestration?.phase === 'discussion' - && durable.orchestration.round === 2 + && durable.orchestration.round === 3 && durable.orchestration.pendingKinds.join(',') === 'codex') { crashCaptured = true recoveryLedger.checkpoint(durable) @@ -1603,14 +1686,14 @@ test('Natural Agent-led V4 recovers the bound owner decision after a stale messa const crashRecord = await crashCheckpoint.promise await initialController.promise const originalCoordinatorRun = crashRecord.agentRuns.find(run => ( - run.round === 2 && run.kind === 'codex' && run.status === 'completed' + run.round === 3 && run.kind === 'codex' && run.status === 'completed' )) assert.ok(originalCoordinatorRun) const recoveryState = JSON.parse(fs.readFileSync(recoveryStoragePath, 'utf8')) const staleCoordinatorMessage = recoveryState.messages.find(message => ( message.role === 'agent' && message.threadRootId === crashRecord.threadRootId - && message.trace?.round === 2 + && message.trace?.round === 3 && message.agentKind === 'codex' )) assert.ok(staleCoordinatorMessage) @@ -1643,10 +1726,10 @@ test('Natural Agent-led V4 recovers the bound owner decision after a stale messa message.role === 'agent' && message.threadRootId === crashRecord.threadRootId )) assert.deepEqual(messages.map(message => `${message.trace.round}:${message.agentKind}`), [ - '1:codex', '1:hermes', '2:codex', + '1:codex', '1:hermes', '2:codex', '2:hermes', '3:codex', ]) const coordinatorMessage = messages.find(message => ( - message.trace.round === 2 && message.agentKind === 'codex' + message.trace.round === 3 && message.agentKind === 'codex' )) assert.equal(coordinatorMessage.trace.agentRunId, originalCoordinatorRun.agentRunId) assertTurnParity(recoveredRecord, messages) diff --git a/desktop/test/workspace/local-workspace-v4-natural-writer.test.cjs b/desktop/test/workspace/local-workspace-v4-natural-writer.test.cjs index 6895d64..764cdb3 100644 --- a/desktop/test/workspace/local-workspace-v4-natural-writer.test.cjs +++ b/desktop/test/workspace/local-workspace-v4-natural-writer.test.cjs @@ -102,8 +102,19 @@ for (const writerKind of ['hermes', '']) { const result = ledger.list(group.id)[0] assert.equal(result.status, 'completed', JSON.stringify(workspace.snapshot().messages.map(message => message.content))) assert.equal(result.orchestration.discussionWriterKind || '', writerKind) - assert.deepEqual(calls.filter(call => call.sandbox === 'workspace-write'), writerKind - ? [{ kind: writerKind, discussion: true, sandbox: 'workspace-write' }] : []) + const writeCalls = calls.filter(call => call.sandbox === 'workspace-write') + if (writerKind) { + // The writer may join both the opening full round and a later owner + // review round, but only the declared writer ever receives write access. + assert.ok(writeCalls.length >= 1) + assert.ok(writeCalls.every(call => ( + call.kind === writerKind && call.discussion === true + ))) + assert.ok(calls.filter(call => call.sandbox !== 'workspace-write') + .every(call => call.sandbox === 'read-only')) + } else { + assert.deepEqual(writeCalls, []) + } }) } diff --git a/frontend/src/locales/en.js b/frontend/src/locales/en.js index 71f15c4..2ea46df 100644 --- a/frontend/src/locales/en.js +++ b/frontend/src/locales/en.js @@ -805,6 +805,7 @@ export default { 'system.autoStopped': 'Automatic discussion stopped: {reason}', 'system.autoDiscussionStalled': 'Automatic discussion stopped because repeated handoffs produced no new contribution.', 'system.autoRoundLimit': 'Automatic discussion reached the {rounds}-round limit without reaching consensus.', + 'system.autoCrossReview': 'The delivery owner signaled completion before peer discussion; one cross-review round runs first.', 'system.autoTaskDecisionMissing': 'Discussion stopped without a valid task completion decision.', 'system.autoTaskBlocked': 'Task blocked: {reason}', 'system.autoTaskNeedsHuman': 'Human decision needed: {reason}', diff --git a/frontend/src/locales/zh.js b/frontend/src/locales/zh.js index ece8d7f..4b6a774 100644 --- a/frontend/src/locales/zh.js +++ b/frontend/src/locales/zh.js @@ -805,6 +805,7 @@ export default { 'system.autoStopped': '自动讨论已停止:{reason}', 'system.autoDiscussionStalled': '自动讨论因重复转交且没有新增内容而停止。', 'system.autoRoundLimit': '自动讨论已达到 {rounds} 轮上限,尚未达成共识。', + 'system.autoCrossReview': '交付负责人在同伴讨论前判断任务完成;将先进行一轮交叉审查。', 'system.autoTaskDecisionMissing': '讨论已停止,尚无有效的任务完成判断。', 'system.autoTaskBlocked': '任务受阻:{reason}', 'system.autoTaskNeedsHuman': '需要人类决定:{reason}',