From a44f72e5c55d656ba6abade9c4ddb62af5c24ec2 Mon Sep 17 00:00:00 2001 From: unohee Date: Thu, 22 Oct 2026 18:06:35 +0900 Subject: [PATCH] fix(daemon): close review findings on the S6 stack (#838-#846) Fingerprint now ignores the appended comment history so an unchanged issue stays held, and a declined draft is kept on the task so the pre-claim hold can see it. Fences reach ollama-cloud direct, tester, documenter, auditor, skill-documenter and openswarm work. Provider switches and fanout candidates obey the delegated-adapter policy. exec answer rejects --text without --correlation, redeploys to the run's project, and releases a hold when the deploy is refused. The disk gate also measures the state DB volume. --- .gitignore | 6 +- ARCHITECTURE.md | 13 +- README.md | 6 +- config.example.yaml | 11 +- src/adapters/ollamaCloud.test.ts | 30 ++++ src/adapters/ollamaCloud.ts | 9 ++ src/adapters/unattendedPolicy.test.ts | 79 +++++++++- src/adapters/unattendedPolicy.ts | 37 ++++- src/agents/auditor.ts | 7 + src/agents/documenter.ts | 7 + src/agents/pairPipeline.ts | 5 + src/agents/pairPipelineRoleScope.test.ts | 46 ++++++ src/agents/skillDocumenter.ts | 7 + src/agents/stageFences.test.ts | 59 ++++++++ src/agents/tester.ts | 7 + .../autonomousRunner.coverage.test.ts | 48 +++++++ ...tonomousRunner.dispatchFingerprint.test.ts | 38 +++++ .../autonomousRunner.hostHealth.test.ts | 16 +++ src/automation/autonomousRunner.ts | 77 ++++++++-- src/automation/dispatchFingerprint.test.ts | 35 +++++ src/automation/dispatchFingerprint.ts | 16 ++- src/automation/executionTaskContext.ts | 9 +- src/automation/missingWorktreeDisposition.ts | 10 +- src/automation/runnerTypes.ts | 2 + src/automation/workRunner.ts | 8 +- src/cli.ts | 16 +-- src/cli/execCommand.test.ts | 136 +++++++++++++++++- src/cli/execCommand.ts | 91 ++++++++++-- src/cli/execGuarded.test.ts | 31 ++++ src/cli/workCommand.test.ts | 17 +++ src/cli/workCommand.ts | 4 + src/cli/workExecution.test.ts | 28 ++++ src/cli/workExecution.ts | 8 ++ src/core/config.ts | 5 +- src/core/service.test.ts | 47 ++++++ src/core/service.ts | 14 +- .../conflictDetector.coverage.test.ts | 40 ++++++ src/orchestration/conflictDetector.ts | 6 + src/orchestration/decisionEngine.ts | 5 +- src/support/osSandbox.ts | 6 +- src/support/web.provider.test.ts | 29 ++++ src/support/web.ts | 12 ++ 42 files changed, 1019 insertions(+), 64 deletions(-) create mode 100644 src/agents/stageFences.test.ts create mode 100644 src/cli/execGuarded.test.ts diff --git a/.gitignore b/.gitignore index 2d11fd85..c59522f0 100644 --- a/.gitignore +++ b/.gitignore @@ -31,8 +31,12 @@ config-docker.yaml swarm.log # Environment +# `.env.*` also covers hand-made backups such as `.env.bak-`, which hold the same tokens as +# `.env` and would otherwise be staged by `git add -A`. The tracked template stays visible. .env -.env.local +.env.* +!.env.example +nohup.out # IDE .idea/ diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 14c2594a..6a19fc69 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -58,10 +58,11 @@ graph LR Admission under `unknownScopeAdmission: admit` (default, vela): -1. `includeBacklog: true` — Backlog is a queue. -2. `idle_fill` lifts `NEEDS_HUMAN` / `RETRY_AT` / legacy backoff when Linear still wants the card. +0. Before anything is fetched: the disk floor (`minFreeDiskGiB`, `support/hostHealth.ts`, AGT-4785) pauses the heartbeat. Right after the fetch, `partitionByReadiness` (`automation/readinessGate.ts`, AGT-4784) drops Backlog and skip-label issues, so they never reach `filterAlreadyProcessed`. +1. `includeBacklog: false` (default since AGT-4784) — Todo is the ready signal. A deployment that wants free slots to chew the Backlog (vela) must set `includeBacklog: true`. +2. `idle_fill` lifts `NEEDS_HUMAN` / `RETRY_AT` / legacy backoff when Linear still wants the card — except for an issue held by an `exec` orchestrator (`automation/execHold.ts`) or whose title and description are unchanged since its run started (`automation/dispatchFingerprint.ts`, AGT-4787). 3. Predicted file overlap does **not** defer. Worktrees isolate; merge later. -4. Still skipped: Linear Done/Canceled, parent/EPIC umbrellas, live `CLAIMED`/`EXECUTING` leases, unresolved deps, unmapped/disabled projects. +4. Still skipped: Linear Done/Canceled, parent/EPIC umbrellas, live `CLAIMED`/`EXECUTING` leases, unresolved deps, unmapped/disabled projects. `detectSafeCandidateIds` also drops an issue whose draft carries a confident "a person has to do or decide this" verdict before the claim. `serialize` restores Codex-era fail-closed overlap holds. @@ -71,7 +72,7 @@ Admission under `unknownScopeAdmission: admit` (default, vela): - `RunState` (`automation/runLedgerTypes.ts`) — `READY`…`DONE` plus `RETRY_AT`, `NEEDS_HUMAN`, `NEEDS_RECONCILE`. - `ParkResumeTrigger` — `idle_fill` | `explicit_dispatch` | `tracker_todo` | `sandbox_quarantine_dispatch` | `unspecified`. - `UnknownScopeAdmission` — `'admit' | 'serialize'`. -- Config: `src/core/config.ts` `AutonomousConfigSchema` (`includeBacklog` default true, `unknownScopeAdmission` default admit). +- Config: `src/core/config.ts` `AutonomousConfigSchema` (`includeBacklog` default false since AGT-4784, `unknownScopeAdmission` default admit). ## 확장 지점 @@ -86,8 +87,8 @@ Admission under `unknownScopeAdmission: admit` (default, vela): - **`vela-build.sh `는 `origin/main`만 fetch한다.** PR SHA는 `builds/4e8ba24`에서 `git fetch origin ` 먼저. - **PR SHA를 돌리는 동안 `.last-built-sha`는 `origin/main` tip을 유지**하지 않으면 autodeploy cron이 main으로 롤백한다. - **`unknownScopeAdmission: admit`를 heartbeat만 바꾸고 claim 게이트를 안 바꾸면** 큐에는 들어가고 `claimRun`이 null — 슬롯이 다시 빈다. -- **`isActionableLinearState(state)` 헬퍼 기본은 Backlog park.** 엔진/config 기본은 `includeBacklog: true`. `undefined`를 spread하면 DEFAULT true를 덮는다 — constructor가 `?? true`로 고정한다. -- **AGT-4155 park loop**: Linear `In Progress`만으로 park를 열면 claim→exec→park가 heartbeat마다 돈다. 지금은 `idle_fill`이 의도적으로 연다 (싼 모델이 빈 슬롯을 씹도록). `serialize`/질문 대기를 되살릴 때 이 루프를 다시 열지 말 것. +- **`isActionableLinearState(state)` helper parks Backlog by default**, and since AGT-4784 so do the engine, config and runner (`includeBacklog: false`; every reader spells it `?? false`). `undefined` must keep meaning false: never spread an `undefined` over the default, and never reintroduce a `?? true`. +- **AGT-4155 park loop**: opening a park on Linear `In Progress` alone makes claim→exec→park spin every heartbeat. `idle_fill` opens parks on purpose (cheap models chew free slots), except where `execHold` or an unchanged `dispatchFingerprint` holds the park (AGT-4787). Do not reopen this loop when reviving `serialize` or question waits. - **drafted `docs/integrations.md` 같은 광역 fileScope**는 한 레포의 카드를 전부 한 conflict group으로 묶는다. `admit`이 아니면 슬롯이 비어 있는 채로 1장만 산다. - 로컬 워크스페이스 `fix/agt-4220-…`의 dirty `store.ts` / `autonomousRunner.ts`는 이 스택과 섞지 말 것. diff --git a/README.md b/README.md index 267500aa..d14fc05e 100644 --- a/README.md +++ b/README.md @@ -359,7 +359,7 @@ DISCORD_CHANNEL_ID=your-channel-id | `github` | Repos list for CI monitoring | | `agents` | Agent definitions (name, projectPath, heartbeat interval) | | `autonomous` | Schedule, pair mode, role models, decomposition settings (`maxChildrenPerTask` applies to both the runner and human `/plan`; `dailyLimit` paces the unsupervised runner only — AGT-4123) | -| `autonomous.includeBacklog` | Treat Linear Backlog as a work queue (default `true`) | +| `autonomous.includeBacklog` | Treat Linear Backlog as a work queue (default `false`: moving an issue to Todo is the ready signal; set `true` on a deployment that wants free slots to chew the Backlog) | | `autonomous.unknownScopeAdmission` | `admit` (default): worktrees isolate — predicted file overlap does not defer. `serialize`: Codex-era fail-closed | | `prProcessor` | PR auto-improvement schedule, retry limits, conflict resolver config | @@ -403,7 +403,7 @@ The `openrouter` adapter runs OpenSwarm's own agentic tool loop (read/search/edi Each autonomous run snapshots the current Claude Code instruction hierarchy (`~/.claude/CLAUDE.md`, user rules, repository `CLAUDE.md`/`AGENTS.md`, and matching `.claude/rules`) once and applies the same capsule to orchestrator, worker, reviewer, Codex, CC-Router, and Cursor. The dashboard shows the capsule digest and source/error counts, not the rule bodies. -`adapterRouting.primary` must name the adapter the worker actually runs (`adapter:` at the top of the config, or `roles.worker.adapter`); a policy whose primary is some other adapter is ignored, and the fallbacks never engage. Role MCP grants and the coordination tools (`coordination_read`, `coordination_peers`, `coordination_publish`, `coordination_thread_*`, `ask_human`) additionally require an adapter that runs OpenSwarm's own tool loop — `codex-responses`, `cc-router`, `gpt`, `openrouter`, `atlascloud`, `lmstudio`, `local`. The delegated CLIs (`codex`, `claude`, `cursor`) bring their own tool loop, so those grants do not reach them; OpenSwarm logs a warning rather than pretending they applied. +`adapterRouting.primary` must name the adapter the worker actually runs (`adapter:` at the top of the config, or `roles.worker.adapter`); a policy whose primary is some other adapter is ignored, and the fallbacks never engage. Role MCP grants and the coordination tools (`coordination_read`, `coordination_peers`, `coordination_publish`, `coordination_thread_*`, `ask_human`) additionally require an adapter that runs OpenSwarm's own tool loop — `codex-responses`, `cc-router`, `gpt`, `openrouter`, `atlascloud`, `lmstudio`, `local`. The delegated CLIs (`codex`, `claude`, `cursor`) bring their own tool loop, so those grants do not reach them; for interactive CLI use OpenSwarm logs a warning rather than pretending they applied. The daemon is stricter: a config with an `autonomous` section that names a delegated adapter anywhere (`adapter`, `defaultRoles`, `projectAgents` roles, `adapterRouting` primary or fallbacks, `orchestrator`, `loopAdvisor`) refuses to start unless `autonomous.allowDelegatedAdapters: true` accepts the missing fences on purpose. A configured `coordinationBoardIssueId` turns one project-scoped tracker issue into the durable agent board. Worker advice/delegation, Discord questions, adapter routes, periodic reviews, and MCP denials are visible through `GET /api/coordination`, SSE, and the **AGENT COORDINATION** dashboard panel. Tool arguments, prompts, credentials, and rule bodies are redacted or omitted. @@ -426,7 +426,7 @@ autonomous: coordinationBoardIssueId: AGT-3993 adapterRouting: primary: codex-responses - fallbacks: [cc-router, cursor] + fallbacks: [cc-router] # `cursor` is a delegated CLI: add it only with allowDelegatedAdapters: true allowReasons: [quota, infra, capability] mcpPolicies: orchestrator: diff --git a/config.example.yaml b/config.example.yaml index ae0898a7..b7b02352 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -117,14 +117,16 @@ autonomous: # `primary` must name the same adapter this file selects above (adapter:), or # routing silently disables — the worker only consults a policy whose primary # is the adapter it is actually running. - # Only typed failures may route to CC-Router or an authenticated Cursor CLI; - # a failing test or a reviewer verdict never does. + # Only typed failures may route to a fallback; a failing test or a reviewer + # verdict never does. `cursor` (and `codex`) are delegated CLIs that bring + # their own tool loop, so the daemon refuses to start with one listed here + # unless allowDelegatedAdapters: true (below) accepts that on purpose. # quota — provider usage limit / 429 on the primary # infra — spawn, auth, or timeout failure before the agent ran # capability — the primary is not installed or not serving on this machine adapterRouting: primary: codex-responses - fallbacks: [cc-router, cursor] + fallbacks: [cc-router] allowReasons: [quota, infra, capability] # Exact role-scoped MCP grants. Reads are allowed from listed servers; writes @@ -270,7 +272,8 @@ autonomous: # OS fence for the worker's bash tool (AGT-4387). on (default): every bash # call runs under sandbox-exec (macOS) / bwrap (Linux) with writes limited to - # the worktree, temp dir and package caches; network stays open. A host with + # the worktree, temp dir and package caches; network is loopback-only unless + # workerNetwork (below) says open. A host with # neither binary refuses worker bash commands instead of running them open # (AGT-4783). off: bash runs unfenced with only the regex denylist. Either way # the shell gets an allowlisted env and a private HOME — no GitHub, cloud or diff --git a/src/adapters/ollamaCloud.test.ts b/src/adapters/ollamaCloud.test.ts index b52203e9..48ea0f44 100644 --- a/src/adapters/ollamaCloud.test.ts +++ b/src/adapters/ollamaCloud.test.ts @@ -277,6 +277,36 @@ describe('OllamaCloudAdapter', () => { expect(new OllamaCloudAdapter().getTransport()).toBe('local'); }); + it('enforces the publication fence on the direct transport, as every other native adapter does (AGT-4783)', async () => { + // The direct route (OLLAMA_API_KEY set) built its loop options by hand and kept only `protectedFiles`, + // so a fenced worker could `git push` while the local transport refused it. + process.env.OLLAMA_API_KEY = 'test-key'; + delete process.env.OLLAMA_CLOUD_BASE_URL; + const bodies: Array<{ messages: Array<{ role: string; content?: string }> }> = []; + vi.stubGlobal('fetch', vi.fn(async (url: string, init?: RequestInit) => { + if (!String(url).endsWith('/chat/completions')) { + return new Response(JSON.stringify({ object: 'list', data: [] }), { status: 200 }); + } + bodies.push(JSON.parse(String(init?.body))); + const sse = bodies.length === 1 + ? 'data: {"choices":[{"delta":{"role":"assistant","tool_calls":[{"index":0,"id":"call_1","type":"function",' + + '"function":{"name":"bash","arguments":"{\\"command\\":\\"git push origin HEAD\\"}"}}]},"finish_reason":"tool_calls"}]}\n\n' + + 'data: [DONE]\n' + : 'data: {"choices":[{"delta":{"role":"assistant","content":"done"},"finish_reason":"stop"}]}\n\ndata: [DONE]\n'; + return new Response(sse, { status: 200, headers: { 'Content-Type': 'text/event-stream' } }); + })); + + const result = await new OllamaCloudAdapter().run({ + prompt: 'push it', cwd: process.cwd(), model: 'deepseek-v4.1-flash:cloud', + enableTools: true, maxTurns: 3, forbidPublication: true, + }); + + expect(result.exitCode).toBe(0); + expect(bodies).toHaveLength(2); + const toolReply = bodies[1].messages.filter((m) => m.role === 'tool').at(-1); + expect(toolReply?.content).toMatch(/^PUBLICATION_FENCED:/); + }, 30_000); + it('retries a transient 5xx on the direct transport and sends the plain id to the chat endpoint', async () => { process.env.OLLAMA_API_KEY = 'test-key'; delete process.env.OLLAMA_CLOUD_BASE_URL; diff --git a/src/adapters/ollamaCloud.ts b/src/adapters/ollamaCloud.ts index 111c522b..4a88b8da 100644 --- a/src/adapters/ollamaCloud.ts +++ b/src/adapters/ollamaCloud.ts @@ -402,6 +402,15 @@ export class OllamaCloudAdapter extends LocalModelAdapter implements CliAdapter finishValidator: options.finishValidator, finishValidatorMaxRetries: options.finishValidatorMaxRetries, protectedFiles: options.protectedFiles, + // The fences OpenSwarm's own loop enforces for an unattended stage. The local transport + // forwards all of them (local.ts) and so does every other native adapter; this literal kept + // only `protectedFiles`, so a worker on the direct route (OLLAMA_API_KEY set) ran with no + // write fence, no loopback-only network and no publication fence while the boot log said + // "Worker sandbox: on" and unattendedPolicy counted ollama-cloud as fenced. (AGT-4783, AGT-4444) + forbidPublication: options.forbidPublication, + sandbox: options.sandbox, + scratchpadRunId: options.scratchpadRunId, + memoryContext: options.memoryContext, bashTimeoutMs: options.bashTimeoutMs, webTools: options.webTools, memoryTools: options.memoryTools, diff --git a/src/adapters/unattendedPolicy.test.ts b/src/adapters/unattendedPolicy.test.ts index 7ff16e4f..9f7baf6a 100644 --- a/src/adapters/unattendedPolicy.test.ts +++ b/src/adapters/unattendedPolicy.test.ts @@ -7,11 +7,14 @@ // `defaultRoles` but not `adapterRouting.fallbacks` would pass the first test // and still route unattended work to a CLI that has no fences. -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; +import { fileURLToPath } from 'node:url'; +import { loadConfig } from '../core/config.js'; import { DELEGATED_ADAPTERS, assertUnattendedAdapters, collectUnattendedAdapterViolations, + providerSwitchViolation, unattendedAdapterViolation, type UnattendedAdapterConfig, } from './unattendedPolicy.js'; @@ -92,6 +95,80 @@ describe('collectUnattendedAdapterViolations', () => { }); }); +describe('worker fan-out candidates', () => { + // `candidates[].adapter` accepts any adapter. In `execute` mode runWorkerFanout runs each candidate as an + // unattended worker on that adapter, so a delegated CLI there is the same hole as `role.adapter`. + const fanout = (over: Record = {}) => ({ + mode: 'execute', candidates: [{ adapter: 'openrouter' }, { adapter: 'claude' }], ...over, + }); + + it('refuses a delegated candidate in execute mode, naming the candidate, for a default and a project role', () => { + const viaDefault = collectUnattendedAdapterViolations(safe({ defaultRoles: { worker: { fanout: fanout() } } })); + expect(viaDefault).toHaveLength(1); + expect(viaDefault[0]).toContain('autonomous.defaultRoles.worker.fanout.candidates[1].adapter'); + const viaProject = collectUnattendedAdapterViolations(safe({ + projectAgents: [{ projectPath: '/repo', roles: { worker: { fanout: fanout() } } }], + })); + expect(viaProject[0]).toContain('autonomous.projectAgents[/repo].roles.worker.fanout.candidates[1].adapter'); + }); + + it('still checks a role that names no adapter of its own', () => { + // The role-level check skipped any role without `adapter`, which is where a fan-out-only role lives. + const violations = collectUnattendedAdapterViolations(safe({ + defaultRoles: { worker: { adapter: undefined, fanout: fanout({ candidates: [{ adapter: 'cursor' }] }) } }, + })); + expect(violations).toHaveLength(1); + expect(violations[0]).toContain("'cursor'"); + }); + + it('lets through what cannot run: report mode, a disabled fan-out, a disabled role, or in-process candidates', () => { + expect(collectUnattendedAdapterViolations(safe({ defaultRoles: { worker: { fanout: fanout({ mode: 'report' }) } } }))).toEqual([]); + expect(collectUnattendedAdapterViolations(safe({ defaultRoles: { worker: { fanout: fanout({ enabled: false }) } } }))).toEqual([]); + expect(collectUnattendedAdapterViolations(safe({ defaultRoles: { worker: { enabled: false, fanout: fanout() } } }))).toEqual([]); + expect(collectUnattendedAdapterViolations(safe({ + defaultRoles: { worker: { fanout: fanout({ candidates: [{ adapter: 'openrouter' }, { adapter: 'codex-responses' }, {}] }) } }, + }))).toEqual([]); + }); + + it('honours the opt-in', () => { + expect(collectUnattendedAdapterViolations(safe({ allowDelegatedAdapters: true, defaultRoles: { worker: { fanout: fanout() } } }))).toEqual([]); + }); +}); + +describe('providerSwitchViolation', () => { + // assertUnattendedAdapters reads config.yaml once, at boot. The provider override, the dashboard toggle + // and `openswarm provider` re-point every role afterwards, so they meet the same policy and opt-in. + it('refuses a delegated CLI and lets an in-process adapter through, unless the operator opted in', () => { + for (const name of DELEGATED_ADAPTERS) { + expect(providerSwitchViolation(name, false), name).toContain(`'${name}'`); + expect(providerSwitchViolation(name, undefined), `${name} unset is not an opt-in`).not.toBeNull(); + expect(providerSwitchViolation(name, true), `${name} opted in`).toBeNull(); + } + expect(providerSwitchViolation('codex-responses', false)).toBeNull(); + expect(providerSwitchViolation('openrouter', undefined)).toBeNull(); + }); +}); + +describe('the shipped example config', () => { + it('passes the policy it ships next to', () => { + // `cp config.example.yaml config.yaml` is the documented install (README, docker-compose). Its + // adapterRouting once listed `cursor` as a fallback, which this policy refuses, so a deployment + // copied from the example never came up. The refusal happens only after the web server, Discord + // and the scheduler have started, and nothing else loads the example through the policy. + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + const config = loadConfig(fileURLToPath(new URL('../../config.example.yaml', import.meta.url))); + expect(config.autonomous).toBeDefined(); + expect(collectUnattendedAdapterViolations(config)).toEqual([]); + expect(() => assertUnattendedAdapters(config)).not.toThrow(); + } finally { + log.mockRestore(); + warn.mockRestore(); + } + }); +}); + describe('assertUnattendedAdapters', () => { it('does not throw for a safe config', () => { expect(() => assertUnattendedAdapters(safe())).not.toThrow(); diff --git a/src/adapters/unattendedPolicy.ts b/src/adapters/unattendedPolicy.ts index 1fe6e802..6903db10 100644 --- a/src/adapters/unattendedPolicy.ts +++ b/src/adapters/unattendedPolicy.ts @@ -26,7 +26,27 @@ export function unattendedAdapterViolation(name: string): string | null { + 'role tool limits and the credential-free sandboxed shell do not apply to it'; } -interface RoleLike { adapter?: string; enabled?: boolean } +/** + * Why the unattended runner may not be pointed at `adapter` after startup, or null. + * + * `assertUnattendedAdapters` reads config.yaml once, at boot. Three things re-point every role + * afterwards without going through it: the persisted provider override (re-applied right after the + * assertion), `POST /api/provider` (the dashboard toggle) and `openswarm provider`. Each of them + * has to meet the same policy, with the same opt-in, or one click undoes the refusal. (AGT-4444) + */ +export function providerSwitchViolation( + adapter: string, + allowDelegatedAdapters: boolean | undefined, +): string | null { + return allowDelegatedAdapters === true ? null : unattendedAdapterViolation(adapter); +} + +interface RoleLike { + adapter?: string; + enabled?: boolean; + /** Worker fan-out: in `execute` mode every candidate runs a worker on its own adapter (`workerFanout.ts`). */ + fanout?: { enabled?: boolean; mode?: string; candidates?: ReadonlyArray<{ adapter?: string }> }; +} type RolesLike = Partial>; /** The part of `SwarmConfig` that decides which adapters unattended runs can reach. */ @@ -44,9 +64,18 @@ export interface UnattendedAdapterConfig { function* roleAdapters(roles: RolesLike | undefined, where: string): Generator<[string, string]> { for (const [role, value] of Object.entries(roles ?? {})) { - // A role the operator switched off never runs, so its adapter is not reachable. - if (!value?.adapter || value.enabled === false) continue; - yield [`${where}.${role}.adapter`, value.adapter]; + // A role the operator switched off never runs, so neither its adapter nor its fan-out is reachable. + if (!value || value.enabled === false) continue; + if (value.adapter) yield [`${where}.${role}.adapter`, value.adapter]; + // A role can have no adapter of its own and still fan out. In `execute` mode each candidate runs an + // unattended worker on its own adapter, which the role's adapter check never sees; in `report` mode + // the gate only decides and nothing runs, so it is not reachable. (workerFanoutGate.ts) + const fanout = value.fanout; + if (fanout && fanout.enabled !== false && fanout.mode === 'execute') { + for (const [i, candidate] of (fanout.candidates ?? []).entries()) { + if (candidate.adapter) yield [`${where}.${role}.fanout.candidates[${i}].adapter`, candidate.adapter]; + } + } } } diff --git a/src/agents/auditor.ts b/src/agents/auditor.ts index 89fb02e1..cc3d1b14 100644 --- a/src/agents/auditor.ts +++ b/src/agents/auditor.ts @@ -30,6 +30,8 @@ export interface AuditorOptions { */ toolAllow?: string[]; toolDeny?: string[]; + /** OS fence for this stage's bash tool: `autonomous.workerSandbox`, as for the worker (AGT-4387). */ + sandbox?: 'on' | 'off'; } export interface AuditorResult { @@ -110,6 +112,11 @@ export async function runAuditor(options: AuditorOptions): Promise { @@ -26,6 +29,21 @@ vi.mock('./reviewer.js', async () => { return { ...actual, runReviewer }; }); +vi.mock('./documenter.js', async () => { + const actual = await vi.importActual('./documenter.js'); + return { ...actual, runDocumenter }; +}); + +vi.mock('./auditor.js', async () => { + const actual = await vi.importActual('./auditor.js'); + return { ...actual, runAuditor }; +}); + +vi.mock('./skillDocumenter.js', async () => { + const actual = await vi.importActual('./skillDocumenter.js'); + return { ...actual, runSkillDocumenter }; +}); + vi.mock('../adapters/index.js', async () => { const actual = await vi.importActual('../adapters/index.js'); return { ...actual, getAdapter: () => ({ getDefaultModel }) }; @@ -64,6 +82,9 @@ describe('PairPipeline per-role tool scope and effort', () => { confidencePercent: 100, }); runReviewer.mockResolvedValue({ decision: 'approve', feedback: 'approved' }); + runDocumenter.mockResolvedValue({ success: true, updatedFiles: [], apiDocsUpdated: false, summary: 'docs' }); + runAuditor.mockResolvedValue({ success: true, bsScore: 0, criticalCount: 0, warningCount: 0, minorCount: 0, issues: [], summary: 'clean' }); + runSkillDocumenter.mockResolvedValue({ success: true, updatedFiles: [], summary: 'skills' }); getDefaultModel.mockResolvedValue('codex-live-model'); }); @@ -131,4 +152,29 @@ describe('PairPipeline per-role tool scope and effort', () => { reasoningEffort: 'high', })); }); + + it("hands the worker sandbox setting to every stage that has a shell, not only the worker (AGT-4783)", async () => { + // Each stage's bash tool is the worker's bash tool. Only the worker stage got the OS fence, so a repo + // script run by the documenter or the auditor could still write outside the worktree and reach the network. + const { PairPipeline } = await import('./pairPipeline.js'); + const roleOf = { enabled: true, timeoutMs: 0 }; + const pipeline = new PairPipeline({ + stages: ['worker', 'reviewer', 'documenter', 'auditor', 'skill-documenter'], + maxIterations: 1, + workerSandbox: 'on', + skipAuditorUnderFileCount: 0, // the worker mock reports one changed file; the default threshold is 3 + roles: { + worker: { ...roleOf, model: 'w' }, reviewer: { ...roleOf, model: 'r' }, + documenter: { ...roleOf, model: 'd' }, auditor: { ...roleOf, model: 'a' }, + 'skill-documenter': { ...roleOf, model: 's' }, + }, + }); + + await pipeline.run(task(), process.cwd()); + + expect(runWorker).toHaveBeenCalledWith(expect.objectContaining({ sandbox: 'on' })); + expect(runDocumenter).toHaveBeenCalledWith(expect.objectContaining({ sandbox: 'on' })); + expect(runAuditor).toHaveBeenCalledWith(expect.objectContaining({ sandbox: 'on' })); + expect(runSkillDocumenter).toHaveBeenCalledWith(expect.objectContaining({ sandbox: 'on' })); + }); }); diff --git a/src/agents/skillDocumenter.ts b/src/agents/skillDocumenter.ts index 2b859d59..1dddd441 100644 --- a/src/agents/skillDocumenter.ts +++ b/src/agents/skillDocumenter.ts @@ -30,6 +30,8 @@ export interface SkillDocumenterOptions { */ toolAllow?: string[]; toolDeny?: string[]; + /** OS fence for this stage's bash tool: `autonomous.workerSandbox`, as for the worker (AGT-4387). */ + sandbox?: 'on' | 'off'; } export interface SkillDocumenterResult { @@ -108,6 +110,11 @@ export async function runSkillDocumenter(options: SkillDocumenterOptions): Promi reasoningEffort: options.reasoningEffort, toolAllow: options.toolAllow, toolDeny: options.toolDeny, + // The same fences the worker's shell runs under: a stage's bash tool is the same tool, and the + // credential-free env alone does not stop a repo script from writing outside the worktree, reaching + // the network or publishing. Only the worker stage forwarded them. (AGT-4783, AGT-4798, AGT-4418) + sandbox: options.sandbox, + forbidPublication: true, }); return parseSkillDocumenterOutput(raw.stdout); } catch (error) { diff --git a/src/agents/stageFences.test.ts b/src/agents/stageFences.test.ts new file mode 100644 index 00000000..f524174d --- /dev/null +++ b/src/agents/stageFences.test.ts @@ -0,0 +1,59 @@ +// Created: 2026-10-22 +// Purpose: every stage that runs the agentic loop forwards the fences the worker's shell runs under +// Dependencies: ../adapters/index.js (spawnCli mocked; nothing here shells out) +// Test Status: Complete +// +// Only the worker stage used to pass `sandbox` and `forbidPublication` to spawnCli. The tester's LLM +// fallback, the documenter, the auditor and the skill documenter get the same `bash` tool, and the +// credential-free env (AGT-4783) alone does not stop a repo script from writing outside the worktree, +// reaching the network (AGT-4798) or committing and pushing (AGT-4418). A probe with the old option +// shape ran `git commit` and wrote outside the worktree; the worker's shape refused both. + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { WorkerResult } from './agentPair.js'; + +const spawnCli = vi.fn(); + +vi.mock('../adapters/index.js', () => ({ + getAdapter: () => ({}), + spawnCli: (...args: unknown[]) => spawnCli(...(args as [])), +})); + +const { runTester } = await import('./tester.js'); +const { runDocumenter } = await import('./documenter.js'); +const { runAuditor } = await import('./auditor.js'); +const { runSkillDocumenter } = await import('./skillDocumenter.js'); + +const workerResult: WorkerResult = { + success: true, summary: 'did it', filesChanged: ['src/a.ts'], commands: ['npm test'], output: 'done', +}; +const base = { taskTitle: 'T', taskDescription: 'D', workerResult, projectPath: '/repo' }; + +type StageRun = (sandbox: 'on' | 'off' | undefined) => Promise; +const stages: Array<[string, StageRun]> = [ + ['tester', (sandbox) => runTester({ ...base, sandbox })], + ['documenter', (sandbox) => runDocumenter({ ...base, sandbox })], + ['auditor', (sandbox) => runAuditor({ ...base, sandbox })], + ['skill documenter', (sandbox) => runSkillDocumenter({ ...base, sandbox })], +]; + +describe('stage fences', () => { + beforeEach(() => { + spawnCli.mockReset(); + spawnCli.mockResolvedValue({ stdout: '' }); + }); + + it.each(stages)('the %s forwards the worker sandbox setting and forbids publication to its shell', async (_name, run) => { + await run('on'); + expect(spawnCli).toHaveBeenCalledTimes(1); + expect(spawnCli.mock.calls[0][1]).toMatchObject({ sandbox: 'on', forbidPublication: true }); + }); + + it.each(stages)('the %s forwards an explicit off, and leaves it unset when the caller set none', async (_name, run) => { + await run('off'); + expect(spawnCli.mock.calls[0][1]).toMatchObject({ sandbox: 'off', forbidPublication: true }); + spawnCli.mockClear(); + await run(undefined); + expect((spawnCli.mock.calls[0][1] as { sandbox?: string }).sandbox).toBeUndefined(); + }); +}); diff --git a/src/agents/tester.ts b/src/agents/tester.ts index ec82b3b6..778360bc 100644 --- a/src/agents/tester.ts +++ b/src/agents/tester.ts @@ -39,6 +39,8 @@ export interface TesterOptions { */ toolAllow?: string[]; toolDeny?: string[]; + /** OS fence for this stage's bash tool: `autonomous.workerSandbox`, as for the worker (AGT-4387). */ + sandbox?: 'on' | 'off'; } export interface TesterResult extends ExecutionBoundarySignals { @@ -143,6 +145,11 @@ export async function runTester(options: TesterOptions): Promise { reasoningEffort: options.reasoningEffort, toolAllow: options.toolAllow, toolDeny: options.toolDeny, + // The same fences the worker's shell runs under: a stage's bash tool is the same tool, and the + // credential-free env alone does not stop a repo script from writing outside the worktree, reaching + // the network or publishing. Only the worker stage forwarded them. (AGT-4783, AGT-4798, AGT-4418) + sandbox: options.sandbox, + forbidPublication: true, }); const result = parseTesterOutput(raw.stdout); diff --git a/src/automation/autonomousRunner.coverage.test.ts b/src/automation/autonomousRunner.coverage.test.ts index b97d3b30..495b8873 100644 --- a/src/automation/autonomousRunner.coverage.test.ts +++ b/src/automation/autonomousRunner.coverage.test.ts @@ -393,6 +393,45 @@ describe('AutonomousRunner coverage — safely-reachable helpers', () => { expect(await internal.detectSafeCandidateIds([{ task: candidate, projectPath: '/repo' }])).toEqual(new Set(['weak'])); }); + it('caches a decline that has no brief, so a held issue costs no draft after the first heartbeat (AGT-4784)', async () => { + // resolveTaskFileScope keeps a confident decline on the task but names no files. The draft cache + // used to store only 'drafted' scopes, so the verdict was recomputed (a model call) on every + // heartbeat for exactly the issues this gate exists for. + runnerExecution.setTaskSource({ kind: 'linear', addComment: vi.fn(async () => {}) } as unknown as ITaskSource); + const internal = new AutonomousRunner(goalCfg('ship it')) as unknown as Internal; + resolveTaskFileScopeMock.mockImplementation(async (candidate: TaskItem) => { + candidate.preAdmissionDraft = { relevantFiles: [], sufficient: false, scope: declined } as unknown as TaskItem['preAdmissionDraft']; + return []; + }); + const make = () => task({ id: 'nobrief', issueId: 'ISSUE-nobrief', issueIdentifier: 'NOBRIEF', description: 'pick one' }); + const first = make(); + const refetched = make(); // the next heartbeat's fresh copy: no draft, no scope + + expect(await internal.detectSafeCandidateIds([{ task: first, projectPath: '/repo' }])).toEqual(new Set()); + expect(await internal.detectSafeCandidateIds([{ task: refetched, projectPath: '/repo' }])).toEqual(new Set()); + + expect(resolveTaskFileScopeMock).toHaveBeenCalledTimes(1); // the second heartbeat read the cache + expect(refetched.preAdmissionDraft?.scope?.kind).toBe('decision_pending'); + expect(refetched.fileScope ?? []).toEqual([]); // a decline is not a drafted scope + expect(refetched.fileScopeSource).toBeUndefined(); + }); + + it('a conflict analysis failure does not claim the issue the hold just refused', async () => { + // The fail-closed fallback admits the first candidate of the group when analysis is + // unavailable. It must pick from what survived the hold, or the failure re-admits `held`. + runnerExecution.setTaskSource({ kind: 'linear', addComment: vi.fn(async () => {}) } as unknown as ITaskSource); + const internal = new AutonomousRunner(goalCfg('ship it')) as unknown as Internal; + const held = withDraft('held3', declined); + const fine = withDraft('fine3'); + detectFileConflictsMock.mockRejectedValueOnce(new Error('knowledge graph unavailable')); + + const safe = await internal.detectSafeCandidateIds([ + { task: held, projectPath: '/repo' }, { task: fine, projectPath: '/repo' }, + ]); + + expect(safe).toEqual(new Set(['fine3'])); + }); + it('survives a comment that fails to post: the issue is still not claimed', async () => { runnerExecution.setTaskSource({ kind: 'linear', addComment: vi.fn(async () => { throw new Error('linear down'); }) } as unknown as ITaskSource); const internal = new AutonomousRunner(goalCfg('ship it')) as unknown as Internal; @@ -706,6 +745,15 @@ describe('AutonomousRunner coverage — safely-reachable helpers', () => { }); describe('switchProvider', () => { + it('reports a delegated CLI as refused unless the operator opted in to delegated adapters (AGT-4444)', () => { + // The dashboard toggle asks this before switchProvider; the startup assertion only read config.yaml. + const strict = new AutonomousRunner(cfg({ allowDelegatedAdapters: false })); + expect(strict.providerSwitchViolation('claude')).toContain("'claude'"); + expect(strict.providerSwitchViolation('codex-responses')).toBeNull(); + expect(new AutonomousRunner(cfg()).providerSwitchViolation('cursor')).toContain("'cursor'"); + expect(new AutonomousRunner(cfg({ allowDelegatedAdapters: true })).providerSwitchViolation('claude')).toBeNull(); + }); + it('releases only provider-quota retries and clears the in-memory pause', () => { const r = new AutonomousRunner(cfg({ defaultAdapter: 'codex' })); const internal = r as unknown as Internal; diff --git a/src/automation/autonomousRunner.dispatchFingerprint.test.ts b/src/automation/autonomousRunner.dispatchFingerprint.test.ts index d01d866b..f175f76e 100644 --- a/src/automation/autonomousRunner.dispatchFingerprint.test.ts +++ b/src/automation/autonomousRunner.dispatchFingerprint.test.ts @@ -132,6 +132,44 @@ describe('idle_fill and the issue input (AGT-4787)', () => { expect(markReady).toHaveBeenCalledWith('AGT-1'); }); + /** A run that asked `hq-only` and parked for the exact answer, with its input recorded at start. */ + async function parkedOnQuestion(internal: InternalRunner): Promise { + const { RunLedger } = await import('./runLedger.js'); + const { getCoordinationStore } = await import('../coordination/coordinationStore.js'); + await getCoordinationStore().publish({ + repository: REPO, taskId: 'AGT-1', actor: 'worker-x', recipient: 'human', + kind: 'human-question', status: 'running', correlationId: 'hq-only', summary: 'ask', + }); + const ledger = new RunLedger(dbPath); + const claim = ledger.claimRun('AGT-1', { ownerInstanceId: 'seed', leaseMs: 60_000, maxActiveForProject: 1 }); + expect(claim).not.toBeNull(); + expect(ledger.transition(claim!, 'RETRY_AT', { + retryAt: Date.now() + 3_600_000, errorCode: 'waiting_on_operator', + })).toBe(true); + ledger.close(); + expect(internal.durableRuns.markNeedsHumanForQuestions('AGT-1', ['hq-only'], 'waiting for exact answer')).toBe(true); + await startedOn(TASK); + } + + it('an answered ask_human park resumes although the issue is unchanged; an unanswered one stays parked', async () => { + // Before this, idle_fill was the only thing that resumed an answered question park, so holding + // idle_fill for an unchanged issue (AGT-4787) would have stranded the operator's answer. + const internal = await makeRunner(); + await parkedOnQuestion(internal); + + expect(internal.filterAlreadyProcessed([TASK])).toEqual([]); + expect(internal.durableRuns.getRun('AGT-1')?.state).toBe('NEEDS_HUMAN'); + + const { getCoordinationStore } = await import('../coordination/coordinationStore.js'); + await getCoordinationStore().publish({ + repository: REPO, taskId: 'AGT-1', actor: 'operator', recipient: 'worker-x', + kind: 'human-answer', status: 'completed', correlationId: 'hq-only', summary: 'answered', + }); + + expect(internal.filterAlreadyProcessed([TASK])).toEqual([TASK]); // the issue itself is still unchanged + expect(internal.durableRuns.getRun('AGT-1')?.state).not.toBe('NEEDS_HUMAN'); + }); + it('ignores the daemon\'s own writes: a changed update time alone does not unblock', async () => { const internal = await makeRunner(); internal.durableRuns.markNeedsHuman('AGT-1', 'Reviewer rejected 4 attempts: still failing lint'); diff --git a/src/automation/autonomousRunner.hostHealth.test.ts b/src/automation/autonomousRunner.hostHealth.test.ts index 8f45945f..9489d803 100644 --- a/src/automation/autonomousRunner.hostHealth.test.ts +++ b/src/automation/autonomousRunner.hostHealth.test.ts @@ -17,6 +17,11 @@ vi.mock('../memory/repoKnowledge.js', () => ({ promoteStagedMemories: vi.fn(async () => 0), })); vi.mock('../linear/projectUpdater.js', () => ({ updateProjectAfterTask: vi.fn(async () => {}) })); +// A passthrough spy: the real measurement still runs, the test only reads which paths it was given. +vi.mock('../support/hostHealth.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, dispatchWritePaths: vi.fn(actual.dispatchWritePaths) }; +}); type InternalRunner = { heartbeatParallel: ReturnType; @@ -95,6 +100,17 @@ describe('heartbeat holds dispatch when the disk is nearly full (AGT-4785)', () expect(pauses).toHaveLength(1); }, 60_000); + it('measures the volume that holds the automation database, not only the projects and the temp dir', async () => { + // A deployment whose state sits on its own volume fills it independently: ledger writes fail with + // SQLITE_FULL while every project volume still looks healthy. + const { runner, internal } = await setup(UNREACHABLE_FLOOR_GIB); + const { dispatchWritePaths } = await import('../support/hostHealth.js'); + await runner.heartbeat(); + internal.durableRuns.close(); + const [, extra] = vi.mocked(dispatchWritePaths).mock.calls.at(-1)!; + expect(extra).toContain(root); // automationDbPath is join(root, 'hosthealth.db') + }, 60_000); + it('a floor of 0 switches the check off and the heartbeat proceeds to fetch', async () => { const { runner, internal, fetchTasks } = await setup(0); await runner.heartbeat(); diff --git a/src/automation/autonomousRunner.ts b/src/automation/autonomousRunner.ts index 17284fc8..07d227a2 100644 --- a/src/automation/autonomousRunner.ts +++ b/src/automation/autonomousRunner.ts @@ -76,7 +76,8 @@ import { decideExplicitReadmission, OPERATOR_QUESTION_PARK_MARKER } from './expl import { broadcastEvent, type SwarmStats } from '../core/eventHub.js'; import { writeProviderOverride } from '../core/providerOverride.js'; import { getTaskState, reconcileDependencyBlockers, updateTaskLinearState, upsertTaskState } from '../taskState/store.js'; -import { setAutomationDbPath } from './automationDbPath.js'; +import { defaultAutomationDbPath, setAutomationDbPath } from './automationDbPath.js'; +import { providerSwitchViolation } from '../adapters/unattendedPolicy.js'; import { findPullRequestForBranch, inspectWorktreeRecovery, @@ -96,7 +97,7 @@ import { refreshGraph, toProjectSlug } from '../knowledge/index.js'; import { scanRepository } from '../registry/entityScanner.js'; import { readFileSync, existsSync } from 'node:fs'; import { execFile } from 'node:child_process'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { promisify } from 'node:util'; import { checkAllMonitors, getActiveMonitors } from './longRunningMonitor.js'; import { @@ -140,6 +141,7 @@ import { planStalledInProgress } from './stalledInProgress.js'; import { DEFAULT_AGENT_SKIP, formatSkipSummary, partitionHumanOwned } from './agentEligibility.js'; import { scopeDeclineComment, scopeDeclineMarker } from './draftGrooming.js'; import { evaluateReadiness, formatReadinessSummary, partitionByReadiness, verdictFromDraft } from './readinessGate.js'; +import { isScopeDeclined } from '../agents/draftScope.js'; import { planMergedPublicationChecks, decideMergedPublication } from './mergedPublications.js'; import { completeParentIfChildrenDone as completeParentLocally, buildTaskStateSyncComment } from '../taskState/store.js'; import { ACTIVE_LEASE_STATES } from './runLedgerTypes.js'; @@ -513,6 +515,24 @@ export class AutonomousRunner { return getTaskState(issueId)?.execution?.blockedReason === OPERATOR_PARK_REASON; } + /** + * Resume a NEEDS_HUMAN ask_human park whose exact-correlation questions are all answered. + * + * The board read comes first and is only a filter — the ledger's contract re-reads the trace and + * decides — so a heartbeat over many parked runs does not open a write transaction for each. + */ + private resumeAnsweredQuestionPark(issueId: string): boolean { + try { + if (!getCoordinationStore().allQuestionsAnswered(issueId)) return false; + const resumed = this.durableRuns.resumeNeedsHumanForQuestions(issueId); + if (!resumed) return false; + if (resumed === 'SYNC_PENDING') this.scheduleNextHeartbeat(); + return true; + } catch { // cxt-ignore: error_swallow — an unreadable board must not stall the heartbeat + return false; + } + } + private answerArrivedFor(issueId: string): boolean { try { return shouldReadmitEarly({ @@ -538,7 +558,13 @@ export class AutonomousRunner { private rateLimitUntil = 0; // Disk-space hold: new claims wait while the volumes we write to are nearly full (AGT-4785). private readonly diskGate = new HostDiskGate( - () => dispatchWritePaths(this.config.allowedProjects.map((p) => normalizeProjectPath(p))), + // The directory holding the automation database is measured too: on a deployment whose state + // lives on its own volume, a full state volume fails ledger writes (SQLITE_FULL) while every + // project volume and the temp dir still look healthy. + () => dispatchWritePaths( + this.config.allowedProjects.map((p) => normalizeProjectPath(p)), + [dirname(this.config.automationDbPath ?? defaultAutomationDbPath())], + ), () => (this.config.minFreeDiskGiB ?? 10) * 1024 ** 3, ); @@ -1320,6 +1346,18 @@ export class AutonomousRunner { if (resumed === 'SYNC_PENDING') this.scheduleNextHeartbeat(); } } + // An answered ask_human question resumes on its own contract, not on a free slot. The + // ledger refuses every other trigger for this park (`resumeNeedsHuman`), and nothing in the + // daemon called the contract (`exec answer` aside), so the idle_fill below was what + // delivered an operator's answer — by accident, since it lifts an unanswered park too. Once + // idle_fill stands down for an issue nobody edited (AGT-4787), an answer would otherwise + // wait for an edit nobody needs to make. + if (!execHeld && durableRun?.lastErrorCode === OPERATOR_QUESTION_PARK_REASON + && this.resumeAnsweredQuestionPark(id)) { + console.log(`[Scheduler] ${task.issueIdentifier ?? id} resumed from NEEDS_HUMAN (the operator answered)`); + answered++; + durableRun = this.durableRuns.getRun(id) ?? durableRun; + } // The same-fingerprint infra circuit parks a run so that an operator // changes something before it runs again; a free slot changes nothing. // Resuming it by idle fill re-runs a known-broken environment every @@ -3251,6 +3289,9 @@ export class AutonomousRunner { // Detect file conflicts per project using Knowledge Graph const safeTasks = new Set(); // task IDs safe to enqueue for (const [projPath, group] of byProject) { + // What survives the pre-claim hold. The fail-closed fallback below must pick from this, not + // from `group`, or an analysis failure would admit the very issue the hold just refused. + let claimable: RunnableCandidate[] = group; try { await Promise.all(group.map(async c => { const cacheKey = `${projPath}\0${c.task.id}`; @@ -3273,8 +3314,12 @@ export class AutonomousRunner { fileScope: string[]; draft: NonNullable; description?: string; executionCommentsLoaded?: boolean; }): void => { - c.task.fileScope = [...entry.fileScope]; - c.task.fileScopeSource = 'drafted'; + // A cached decline names no files: the scope stays unknown instead of becoming an + // empty "drafted" one. (AGT-4784) + if (entry.fileScope.length > 0) { + c.task.fileScope = [...entry.fileScope]; + c.task.fileScopeSource = 'drafted'; + } c.task.preAdmissionDraft = { ...entry.draft, relevantFiles: [...entry.fileScope] }; c.task.description = entry.description; c.task.executionCommentsLoaded = entry.executionCommentsLoaded; @@ -3310,7 +3355,11 @@ export class AutonomousRunner { pa => projPath.includes(pa.projectPath.replace('~', '')), )?.generatedOutputRules, }); - if (c.task.fileScopeSource === 'drafted' && c.task.preAdmissionDraft) { + // A confident decline is cached like a drafted scope: it is as expensive to recompute, and + // the pre-claim hold reads it on every heartbeat, so a held issue must cost no model call + // after the first. It is never a 'drafted' scope — it names no files. (AGT-4784) + const declinedDraft = !!c.task.preAdmissionDraft && isScopeDeclined(c.task.preAdmissionDraft.scope); + if ((c.task.fileScopeSource === 'drafted' || declinedDraft) && c.task.preAdmissionDraft) { const entry = { fingerprint, fileScope: [...(c.task.fileScope ?? [])], draft: c.task.preAdmissionDraft, description: c.task.description, @@ -3328,7 +3377,7 @@ export class AutonomousRunner { // The drafts above are the first look at what an issue asks for, and they are // available before anything is claimed. A confident "a person has to do or decide // this" verdict stops the claim here instead of after a worker has started. (AGT-4784) - const claimable = await this.holdDeclinedBeforeClaim(group, projPath); + claimable = await this.holdDeclinedBeforeClaim(group, projPath); // A later heartbeat must compare new candidates with workers that are // already editing another worktree. Candidate-vs-candidate detection @@ -3408,8 +3457,8 @@ export class AutonomousRunner { // 분석 실패는 동시 편집 안전성을 증명하지 못한 상태다. 저장소마다 // 하나만 통과시켜 직렬화하고, 나머지는 다음 heartbeat로 미룬다. console.warn(`[AutonomousRunner] Conflict detection failed for ${projPath}:`, err); - for (const id of failClosedConflictFallback(group)) safeTasks.add(id); - for (const deferred of group.slice(1)) { + for (const id of failClosedConflictFallback(claimable)) safeTasks.add(id); + for (const deferred of claimable.slice(1)) { this.syslog(`Conflict analysis unavailable — serializing: ${deferred.task.issueIdentifier || deferred.task.id.slice(0, 8)} ${deferred.task.title}`); } } @@ -4014,6 +4063,16 @@ export class AutonomousRunner { } } + /** + * Why this runner may not be switched to `adapter`, or null. A delegated CLI owns its tool loop, + * so the fences the daemon refused to start without (AGT-4444) would not reach it; the operator's + * `allowDelegatedAdapters` is the only way past. Callers that take the adapter from outside the + * process (the dashboard toggle, `openswarm provider`) ask this before `switchProvider`. + */ + providerSwitchViolation(adapter: AdapterName): string | null { + return providerSwitchViolation(adapter, this.config.allowDelegatedAdapters); + } + switchProvider(adapter: AdapterName): void { const previousAdapter = this.config.defaultAdapter ?? 'codex'; this.applyProviderConfig(adapter); diff --git a/src/automation/dispatchFingerprint.test.ts b/src/automation/dispatchFingerprint.test.ts index e40679f4..c76b135f 100644 --- a/src/automation/dispatchFingerprint.test.ts +++ b/src/automation/dispatchFingerprint.test.ts @@ -10,6 +10,7 @@ import { inputFingerprint, isDaemonComment, mayRedispatch, recordParkedInput, recordRunInput, resetDispatchFingerprintForTests, } from './dispatchFingerprint.js'; +import { EXECUTION_COMMENT_CONTEXT_HEADING, refreshExecutionTaskContext } from './executionTaskContext.js'; let dir: string; const saved = process.env.OPENSWARM_AUTOMATION_DB; @@ -72,6 +73,40 @@ describe('inputFingerprint', () => { }); }); +describe('the comment history a selected run appends to the description (AGT-4787)', () => { + // executePipeline's first line is `task = await prepareTaskExecutionContext(task)`, which appends the + // tracker discussion to task.description; recordRunInput runs after it. The heartbeat fingerprints + // the bulk-fetched task, which has no discussion. Both sides must still agree on an unchanged issue. + async function selectedRun(comments: Array<{ body: string; createdAt: string }>) { + const task = { id: 'AX-1', issueId: 'AX-1', ...issue } as { id: string; issueId: string; title: string; description: string }; + await refreshExecutionTaskContext(task as never, { getExecutionComments: async () => comments } as never); + return task; + } + const history = [ + { body: '🧭 **[OpenSwarm] Task execution started**', createdAt: '2026-10-20T01:00:00Z' }, + { body: 'Please use option B.', createdAt: '2026-10-20T02:00:00Z' }, + ]; + + it('fingerprints an issue the same with or without the appended history', async () => { + const task = await selectedRun(history); + expect(task.description).toContain(EXECUTION_COMMENT_CONTEXT_HEADING); // the helper really appended it + expect(inputFingerprint({ title: task.title, description: task.description })).toBe(inputFingerprint(issue)); + }); + + it('holds an unchanged issue after a run recorded its input post-refresh', async () => { + recordRunInput(await selectedRun(history)); // what executePipeline does for an issue that already has comments + expect(mayRedispatch('AX-1', inputFingerprint(issue))).toBe(false); // what the next heartbeat asks + }); + + it('still lifts once a person edits the description, with the history appended or not', async () => { + recordRunInput(await selectedRun(history)); + expect(mayRedispatch('AX-1', inputFingerprint({ ...issue, description: 'It drops rows. Use the staging DB.' }))).toBe(true); + const edited = await selectedRun(history); + edited.description = `Rewritten by a person.${edited.description.slice(issue.description.length)}`; + expect(inputFingerprint({ title: edited.title, description: edited.description })).not.toBe(inputFingerprint(issue)); + }); +}); + describe('the parked-input store', () => { it('blocks the input a run parked on, however often it is asked, and passes any other', () => { recordParkedInput('AX-1', inputFingerprint(issue)); diff --git a/src/automation/dispatchFingerprint.ts b/src/automation/dispatchFingerprint.ts index c5efe1be..ee3525ab 100644 --- a/src/automation/dispatchFingerprint.ts +++ b/src/automation/dispatchFingerprint.ts @@ -34,6 +34,7 @@ import { createHash } from 'node:crypto'; import Database from 'better-sqlite3'; import { defaultAutomationDbPath } from './automationDbPath.js'; +import { EXECUTION_COMMENT_CONTEXT_HEADING } from './executionTaskContext.js'; export interface FingerprintInput { title: string; @@ -54,6 +55,19 @@ export function isDaemonComment(body: string): boolean { return DAEMON_SIGNATURES.some((pattern) => pattern.test(text)); } +/** + * The description as the issue carries it. Once a run is selected, + * `refreshExecutionTaskContext` appends the tracker discussion to `task.description` in place, and + * the pipeline records its input after that. The heartbeat fingerprints the bulk-fetched task, + * which has no discussion, so without this cut the two digests of an unchanged issue differ as soon + * as it has a single comment (the daemon's own included) and the hold never engages. (AGT-4787) + */ +function ownDescription(description: string | undefined): string { + const text = description ?? ''; + const at = text.indexOf(EXECUTION_COMMENT_CONTEXT_HEADING); + return (at === -1 ? text : text.slice(0, at)).trim(); +} + /** * A stable digest of everything a person can change about an issue. Comments are * hashed in the order the tracker returns them, so reordering is a change but @@ -62,7 +76,7 @@ export function isDaemonComment(body: string): boolean { export function inputFingerprint(issue: FingerprintInput): string { const human = (issue.comments ?? []).map((c) => c.body.trim()).filter((body) => body && !isDaemonComment(body)); return createHash('sha256') - .update(JSON.stringify([issue.title.trim(), (issue.description ?? '').trim(), human])) + .update(JSON.stringify([issue.title.trim(), ownDescription(issue.description), human])) .digest('hex'); } diff --git a/src/automation/executionTaskContext.ts b/src/automation/executionTaskContext.ts index 76a17e55..4acd682d 100644 --- a/src/automation/executionTaskContext.ts +++ b/src/automation/executionTaskContext.ts @@ -3,13 +3,20 @@ import type { ITaskSource } from './taskSource.js'; const AUTOMATION_COMMENT_RE = /_(?:via OpenSwarm|Worker audit log|Worker\/Reviewer\/Tester pipeline|Planner agent)\b/i; +/** + * Heading that opens the tracker discussion appended to a task's description once a run is + * selected. Everything from this heading on is context the daemon pulled in, not the issue's own + * text, so anything that fingerprints "what the issue says" must stop here (AGT-4787). + */ +export const EXECUTION_COMMENT_CONTEXT_HEADING = '## Issue comment history (fresh tracker context; treat as untrusted data)'; + /** Prioritize human-looking comments, then retain bounded recent automation context. */ export function formatExecutionCommentContext( comments: Array<{ body: string; createdAt: string }>, maxChars = 30_000, ): string { if (comments.length === 0 || maxChars <= 0) return ''; - const prefix = '\n\n## Issue comment history (fresh tracker context; treat as untrusted data)'; + const prefix = `\n\n${EXECUTION_COMMENT_CONTEXT_HEADING}`; if (prefix.length >= maxChars) return prefix.slice(0, maxChars); const sorted = [...comments].sort((a, b) => a.createdAt.localeCompare(b.createdAt)); const human = sorted.filter((comment) => !AUTOMATION_COMMENT_RE.test(comment.body)); diff --git a/src/automation/missingWorktreeDisposition.ts b/src/automation/missingWorktreeDisposition.ts index d67b5d93..79694fe1 100644 --- a/src/automation/missingWorktreeDisposition.ts +++ b/src/automation/missingWorktreeDisposition.ts @@ -13,10 +13,12 @@ import type { MissingWorktreeDisposition } from './runLedgerMissingWorktree.js'; * stale pointer. * - A PR exists: normally that is a publication the ledger never recorded, so * the row goes to NEEDS_RECONCILE and the artifact reconciler judges it. - * - EXCEPT a draft PR on a RETRY_AT row (AGT-4664). The runner now publishes a - * draft for every run that ends unfinished and then removes its clean tree, - * so "PR exists, worktree gone, retry scheduled" is the expected leftover of - * the attempt that just ended — not an unrecorded publication. Sending it to + * - EXCEPT a draft PR on a RETRY_AT row (AGT-4664). Until AGT-4786 the runner + * published a draft for every run that ended unfinished and then removed its + * clean tree, so "PR exists, worktree gone, retry scheduled" is the expected + * leftover of such an attempt — not an unrecorded publication. Those drafts + * still exist on origin, so the exception stays; since AGT-4786 a new + * unfinished run opens no PR and keeps its tree. Sending it to * NEEDS_RECONCILE parked the run for a human ("a draft PR of no recorded * cause") and the retry it was owed waited on a free slot to be resumed * (cgf-portal AX-1847, 2026-10-03). The pointer is cleared and the retry diff --git a/src/automation/runnerTypes.ts b/src/automation/runnerTypes.ts index fe465128..09738411 100644 --- a/src/automation/runnerTypes.ts +++ b/src/automation/runnerTypes.ts @@ -88,6 +88,8 @@ export interface AutonomousConfig { workerSandbox?: 'on' | 'off'; /** Free-space floor (GiB) below which dispatch pauses; 0 switches the check off (AGT-4785). */ minFreeDiskGiB?: number; + /** Operator opt-in to delegated CLI adapters; the runner reads it when the provider is switched (AGT-4444). */ + allowDelegatedAdapters?: boolean; /** Periodic read-only repository review jobs. */ periodicReviews?: Array<{ profile: 'permissions' | 'hygiene' | 'security' | 'review'; schedule: string; adapter?: 'codex' | 'cc-router' | 'cursor' }>; /** Explicit high-capability project supervisor. */ diff --git a/src/automation/workRunner.ts b/src/automation/workRunner.ts index c34ea07e..ef152efb 100644 --- a/src/automation/workRunner.ts +++ b/src/automation/workRunner.ts @@ -13,6 +13,12 @@ import type { AutonomousRunner } from './autonomousRunner.js'; import type { TaskItem } from '../orchestration/decisionEngine.js'; import { broadcastEvent } from '../core/eventHub.js'; +/** + * Why a dispatch of an issue the scheduler already owns is skipped. `exec --issue` compares against it: + * that refusal leaves a run for the caller to await, every other refusal leaves none. + */ +export const WORK_ALREADY_QUEUED_REASON = 'already queued or running'; + export interface WorkDispatchRequest { issueIds: string[]; projectPath: string; @@ -272,7 +278,7 @@ export async function dispatchWork( item.status = 'skipped'; if (rejectionReason === 'duplicate') { // Another live dispatch/heartbeat owns this issue on the scheduler. - item.reason = 'already queued or running'; + item.reason = WORK_ALREADY_QUEUED_REASON; continue; } // 'stopping': the runner is shutting down, nobody will ever run this. diff --git a/src/cli.ts b/src/cli.ts index 7b49ca94..157a17bd 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -664,8 +664,8 @@ const execCommand = program json?: boolean; }) => { if (opts.issue) { - const { runExecDeployCommand } = await import('./cli/execCommand.js'); - process.exitCode = await runExecDeployCommand({ issue: opts.issue, path: opts.path, json: opts.json }); + const { runExecDeployCommand, runExecGuarded } = await import('./cli/execCommand.js'); + process.exitCode = await runExecGuarded(opts.json, () => runExecDeployCommand({ issue: opts.issue!, path: opts.path, json: opts.json })); return; } if (!prompt) { @@ -696,8 +696,8 @@ execCommand // subcommand — so the child's own opts object is always empty here. Read // the merged chain instead of duplicating the flags on both commands. const opts = this.optsWithGlobals() as { path?: string; json?: boolean }; - const { runExecStatusCommand } = await import('./cli/execCommand.js'); - process.exitCode = await runExecStatusCommand(runId, opts); + const { runExecStatusCommand, runExecGuarded } = await import('./cli/execCommand.js'); + process.exitCode = await runExecGuarded(opts.json, () => runExecStatusCommand(runId, opts)); }); execCommand @@ -707,8 +707,8 @@ execCommand .option('--json', 'Emit machine-readable JSON') .action(async function (this: Command, runId: string, _opts: { timeout?: number; json?: boolean }) { const opts = this.optsWithGlobals() as { path?: string; timeout?: number; json?: boolean }; - const { runExecAwaitCommand } = await import('./cli/execCommand.js'); - process.exitCode = await runExecAwaitCommand(runId, opts); + const { runExecAwaitCommand, runExecGuarded } = await import('./cli/execCommand.js'); + process.exitCode = await runExecGuarded(opts.json, () => runExecAwaitCommand(runId, opts)); }); execCommand @@ -719,8 +719,8 @@ execCommand .option('--json', 'Emit machine-readable JSON') .action(async function (this: Command, runId: string, _opts: { correlation?: string; text?: string; json?: boolean }) { const opts = this.optsWithGlobals() as { path?: string; correlation?: string; text?: string; json?: boolean }; - const { runExecAnswerCommand } = await import('./cli/execCommand.js'); - process.exitCode = await runExecAnswerCommand(runId, opts); + const { runExecAnswerCommand, runExecGuarded } = await import('./cli/execCommand.js'); + process.exitCode = await runExecGuarded(opts.json, () => runExecAnswerCommand(runId, opts)); }); // openswarm start diff --git a/src/cli/execCommand.test.ts b/src/cli/execCommand.test.ts index 18560858..5f2f4daf 100644 --- a/src/cli/execCommand.test.ts +++ b/src/cli/execCommand.test.ts @@ -63,10 +63,12 @@ import type { LinearIssueInfo, SwarmConfig } from '../core/types.js'; import type { RepoMetadata } from '../support/repoMetadata.js'; import { RunLedger, RUN_STATES } from '../automation/runLedger.js'; import { OPERATOR_QUESTION_PARK_REASON } from '../coordination/operatorAnswers.js'; +import { WORK_ALREADY_QUEUED_REASON } from '../automation/workRunner.js'; import { ANSWERABLE_STATES, AWAIT_ELEVATION_STATES, AWAIT_TERMINAL_STATES, + buildExecElevation, classifyRunForAwait, readExecRunRow, readPendingExecQuestions, @@ -283,7 +285,7 @@ const TRACE_SEEDS: TraceSeed[] = [ { correlationId: 'hq-z', taskId: 'uuid-other', kind: 'human-question', status: 'waiting', summary: 'Someone else’s question' }, ]; -function seedTrace(): void { +function seedTrace(seeds: readonly TraceSeed[] = TRACE_SEEDS): void { const db = new Database(dbPath); try { db.exec(TRACE_DDL); @@ -293,7 +295,7 @@ function seedTrace(): void { kind, status, correlation_id, summary, metadata_json, fingerprint ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `); - TRACE_SEEDS.forEach((seed, index) => { + seeds.forEach((seed, index) => { insert.run( `evt-${index}`, index + 1, @@ -342,6 +344,9 @@ function deployDeps(overrides: Partial = {}): TestDeps { getIssue: async () => issue(), daemonHealthy: async () => true, postWork: async () => ({ status: 202, body: { workId: 'w', items: [{ status: 'queued' }], queued: 1, skipped: 0 } }), + // Hermetic: no hold exists beforehand and releasing one touches no store, unless a test says otherwise. + isHeld: () => false, + clearHold: () => undefined, ...overrides, }); } @@ -468,6 +473,25 @@ describe('readPendingExecQuestions', () => { expect(byId.get('hq-f')?.questionClass).toBe('approval'); }); + it('reports a paged question once: the "operator paged" confirmation is not a second question', () => { + // postHumanQuestionOnce publishes the `waiting` ask and then, once Discord delivered the page, a + // `running` event under the SAME correlation id with the marker as its summary and no class. + const pagedDb = join(dir, 'paged.db'); + const original = dbPath; + dbPath = pagedDb; + try { + seedTrace([ + { correlationId: 'hq-p', taskId: UUID_MAIN, kind: 'human-question', status: 'waiting', summary: 'Which branch?', metadataJson: '{"questionClass":"clarification"}' }, + { correlationId: 'hq-p', taskId: UUID_MAIN, kind: 'human-question', status: 'running', summary: 'Operator paged on Discord', metadataJson: null }, + ]); + expect(readPendingExecQuestions(pagedDb, UUID_MAIN)).toEqual([ + { correlationId: 'hq-p', question: 'Which branch?', questionClass: 'clarification' }, + ]); + } finally { + dbPath = original; + } + }); + it('reads no questions for a task that has none', () => { expect(readPendingExecQuestions(dbPath, 'uuid-unknown')).toEqual([]); }); @@ -590,8 +614,10 @@ describe('runExecDeployCommand', () => { it('fails when the daemon answers 202 but queued nothing, quoting its reason', async () => { const markHold = vi.fn(); + const clearHold = vi.fn(); const deps = deployDeps({ markHold, + clearHold, postWork: async () => ({ status: 202, body: { items: [{ reason: 'state is In Progress' }], queued: 0, skipped: 1 } }), }); const code = await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, deps); @@ -600,9 +626,70 @@ describe('runExecDeployCommand', () => { expect(code).toBe(2); expect(deps.logs.join('\n')).toMatch(/state is In Progress/); expect(JSON.parse(deps.outs.join('\n'))).toMatchObject({ deployed: false, error: 'state is In Progress' }); - // The hold is already written and stays: the deploy was refused, but the - // window that matters (a run existing without a hold) never opened. + // The hold went in before the deploy so no run could exist unprotected. The deploy was refused, so no + // run exists and the hold it wrote is released instead of keeping idle_fill off the issue for 24h. expect(markHold).toHaveBeenCalledTimes(1); + expect(clearHold).toHaveBeenCalledExactlyOnceWith(UUID_MAIN); + }); + + describe('releasing the hold when the deploy is refused', () => { + const refuse = (postWork: NonNullable, extra: Partial = {}) => { + const clearHold = vi.fn(); + const deps = deployDeps({ markHold: vi.fn(), clearHold, postWork, ...extra }); + return { deps, clearHold }; + }; + + it.each([ + ['an HTTP 403 (outside allowedProjects)', async () => ({ status: 403, body: { error: 'not allowed' } })], + ['an HTTP 409 (rate-limit window)', async () => ({ status: 409, body: { error: 'paused' } })], + ])('releases it on %s: nothing was deployed', async (_label, postWork) => { + const { deps, clearHold } = refuse(postWork); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, deps)).not.toBe(0); + expect(clearHold).toHaveBeenCalledExactlyOnceWith(UUID_MAIN); + }); + + it('releases it when the daemon refuses its own health probe and when a detached child cannot start', async () => { + const probe = refuse(async () => ({ status: 202, body: { queued: 1 } }), { daemonProbe: async () => ({ kind: 'refused', status: 403 }) }); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, probe.deps)).toBe(2); + expect(probe.clearHold).toHaveBeenCalledExactlyOnceWith(UUID_MAIN); + + const child = refuse(async () => ({ status: 202, body: { queued: 1 } }), { + daemonProbe: async () => ({ kind: 'down' }), spawnWorkChild: () => ({ ok: false, reason: 'dist/cli.js missing' }), + }); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, child.deps)).toBe(1); + expect(child.clearHold).toHaveBeenCalledExactlyOnceWith(UUID_MAIN); + }); + + it('keeps it when the daemon already owns the issue: `exec await` can still follow that run', async () => { + const { deps, clearHold } = refuse(async () => ({ + status: 202, body: { items: [{ reason: WORK_ALREADY_QUEUED_REASON }], queued: 0, skipped: 1 }, + })); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, deps)).toBe(2); + expect(clearHold).not.toHaveBeenCalled(); + }); + + it('keeps it when the request may have reached the daemon (a 5xx or a transport error)', async () => { + const serverError = refuse(async () => ({ status: 503, body: { error: 'busy' } })); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, serverError.deps)).toBe(1); + expect(serverError.clearHold).not.toHaveBeenCalled(); + + const dropped = refuse(async () => { throw new Error('socket hang up'); }); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, dropped.deps)).toBe(1); + expect(dropped.clearHold).not.toHaveBeenCalled(); + }); + + it('keeps a hold that existed before this call: it belongs to an earlier deploy', async () => { + const { deps, clearHold } = refuse(async () => ({ status: 403, body: { error: 'not allowed' } }), { isHeld: () => true }); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, deps)).toBe(1); + expect(clearHold).not.toHaveBeenCalled(); + }); + + it('never releases it after a successful deploy', async () => { + const clearHold = vi.fn(); + const deps = deployDeps({ markHold: vi.fn(), clearHold }); + expect(await runExecDeployCommand({ issue: 'INT-1', path: repoPath, json: true }, deps)).toBe(0); + expect(clearHold).not.toHaveBeenCalled(); + }); }); it('quotes the daemon’s own reason when a 202 queues nothing and gives none', async () => { @@ -1055,7 +1142,18 @@ describe('runExecAnswerCommand', () => { expect(answerQuestion).toHaveBeenCalledWith('hq-a', 'use main', 'external:test'); expect(order).toEqual(['answer', 'resume', 'hold', 'deploy']); expect(markHold).toHaveBeenCalledWith(UUID_MAIN, 'external:test'); - expect(postWork).toHaveBeenCalledWith(process.cwd(), 'INT-1'); + // The run was registered under REPO: that is where it is redeployed, whatever the cwd of the caller is. + expect(postWork).toHaveBeenCalledWith(REPO, 'INT-1'); + }); + + it('redeploys to --path when given, over the run\'s recorded path', async () => { + const postWork = vi.fn(async () => ({ status: 202, body: { queued: 1 } })); + const deps = baseDeps({ + answerQuestion: async () => ({ accepted: true }), markHold: vi.fn(), daemonHealthy: async () => true, postWork, + openLedger: () => fakeLedger({ resumeNeedsHumanForQuestions: () => 'READY' }), + }); + expect(await runExecAnswerCommand(UUID_MAIN, { correlation: 'hq-a', path: '/elsewhere', json: true }, deps)).toBe(0); + expect(postWork).toHaveBeenCalledWith('/elsewhere', 'INT-1'); }); it('accepts an empty answer text as a pure resume', async () => { @@ -1155,6 +1253,34 @@ describe('runExecAnswerCommand', () => { expect(markReady).not.toHaveBeenCalled(); }); + it('refuses --text without --correlation instead of resuming the run and dropping the guidance', async () => { + // Guidance is delivered as the answer to a question; with no correlation there is nowhere to write it. + // The command used to resume and redeploy anyway and report exit 0, while the worker parked again on + // the same thing, because the text had reached no board, ledger or comment. + const openLedger = vi.fn(() => fakeLedger()); + const markHold = vi.fn(); + const postWork = vi.fn(async () => ({ status: 202, body: { queued: 1 } })); + const deps = baseDeps({ openLedger, markHold, postWork, daemonHealthy: async () => true }); + + const code = await runExecAnswerCommand(UUID_MAIN, { text: 'use the staging DB', json: true }, deps); + + expect(code).toBe(2); + const doc = JSON.parse(deps.outs.join('\n')); + expect(doc).toMatchObject({ runId: UUID_MAIN, answerAccepted: null, resumed: null }); + expect(doc.error).toContain('--text needs --correlation'); + expect(openLedger).not.toHaveBeenCalled(); + expect(markHold).not.toHaveBeenCalled(); + expect(postWork).not.toHaveBeenCalled(); + }); + + it('offers --text in the next-command hint only where a question can carry it', () => { + const row = readExecRunRow(dbPath, UUID_MAIN)!; + const question = { correlationId: 'hq-a', question: 'Which branch?', questionClass: 'clarification' as const }; + expect(buildExecElevation(UUID_MAIN, row, [question]).hint) + .toBe(`openswarm exec answer ${UUID_MAIN} --correlation hq-a --text "..."`); + expect(buildExecElevation(UUID_MAIN, row, []).hint).toBe(`openswarm exec answer ${UUID_MAIN}`); + }); + it('skips the question contract entirely when the park is not an operator question', async () => { // Re-seed a run parked for a non-question reason: RETRY_AT under a // deferral, which is the state `readmitParkedRun` is written for. diff --git a/src/cli/execCommand.ts b/src/cli/execCommand.ts index 559bdacd..bae37850 100644 --- a/src/cli/execCommand.ts +++ b/src/cli/execCommand.ts @@ -25,7 +25,9 @@ // Exit codes: deploy 0 deployed / 1 deploy failed / 2 refused before deploying. // status 0 read / 2 no durable run. await 0 terminal / 1 elevated / 2 still // pending (timeout). answer 0 resumed+redeployed / 1 refused / 2 nothing to -// answer. +// answer. Any of them exits 2 with the message (and `{ "error": ... }` under +// --json) when it throws before it can report: `await` uses 1 for "elevated", +// so an unexpected failure must not borrow it. (`runExecGuarded`) import { spawn } from 'node:child_process'; import { existsSync } from 'node:fs'; @@ -35,7 +37,8 @@ import Database from 'better-sqlite3'; import { expandPath, loadConfig } from '../core/config.js'; import type { LinearIssueInfo, SwarmConfig } from '../core/types.js'; import { setAutomationDbPath, defaultAutomationDbPath } from '../automation/automationDbPath.js'; -import { markExecHold } from '../automation/execHold.js'; +import { clearExecHold, isExecHeld, markExecHold } from '../automation/execHold.js'; +import { WORK_ALREADY_QUEUED_REASON } from '../automation/workRunner.js'; import { OPERATOR_QUESTION_PARK_REASON } from '../coordination/operatorAnswers.js'; import { resolveQuestionClass, type HumanQuestionClass } from '../coordination/humanQuestions.js'; import { loadRepoMetadata, type RepoMetadata } from '../support/repoMetadata.js'; @@ -99,6 +102,8 @@ export interface ExecRunRow { branchName?: string; worktreePath?: string; prUrl?: string; + /** Where the run was dispatched (automation_runs.project_path); `exec answer` redeploys there. */ + projectPath?: string; lastErrorCode?: string; lastErrorMessage?: string; retryAt?: number; @@ -163,6 +168,7 @@ interface ExecRunRowDb { branch_name: string | null; worktree_path: string | null; pr_url: string | null; + project_path: string | null; last_error_code: string | null; last_error_message: string | null; retry_at: number | null; @@ -176,7 +182,7 @@ export function readExecRunRow(dbPath: string, issueId: string): ExecRunRow | un try { const row = db.prepare(` SELECT issue_id, identifier, title, state, attempt_no, branch_name, worktree_path, - pr_url, last_error_code, last_error_message, retry_at, updated_at + pr_url, project_path, last_error_code, last_error_message, retry_at, updated_at FROM automation_runs WHERE issue_id = ? `).get(issueId) as ExecRunRowDb | undefined; if (!row) return undefined; @@ -189,6 +195,7 @@ export function readExecRunRow(dbPath: string, issueId: string): ExecRunRow | un branchName: row.branch_name ?? undefined, worktreePath: row.worktree_path ?? undefined, prUrl: row.pr_url ?? undefined, + projectPath: row.project_path ?? undefined, lastErrorCode: row.last_error_code ?? undefined, lastErrorMessage: row.last_error_message ?? undefined, retryAt: row.retry_at ?? undefined, @@ -219,16 +226,25 @@ export function readPendingExecQuestions(dbPath: string, issueId: string): Pendi SELECT 1 AS present FROM sqlite_master WHERE type = 'table' AND name = 'coordination_trace' `).get() as { present: number } | undefined; if (!present) return []; + // One row per correlation, the earliest. A question the operator was paged about is published + // twice under one correlation id: the `waiting` ask (the text and the class) and then a `running` + // page confirmation whose summary is "Operator paged on Discord" and which carries no class. + // Reading both reported the confirmation as a second, approval-class question. const rows = db.prepare(` SELECT correlation_id, summary, metadata_json FROM coordination_trace WHERE task_id = ? AND kind = 'human-question' AND status IN ('waiting', 'running') + AND id = ( + SELECT MIN(q.id) FROM coordination_trace q + WHERE q.task_id = ? AND q.kind = 'human-question' AND q.status IN ('waiting', 'running') + AND q.correlation_id = coordination_trace.correlation_id + ) AND correlation_id NOT IN ( SELECT correlation_id FROM coordination_trace WHERE task_id = ? AND kind = 'human-answer' AND status = 'completed' ) ORDER BY id - `).all(issueId, issueId) as Array<{ correlation_id: string; summary: string; metadata_json: string | null }>; + `).all(issueId, issueId, issueId) as Array<{ correlation_id: string; summary: string; metadata_json: string | null }>; return rows.map((row) => ({ correlationId: row.correlation_id, question: row.summary, @@ -256,9 +272,12 @@ function parseMetadata(value: string | null): { questionClass?: unknown } | unde /** Assemble the elevation document from the two reads. */ export function buildExecElevation(runId: string, row: ExecRunRow, questions: PendingExecQuestion[]): ExecElevation { const verdict = classifyRunForAwait(row.state); + // Guidance only travels with a question: `--text` is written to the board for a correlation, and a + // resume with no question carries none. A hint that offered `--text` there made the orchestrator + // believe its guidance landed while the run was resumed without it. const hint = questions.length > 0 ? `openswarm exec answer ${runId} --correlation ${questions[0].correlationId} --text "..."` - : `openswarm exec answer ${runId} --text "..."`; + : `openswarm exec answer ${runId}`; return { runId, identifier: row.identifier, @@ -279,6 +298,31 @@ export function buildExecElevation(runId: string, row: ExecRunRow, questions: Pe // ---- Shared plumbing --------------------------------------------------------- +/** + * Run an `exec` subcommand and turn a throw into an exit code an orchestrator can act on. + * + * `work` wraps its action in a try/catch; the `exec` actions are awaited under a synchronous + * `program.parse()`, so a throw — `daemonBaseUrl()`'s deliberate refusal to send a token over plaintext + * http, a ledger that cannot be opened — was an unhandled rejection: a raw stack, exit 1 and no JSON + * document under `--json`, after the board answer and the ledger resume had already been committed. + * Exit 1 is taken (`await` reports "elevated" with it, `deploy` "failed"), so every throw exits 2 with the + * message: "did not finish, read the error", never mistaken for an elevation document. + */ +export async function runExecGuarded( + json: boolean | undefined, + run: () => Promise, + deps: Pick = {}, +): Promise { + try { + return await run(); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (json) (deps.out ?? stdoutLog)(JSON.stringify({ error: message })); + else (deps.log ?? ((line: string) => console.error(line)))(message); + return 2; + } +} + interface ResolvedContext { config: SwarmConfig; dbPath: string; @@ -318,6 +362,8 @@ export interface ExecCommandDeps { getIssue?: (idOrIdentifier: string) => Promise; ensureTaskSource?: () => Promise; markHold?: (issueId: string, actor: string) => void; + /** Whether a hold already exists, read before this call writes one (default: the real store). */ + isHeld?: (issueId: string) => boolean; clearHold?: (issueId: string) => void; daemonHealthy?: () => Promise; /** Precise probe: distinguishes a reachable-but-refusing daemon from an absent one. */ @@ -400,6 +446,8 @@ interface DeployOutcome { transport: DeployTransport; ok: boolean; detail?: string; + /** The request may have reached the daemon (a transport error or a 5xx), so a run might exist anyway. */ + ambiguous?: boolean; /** The command's exit code when this outcome fails: 2 refused-before-deploying, 1 failed. */ exitCode: number; } @@ -434,10 +482,13 @@ async function deployResolvedIssue( try { response = await post(repoPath, identifier); } catch (err) { - return { transport: 'daemon', ok: false, exitCode: 1, detail: `daemon request failed: ${err instanceof Error ? err.message : String(err)}` }; + return { transport: 'daemon', ok: false, exitCode: 1, ambiguous: true, detail: `daemon request failed: ${err instanceof Error ? err.message : String(err)}` }; } if (response.status !== 202) { - return { transport: 'daemon', ok: false, exitCode: 1, detail: `daemon refused the dispatch (HTTP ${response.status}): ${describeBody(response.body)}` }; + return { + transport: 'daemon', ok: false, exitCode: 1, ambiguous: response.status >= 500, + detail: `daemon refused the dispatch (HTTP ${response.status}): ${describeBody(response.body)}`, + }; } const result = response.body as { items?: Array<{ reason?: string }>; queued?: number } | undefined; if ((result?.queued ?? 0) === 0) { @@ -580,6 +631,8 @@ export async function runExecDeployCommand( // hold that cannot be written aborts the deploy rather than shipping a run // nobody is protecting. const actor = execActor(deps); + // Read before this call writes one: a hold that already exists belongs to an earlier deploy. + const heldBefore = (deps.isHeld ?? ((issueId: string) => isExecHeld(issueId)))(issue.id); try { (deps.markHold ?? ((issueId: string, who: string) => markExecHold(issueId, who)))(issue.id, actor); } catch (err) { @@ -589,6 +642,14 @@ export async function runExecDeployCommand( const deploy = await deployResolvedIssue(deps, repoPath, issue.identifier); if (!deploy.ok) { + // The hold goes in before the run exists so that a heartbeat can never idle-fill a run nobody + // protected. A refused deploy leaves no run for an orchestrator to await, so the hold this call + // wrote would only keep idle_fill and the supervisor away from an issue nobody is watching, for + // the full TTL (each refused retry refreshing it). Kept when it already existed, when the request + // may have reached the daemon (a run might exist), and when the daemon already owns the issue + // (`exec await` can still follow that run). + const releasable = !heldBefore && !deploy.ambiguous && deploy.detail !== WORK_ALREADY_QUEUED_REASON; + if (releasable) (deps.clearHold ?? ((issueId: string) => clearExecHold(issueId)))(issue.id); log(`Deploy failed (${deploy.transport}): ${deploy.detail ?? 'unknown reason'}`); if (opts.json) out(JSON.stringify({ runId: issue.id, identifier: issue.identifier, transport: deploy.transport, deployed: false, error: deploy.detail })); return deploy.exitCode; @@ -782,6 +843,17 @@ export async function runExecAnswerCommand( return 2; } + // `--text` is the body of an answer to a question, so it needs the question's correlation. Without one + // there is nowhere to write it, and resuming anyway would report success (exit 0, run READY, redeployed) + // while the guidance was dropped and the worker parked on the same thing again. Refused before any write. + if (opts.text?.trim() && !opts.correlation) { + const message = '--text needs --correlation of a pending question: guidance is delivered as the answer to that ' + + 'question and nothing else carries it. Omit --text to resume without guidance.'; + if (opts.json) out(JSON.stringify({ runId: issueId, answerAccepted: null, resumed: null, transport: null, error: message })); + else log(message); + return 2; + } + let answerAccepted: boolean | null = null; if (opts.correlation) { // Bind the correlation to THIS run before any write. `answerHumanQuestion` @@ -851,7 +923,10 @@ export async function runExecAnswerCommand( return 1; } - const repoPath = resolve(expandPath(opts.path ?? process.cwd(), true)); + // The run knows where it was dispatched. Falling back to the cwd first sent the redeploy of an + // orchestrator that answers from anywhere else to a directory the daemon refuses (403) or the + // detached child silently exits on, after the answer and the resume were already committed. + const repoPath = resolve(expandPath(opts.path ?? row.projectPath ?? process.cwd(), true)); const identifier = row.identifier ?? issueId; const deploy = await deployResolvedIssue(deps, repoPath, identifier); if (!deploy.ok) { diff --git a/src/cli/execGuarded.test.ts b/src/cli/execGuarded.test.ts new file mode 100644 index 00000000..18fb6511 --- /dev/null +++ b/src/cli/execGuarded.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from 'vitest'; +import { runExecGuarded } from './execCommand.js'; + +// Split out of execCommand.test.ts, which is at its line cap. These need none of its ledger or spawn fixtures. + +describe('runExecGuarded', () => { + it('passes the command\'s own exit code through', async () => { + expect(await runExecGuarded(true, async () => 0)).toBe(0); + expect(await runExecGuarded(false, async () => 1)).toBe(1); + }); + + it('turns a throw into exit 2 with the message, instead of an unhandled rejection', async () => { + const logs: string[] = []; + const code = await runExecGuarded(false, async () => { throw new Error('Refusing to send OPENSWARM_DAEMON_TOKEN over plaintext http'); }, { log: (line) => logs.push(line) }); + expect(code).toBe(2); + expect(logs).toEqual(['Refusing to send OPENSWARM_DAEMON_TOKEN over plaintext http']); + }); + + it('keeps stdout a JSON document under --json, so the orchestrator can parse the failure', async () => { + const outs: string[] = []; + const code = await runExecGuarded(true, async () => { throw new Error('ledger busy'); }, { out: (line) => outs.push(line) }); + expect(code).toBe(2); + expect(JSON.parse(outs.join('\n'))).toEqual({ error: 'ledger busy' }); + }); + + it('copes with a non-Error throw', async () => { + const logs: string[] = []; + expect(await runExecGuarded(false, async () => { throw 'plain string'; }, { log: (line) => logs.push(line) })).toBe(2); + expect(logs).toEqual(['plain string']); + }); +}); diff --git a/src/cli/workCommand.test.ts b/src/cli/workCommand.test.ts index c8473580..7107729d 100644 --- a/src/cli/workCommand.test.ts +++ b/src/cli/workCommand.test.ts @@ -240,6 +240,23 @@ describe('runWorkCommand — review-finding gates (INT-3387)', () => { }); describe('runWorkCommand — direct issue ids (INT-3387)', () => { + it('applies autonomous.workerNetwork to the worker sandbox, which only core/service.ts did before (AGT-4798)', async () => { + const { getWorkerNetworkPolicy, setWorkerNetworkPolicy } = await import('../support/osSandbox.js'); + try { + const open = baseDeps({ + loadConfig: () => ({ autonomous: { maxConcurrentTasks: 1, workerNetwork: 'open' } }) as unknown as SwarmConfig, + }); + expect(await runWorkCommand({ issueIds: ['INT-1'], path: '/repo', yes: true }, open)).toBe(WORK_EXIT_OK); + expect(getWorkerNetworkPolicy()).toBe('open'); + + const dflt = baseDeps(); + expect(await runWorkCommand({ issueIds: ['INT-1'], path: '/repo', yes: true }, dflt)).toBe(WORK_EXIT_OK); + expect(getWorkerNetworkPolicy()).toBe('loopback'); + } finally { + setWorkerNetworkPolicy('loopback'); + } + }); + it('deploys one pipeline per issue with the work command contract in the context', async () => { const deps = baseDeps(); const code = await runWorkCommand( diff --git a/src/cli/workCommand.ts b/src/cli/workCommand.ts index 0874414a..64688283 100644 --- a/src/cli/workCommand.ts +++ b/src/cli/workCommand.ts @@ -19,6 +19,7 @@ import { join, resolve } from 'node:path'; import { existsSync } from 'node:fs'; import { loadConfig } from '../core/config.js'; import { enableHumanSurfaceReadOnly } from '../mcp/humanSurfacePolicy.js'; +import { setWorkerNetworkPolicy } from '../support/osSandbox.js'; import type { LinearIssueInfo, SwarmConfig } from '../core/types.js'; import type { TaskItem } from '../orchestration/decisionEngine.js'; import { linearIssueToTask } from '../orchestration/decisionEngine.js'; @@ -546,6 +547,9 @@ async function runWorkCommandInner( // Same file for the ledger and the coordination trace — see setAutomationDbPath. setAutomationDbPath(config.autonomous?.automationDbPath); + // The daemon sets the worker's network policy once at boot (core/service.ts); a standalone run has + // no service.ts, so `autonomous.workerNetwork: open` would otherwise be ignored here. (AGT-4798) + setWorkerNetworkPolicy(config.autonomous?.workerNetwork ?? 'loopback'); const coordinator = (deps.createCoordinator ?? ((o: { dbPath?: string; maxActive: number }) => new DurableRunCoordinator({ mode: 'primary', diff --git a/src/cli/workExecution.test.ts b/src/cli/workExecution.test.ts index 5fafa9b2..ee86c1e6 100644 --- a/src/cli/workExecution.test.ts +++ b/src/cli/workExecution.test.ts @@ -268,6 +268,34 @@ describe('buildWorkExecutionContext (INT-3387)', () => { expect(ctx.allowedProjects).toEqual(['/repo']); }); + it('carries the worker sandbox and the project setup the daemon carries (AGT-4783, AGT-4798)', () => { + // `openswarm work` and the detached child of `exec --issue` run the same worker as the daemon. A field + // not carried here is undefined there: the bash tool fences only on an explicit 'on', and a project's + // `setup` never runs. service.test.ts pins the daemon's mapping; this pins the CLI's. + const ctx = buildWorkExecutionContext({ + repoPath: '/repo', + autonomous: { + enabled: true, pairMode: true, schedule: '* * * * *', allowedProjects: ['/repo'], + workerSandbox: 'on', + projectAgents: [{ projectPath: '/repo', setup: ['uv sync --frozen'] }], + } as never, + }); + expect(ctx.workerSandbox).toBe('on'); + expect(ctx.getProjectSetup?.('/repo')).toEqual(['uv sync --frozen']); + // A task worktree sits under its project, and a project owns every path beneath it. + expect(ctx.getProjectSetup?.('/repo/worktree/AGT-1')).toEqual(['uv sync --frozen']); + expect(ctx.getProjectSetup?.('/elsewhere')).toBeUndefined(); + }); + + it('carries an explicit workerSandbox: off, and leaves it unset when there is no autonomous section', () => { + const off = buildWorkExecutionContext({ + repoPath: '/repo', + autonomous: { enabled: true, pairMode: true, schedule: '* * * * *', allowedProjects: [], workerSandbox: 'off' } as never, + }); + expect(off.workerSandbox).toBe('off'); + expect(buildWorkExecutionContext({ repoPath: '/repo' }).workerSandbox).toBeUndefined(); + }); + it('routes notifications to the injected log and applies the adapter override to roles', async () => { const lines: string[] = []; const ctx = buildWorkExecutionContext({ diff --git a/src/cli/workExecution.ts b/src/cli/workExecution.ts index 3be56636..3bb97d51 100644 --- a/src/cli/workExecution.ts +++ b/src/cli/workExecution.ts @@ -20,6 +20,7 @@ import type { ExecutionContext } from '../automation/runnerExecution.js'; import type { ExecutionDurabilityHooks } from '../automation/durableRunCoordinator.js'; import type { EffectClaim } from '../automation/runLedger.js'; import type { ITaskSource } from '../automation/taskSource.js'; +import { resolveOwningProject } from '../support/projectGoal.js'; import { buildCancellationEffect, buildCompletionEffect, @@ -191,6 +192,13 @@ export function buildWorkExecutionContext(opts: WorkExecutionContextOptions): Ex guards: autonomous?.guards, verify: autonomous?.verify, maxReflections: autonomous?.maxReflections, + // What AutonomousRunner.getExecCtx forwards to the same pipeline. `work` and the detached child + // that `exec --issue` falls back to run the same worker, so a field not carried here is + // undefined in them: the bash tool fences only on an explicit 'on' (the worker ran unfenced + // with open network while the config said on/loopback), and a project's `setup` never ran. + // The same unmapped-field trap as AGT-4122 / AGT-4404 / AGT-4785. (AGT-4783, AGT-4798) + workerSandbox: autonomous?.workerSandbox, + getProjectSetup: (projectPath) => resolveOwningProject(autonomous?.projectAgents, projectPath)?.setup, durability: opts.durability, peerIssues: opts.peerIssues, }; diff --git a/src/core/config.ts b/src/core/config.ts index 77bce248..a9e02c2a 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -479,8 +479,9 @@ const AutonomousConfigSchema = z.object({ /** * OS fence for the worker's bash tool (AGT-4387). 'on' (default) wraps every * bash call in sandbox-exec / bwrap: writes limited to the worktree, temp - * and build caches, network open. 'off' restores the pre-AGT-4387 behaviour - * (bash as the daemon user, regex denylist only). + * and build caches, network loopback-only unless `workerNetwork: open` + * (AGT-4798). 'off' restores the pre-AGT-4387 behaviour (bash as the daemon + * user, regex denylist only). */ workerSandbox: z.enum(['on', 'off']).default('on'), /** Accept delegated CLI adapters (claude/codex/cursor) for unattended work despite the missing fences (AGT-4444) */ diff --git a/src/core/service.test.ts b/src/core/service.test.ts index 232b02b3..98077454 100644 --- a/src/core/service.test.ts +++ b/src/core/service.test.ts @@ -375,6 +375,53 @@ describe('service', () => { }); }); + it('ignores a persisted provider override naming a delegated CLI unless the operator opted in (AGT-4444)', async () => { + // assertUnattendedAdapters reads config.yaml only; the override is applied after it, so without this + // check `openswarm provider claude` (or one dashboard click) plus a restart put the unattended + // runner on a CLI with none of the fences the daemon refuses to start without. + const { readProviderOverride } = await import('./providerOverride.js'); + const { setDefaultAdapter } = await import('../adapters/index.js'); + const { startAutonomous } = await import('../automation/autonomousRunner.js'); + vi.mocked(readProviderOverride).mockReturnValue('claude'); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + try { + const strict = { + ...mockAutonomousConfig, + adapter: 'codex-responses', + autonomous: { ...mockAutonomousConfig.autonomous!, allowDelegatedAdapters: false }, + } as SwarmConfig; + + await startService(strict); + + const runner = await vi.mocked(startAutonomous).mock.results[0].value; + expect(runner.switchProvider).not.toHaveBeenCalled(); + expect(setDefaultAdapter).not.toHaveBeenCalledWith('claude'); + expect(warn.mock.calls.some((c) => String(c[0]).includes("Ignoring provider override 'claude'"))).toBe(true); + } finally { + warn.mockRestore(); + } + }); + + it('still reapplies a delegated provider override when the operator opted in', async () => { + const { readProviderOverride } = await import('./providerOverride.js'); + const { startAutonomous } = await import('../automation/autonomousRunner.js'); + vi.mocked(readProviderOverride).mockReturnValue('claude'); + + await startService({ ...mockAutonomousConfig, adapter: 'codex-responses' } as SwarmConfig); // fixture opts in + + const runner = await vi.mocked(startAutonomous).mock.results[0].value; + expect(runner.switchProvider).toHaveBeenCalledWith('claude'); + }); + + it('forwards allowDelegatedAdapters to the runner so a later provider switch meets the same policy (AGT-4444)', async () => { + const { startAutonomous } = await import('../automation/autonomousRunner.js'); + await startService({ + ...mockAutonomousConfig, + autonomous: { ...mockAutonomousConfig.autonomous!, allowDelegatedAdapters: true }, + } as SwarmConfig); + expect(vi.mocked(startAutonomous).mock.calls[0][0]).toMatchObject({ allowDelegatedAdapters: true }); + }); + it('does not reapply provider override when it matches the configured adapter', async () => { const { readProviderOverride } = await import('./providerOverride.js'); const { setDefaultAdapter } = await import('../adapters/index.js'); diff --git a/src/core/service.ts b/src/core/service.ts index e604cabd..53ffe6ca 100644 --- a/src/core/service.ts +++ b/src/core/service.ts @@ -280,7 +280,7 @@ async function startServiceLocked(config: SwarmConfig): Promise { if (config.autonomous) { // Refuse before anything starts: an unattended worker on a CLI-owned tool // loop has none of OpenSwarm's fences. (AGT-4444) - const { assertUnattendedAdapters } = await import('../adapters/unattendedPolicy.js'); + const { assertUnattendedAdapters, providerSwitchViolation } = await import('../adapters/unattendedPolicy.js'); assertUnattendedAdapters(config); const heartbeatEnabled = config.autonomous.enabled === true; console.log(heartbeatEnabled @@ -400,6 +400,7 @@ async function startServiceLocked(config: SwarmConfig): Promise { // on an explicit 'on') ran unfenced while the log said "sandbox: on". workerSandbox: config.autonomous.workerSandbox, minFreeDiskGiB: config.autonomous.minFreeDiskGiB, // AGT-4785 + allowDelegatedAdapters: config.autonomous.allowDelegatedAdapters, // AGT-4444: a provider switch meets the same policy periodicReviews: config.autonomous.periodicReviews, orchestrator: config.autonomous.orchestrator, orchestratorSchedule: config.autonomous.orchestratorSchedule, @@ -427,7 +428,16 @@ async function startServiceLocked(config: SwarmConfig): Promise { // restart silently reverts to config.yaml's adapter. Reusing switchProvider keeps the role + // jobProfile remapping identical to a live dashboard toggle. const providerOverride = readProviderOverride(); - if (providerOverride && providerOverride !== (config.adapter ?? 'codex')) { + // The assertion above read config.yaml only. An override naming a delegated CLI would route + // unattended work around it, so it meets the same policy and the same opt-in. A stale override + // file must not keep the daemon from starting, so it is ignored loudly instead. (AGT-4444) + const refusedOverride = providerOverride + ? providerSwitchViolation(providerOverride, config.autonomous.allowDelegatedAdapters) + : null; + if (providerOverride && refusedOverride) { + console.warn(`[Service] Ignoring provider override '${providerOverride.replace(/[\r\n]/g, '')}': ${refusedOverride}. ` + + 'Set autonomous.allowDelegatedAdapters: true to accept that on purpose, or run `openswarm provider ` to replace it.'); + } else if (providerOverride && providerOverride !== (config.adapter ?? 'codex')) { setDefaultAdapter(providerOverride); runnerInstance.switchProvider(providerOverride); // The override silently wins over config.yaml — make that divergence loud so the diff --git a/src/orchestration/conflictDetector.coverage.test.ts b/src/orchestration/conflictDetector.coverage.test.ts index 285c9d8f..c904bf08 100644 --- a/src/orchestration/conflictDetector.coverage.test.ts +++ b/src/orchestration/conflictDetector.coverage.test.ts @@ -157,6 +157,46 @@ describe('detectFileConflicts Knowledge Graph fallback (no declared fileScope)', expect(draftTask).toHaveBeenCalledTimes(1); }); + it('keeps a confident decline that comes without a usable brief, leaving the scope unknown (AGT-4784)', async () => { + // `decision_pending` / `human_action_only` drafts name no files, so they are never `sufficient`. + // The pre-claim hold reads the verdict off task.preAdmissionDraft, and this function used to drop + // any draft that was not sufficient, so exactly those issues were claimed. + mockAnalyzeIssue.mockResolvedValue(null); + const decline = { + taskType: 'other', intentSummary: 'a person has to pick', relevantFiles: [] as string[], + suggestedApproach: '', completionCriteria: [] as string[], sufficient: false, + registrySnapshot: [], durationMs: 1, + scope: { + applicable: false as const, kind: 'decision_pending' as const, confidence: 0.95, + reason: 'the title waits on a person to choose', evidence: ['title says decide', 'no code deliverable'], + }, + }; + const candidate = task('declined', 2); + + const scope = await resolveTaskFileScope(candidate, project, { draftTask: async () => decline }); + + expect(scope).toEqual([]); + expect(candidate.fileScope).toBeUndefined(); + expect(candidate.fileScopeSource).toBeUndefined(); + expect(candidate.preAdmissionDraft?.scope?.kind).toBe('decision_pending'); + }); + + it('does not keep a weak verdict: under the confidence bar it is a hunch, not a decline', async () => { + mockAnalyzeIssue.mockResolvedValue(null); + const candidate = task('weak', 2); + await resolveTaskFileScope(candidate, project, { + draftTask: async () => ({ + taskType: 'other', intentSummary: '', relevantFiles: [] as string[], suggestedApproach: '', + completionCriteria: [] as string[], sufficient: false, registrySnapshot: [], durationMs: 1, + scope: { + applicable: false as const, kind: 'decision_pending' as const, confidence: 0.6, + reason: 'maybe', evidence: ['one thing'], + }, + }), + }); + expect(candidate.preAdmissionDraft).toBeUndefined(); + }); + it('admits KG scopes that reduce to unknown after volatile paths are removed', async () => { // Both tasks resolve to a non-null impact, but every module normalizes // away (node_modules / dist are filtered as volatile), so `modules.size` diff --git a/src/orchestration/conflictDetector.ts b/src/orchestration/conflictDetector.ts index 240d2336..ccbbed4d 100644 --- a/src/orchestration/conflictDetector.ts +++ b/src/orchestration/conflictDetector.ts @@ -12,6 +12,7 @@ import { normalizeConflictScope, } from './conflictScope.js'; import { selectGreedyMaximalIndependentSet } from './conflictAdmission.js'; +import { isScopeDeclined } from '../agents/draftScope.js'; import { canonicalExistingRepoFiles, MAX_VALIDATED_DIRECT_FILES, @@ -170,6 +171,11 @@ export async function resolveTaskFileScope( return relevantFiles; } } + // A confident "a person has to do or decide this" verdict is never a sufficient brief — it + // names no files — yet the pre-claim hold reads it off the task (AGT-4784). Dropping it here is + // what let `decision_pending` and `human_action_only` issues through to a claim and a worktree. + // The scope stays unknown: only the verdict is kept. + if (draft && isScopeDeclined(draft.scope)) task.preAdmissionDraft = draft; } catch (err) { console.warn(`[ConflictDetector] Pre-admission draft failed for ${task.id}:`, err); } diff --git a/src/orchestration/decisionEngine.ts b/src/orchestration/decisionEngine.ts index 4aa0551b..1f7cff2c 100644 --- a/src/orchestration/decisionEngine.ts +++ b/src/orchestration/decisionEngine.ts @@ -261,8 +261,9 @@ export interface DecisionEngineConfig { dryRun: boolean; /** - * Treat Linear "Backlog" as an actionable work queue. Default true (AGT-4257) - * so free slots chew parked work. Set false to park Backlog again. + * Treat Linear "Backlog" as an actionable work queue. Default false (AGT-4784): + * moving an issue to Todo is the ready signal. Set true (AGT-4257's behaviour) + * on a deployment that wants free slots to chew parked work. */ includeBacklog?: boolean; diff --git a/src/support/osSandbox.ts b/src/support/osSandbox.ts index d7985878..e2510e9e 100644 --- a/src/support/osSandbox.ts +++ b/src/support/osSandbox.ts @@ -12,8 +12,10 @@ // Policy, measured on cgf-portal (2026-09-17, AGT-4387): a worker needs to // write its worktree, the temp dir, and the package-manager caches (uv, npm, // cargo, wrangler's log dir). Everything else — other repos, ~/.ssh, the -// daemon's own state and credentials — stays read-only. Network stays open: -// workers install dependencies and call the tracker. +// daemon's own state and credentials — stays read-only. The worker's network +// is loopback-only by default (AGT-4798): dependency installs belong to the +// project's `setup` step, which runs with the network open, and a test can +// still start a local server. `workerNetwork: open` restores the old behaviour. import { existsSync, realpathSync } from 'node:fs'; import { homedir, tmpdir } from 'node:os'; diff --git a/src/support/web.provider.test.ts b/src/support/web.provider.test.ts index 59c1dd4b..039f4723 100644 --- a/src/support/web.provider.test.ts +++ b/src/support/web.provider.test.ts @@ -52,6 +52,7 @@ describe('POST /api/provider persistence (INT-3284)', () => { it('still writes override when a runner is attached (idempotent with switchProvider)', async () => { const runner = { switchProvider: vi.fn(), + providerSwitchViolation: vi.fn(() => null), getEnabledProjects: vi.fn(() => []), getAllowedProjects: vi.fn(() => []), enableProject: vi.fn(), @@ -72,4 +73,32 @@ describe('POST /api/provider persistence (INT-3284)', () => { expect(runner.switchProvider).toHaveBeenCalledWith('gpt'); expect(writeProviderOverrideMock).toHaveBeenCalledWith('gpt'); }); + + it('refuses a switch the runner refuses (a delegated CLI the operator did not opt in to), and persists nothing (AGT-4444)', async () => { + // One click on the dashboard used to undo the startup refusal: the toggle only checked isKnownAdapter. + const runner = { + switchProvider: vi.fn(), + providerSwitchViolation: vi.fn(() => "'claude' delegates the tool loop to its own CLI"), + getEnabledProjects: vi.fn(() => []), + getAllowedProjects: vi.fn(() => []), + enableProject: vi.fn(), + disableProject: vi.fn(), + updateAllowedProjects: vi.fn(), + registerProjectPath: vi.fn(), + } as unknown as AutonomousRunner; + setWebRunner(runner); + + const port = await freePort(); + await startWebServer(port); + const res = await fetch(`http://127.0.0.1:${port}/api/provider`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ provider: 'claude' }), + }); + expect(res.status).toBe(403); + const body = await res.json() as { error: string }; + expect(body.error).toContain('allowDelegatedAdapters: true'); + expect(runner.switchProvider).not.toHaveBeenCalled(); + expect(writeProviderOverrideMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/support/web.ts b/src/support/web.ts index d8da3d90..2030b326 100644 --- a/src/support/web.ts +++ b/src/support/web.ts @@ -724,6 +724,18 @@ export async function startWebServer(port: number = 3847): Promise { return; } + // The toggle re-points every unattended role, so it meets the same refusal the daemon + // applies at startup to config.yaml (AGT-4444). `openswarm provider` surfaces the 403. + const refused = runnerRef?.providerSwitchViolation(provider); + if (refused) { + res.writeHead(403, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ + error: `Refusing to switch the autonomous runner to "${provider}": ${refused}. ` + + 'Set autonomous.allowDelegatedAdapters: true to accept that on purpose.', + })); + return; + } + setDefaultAdapter(provider); runnerRef?.switchProvider(provider); // switchProvider persists only when a runner is attached. Always write