diff --git a/.env.example b/.env.example index 1d30853..ff09af1 100644 --- a/.env.example +++ b/.env.example @@ -58,8 +58,8 @@ FORGE_WORKER_CLAIM_TIMEOUT_SECONDS=5 # generated files. # FORGE_WORK_PACKAGE_EXECUTION=1 # FORGE_HOST_REPOSITORY_WRITES=1 -# ACP package execution remains separately disabled by default because ACP -# adapters are local processes and are not OS-confined by Forge. +# ACP package execution is enabled after task approval. Set this to 0 when local +# adapter process access is not acceptable; ACP is not OS-confined by Forge. # FORGE_ACP_WORK_PACKAGE_EXECUTION=0 # Workspace root — where Forge stores user-owned operational files: projects, diff --git a/docs/acp-zed-connector.md b/docs/acp-zed-connector.md index 5aae6d2..5985238 100644 --- a/docs/acp-zed-connector.md +++ b/docs/acp-zed-connector.md @@ -71,10 +71,11 @@ When a task uses an ACP provider, Forge: Forge uses one adapter process per call. It does not keep a long-lived pool of ACP agents. -Executable Workforce packages do not use ACP providers by default. ACP adapters -are local processes and Forge does not OS-confine them to the package attempt -sandbox. Set `FORGE_ACP_WORK_PACKAGE_EXECUTION=1` only after accepting that -local process risk for the repositories where Forge runs. +Executable Workforce packages may use a configured ACP provider after task +approval. ACP adapters are local processes and Forge does not OS-confine them +to the package attempt sandbox. Set `FORGE_ACP_WORK_PACKAGE_EXECUTION=0` to +disable ACP package execution on hosts where that local process risk is not +acceptable. ## Current Limits @@ -82,9 +83,10 @@ local process risk for the repositories where Forge runs. - Forge does not receive detailed token usage from ACP. - Tool calls from the underlying coding agent are not exposed as Forge runtime MCP grants. -- ACP package execution is separately opt-in. When enabled, it relies on the - package attempt working directory and Forge's execution JSON path guards; it is - not an OS sandbox or a general live tool grant. +- ACP package execution is enabled after task approval unless the operator sets + `FORGE_ACP_WORK_PACKAGE_EXECUTION=0`. When active, it relies on the package + attempt working directory and Forge's execution JSON path guards; it is not an + OS sandbox or a general live tool grant. - If a runtime does not expose model selection through ACP, Forge stores the selected model on the provider record but cannot force the local runtime to use it. diff --git a/docs/adr/0006-executable-workforce-beta-boundary.md b/docs/adr/0006-executable-workforce-beta-boundary.md index 8aa0f46..1f29f26 100644 --- a/docs/adr/0006-executable-workforce-beta-boundary.md +++ b/docs/adr/0006-executable-workforce-beta-boundary.md @@ -28,9 +28,10 @@ edits and a reviewable sandbox copy of every generated file. disables model execution and keeps the handoff-artifact-only path. - `FORGE_HOST_REPOSITORY_WRITES=0`, `false`, `off`, `no`, or `disabled` lets package models run but keeps generated files sandbox-only. -- ACP-backed work-package execution is separately disabled by default. Set - `FORGE_ACP_WORK_PACKAGE_EXECUTION=1` only when the operator accepts that ACP - adapters are local processes and are not OS-confined by Forge. +- ACP-backed work-package execution is enabled after an operator configures an + ACP provider and approves the task. ACP adapters are local processes and are + not OS-confined by Forge. Set `FORGE_ACP_WORK_PACKAGE_EXECUTION=0`, `false`, + `off`, `no`, or `disabled` to prevent ACP package execution. - After Architect plan approval, Forge may execute one eligible specialist work package at a time. Parallel specialist execution remains out of scope. - Forge may collect bounded read-only host-repository context before a package diff --git a/docs/developer-guide.md b/docs/developer-guide.md index f997c8d..8e85f86 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -35,10 +35,10 @@ back through the same provider interface used by the worker. The currently wired Agent Client Protocol adapters wrap local tools such as Codex CLI and Claude Code; the underlying CLI must already be installed, authenticated, and runnable on the worker host. Architect ACP calls run in an isolated runtime directory. -Executable work-package ACP calls are disabled by default because ACP adapters -are local processes, not OS-confined sandboxes. Operators can opt in with -`FORGE_ACP_WORK_PACKAGE_EXECUTION=1` after accepting that risk. See [ACP and the -Zed connector](acp-zed-connector.md). +Executable work-package ACP calls are enabled after task approval. ACP adapters +are local processes, not OS-confined sandboxes. Operators can opt out with +`FORGE_ACP_WORK_PACKAGE_EXECUTION=0` when that risk is not acceptable. See [ACP +and the Zed connector](acp-zed-connector.md). ## Local Development @@ -155,7 +155,7 @@ Feature flag defaults: | `FORGE_WORK_PACKAGE_HANDOFF` | enabled | Set `0` or `false` to stop package handoff claims. | | `FORGE_WORK_PACKAGE_EXECUTION` | enabled | Set `0`, `false`, `off`, `no`, or `disabled` to stop specialist package execution and create handoff artifacts only. | | `FORGE_HOST_REPOSITORY_WRITES` | enabled | Set `0`, `false`, `off`, `no`, or `disabled` to keep generated files sandbox-only and skip local project edits. | -| `FORGE_ACP_WORK_PACKAGE_EXECUTION` | disabled | Set `1`, `true`, `on`, `yes`, or `enabled` only when local ACP package execution is an accepted operator risk. | +| `FORGE_ACP_WORK_PACKAGE_EXECUTION` | enabled | Set `0`, `false`, `off`, `no`, or `disabled` to block ACP package execution when local adapter process access is not acceptable. | | `FORGE_RUNNING_WORK_PACKAGE_STALE_SECONDS` | `900` | Recovery window before a retry marks an interrupted running work package blocked and starts the next eligible attempt. | ### Executable Workforce Beta diff --git a/docs/operator-guide.md b/docs/operator-guide.md index 012e5a5..4f2426b 100644 --- a/docs/operator-guide.md +++ b/docs/operator-guide.md @@ -153,11 +153,11 @@ For the currently wired ACP adapters: - The adapter wraps the local `codex` or `claude` CLI. - The local CLI must already be installed and logged in. - The Forge project must have a local folder so Forge can validate and bound - repository context. Architect planning uses an isolated runtime directory; - executable work-package ACP sessions are disabled by default because local ACP - adapters are not OS-confined by Forge. Set - `FORGE_ACP_WORK_PACKAGE_EXECUTION=1` only for repositories where that local - process access is acceptable. + repository context. Architect planning uses an isolated runtime directory. + Executable work-package ACP sessions are enabled after task approval, but the + local adapter is not OS-confined by Forge. Set + `FORGE_ACP_WORK_PACKAGE_EXECUTION=0` where that local process access is not + acceptable. - Installing the Zed editor is not required; Forge uses Agent Client Protocol adapter packages, not the editor itself. @@ -317,7 +317,7 @@ Worker and workspace options: | `FORGE_WORK_PACKAGE_HANDOFF` | Set `0` or `false` to disable default work-package handoff claims | | `FORGE_WORK_PACKAGE_EXECUTION` | Set `0`, `false`, `off`, `no`, or `disabled` to disable default package execution and create handoff artifacts only | | `FORGE_HOST_REPOSITORY_WRITES` | Set `0`, `false`, `off`, `no`, or `disabled` to keep generated files sandbox-only and skip local project edits | -| `FORGE_ACP_WORK_PACKAGE_EXECUTION` | Set `1`, `true`, `on`, `yes`, or `enabled` only when local ACP package execution is an accepted operator risk | +| `FORGE_ACP_WORK_PACKAGE_EXECUTION` | Set `0`, `false`, `off`, `no`, or `disabled` to block ACP package execution when local adapter process access is not acceptable | | `FORGE_RUNNING_WORK_PACKAGE_STALE_SECONDS` | Defaults to `900`; retry handoff treats older running package rows as interrupted and recovers them before continuing | | `FORGE_WORKSPACE_ROOT` | Fixed workspace root override | | `FORGE_MCPS_ROOT` | Fixed shared MCP root override | diff --git a/web/__tests__/work-package-execution-context.test.ts b/web/__tests__/work-package-execution-context.test.ts index a50fcfe..19fae5e 100644 --- a/web/__tests__/work-package-execution-context.test.ts +++ b/web/__tests__/work-package-execution-context.test.ts @@ -104,29 +104,39 @@ describe('loadWorkPackageExecutionContext', () => { expect(context.validatedProjectRoot).toBe('/workspace/real-project') }) - it('blocks ACP-backed executable work packages unless explicitly enabled', async () => { - vi.clearAllMocks() - const project = { id: 'project-1', localPath: '/workspace/project' } - const task = { id: 'task-1', projectId: 'project-1', pmProviderConfigId: 'provider-task' } - const workPackage = { id: 'pkg-1', assignedRole: 'backend' } - mocks.dbSelect - .mockReturnValueOnce(chain([{ task, project, workPackage }])) - .mockReturnValueOnce(chain([{ id: 'agent-backend', providerConfigId: null }])) - mocks.getProvider.mockResolvedValue({ - config: { providerType: 'acp', modelId: 'codex-cli::gpt-5.3-codex-spark' }, - }) - - await expect(loadWorkPackageExecutionContext('task-1', 'pkg-1')) - .rejects.toThrow(/ACP work-package execution is disabled/i) + it.each(['0', 'flase'])( + 'blocks ACP-backed executable work packages when the setting is %s', + async (setting) => { + vi.clearAllMocks() + const previous = process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION + process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION = setting + const project = { id: 'project-1', localPath: '/workspace/project' } + const task = { id: 'task-1', projectId: 'project-1', pmProviderConfigId: 'provider-task' } + const workPackage = { id: 'pkg-1', assignedRole: 'backend' } + mocks.dbSelect + .mockReturnValueOnce(chain([{ task, project, workPackage }])) + .mockReturnValueOnce(chain([{ id: 'agent-backend', providerConfigId: null }])) + mocks.getProvider.mockResolvedValue({ + config: { providerType: 'acp', modelId: 'codex-cli::gpt-5.3-codex-spark' }, + }) + + try { + await expect(loadWorkPackageExecutionContext('task-1', 'pkg-1')) + .rejects.toThrow(/ACP work-package execution is disabled/i) + } finally { + if (previous === undefined) delete process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION + else process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION = previous + } - expect(mocks.assertProjectLocalPathForExecution).not.toHaveBeenCalled() - expect(mocks.getModel).not.toHaveBeenCalled() - }) + expect(mocks.assertProjectLocalPathForExecution).not.toHaveBeenCalled() + expect(mocks.getModel).not.toHaveBeenCalled() + }, + ) - it('allows ACP-backed executable work packages when ACP execution is explicitly enabled', async () => { + it('allows ACP-backed executable work packages by default', async () => { vi.clearAllMocks() const previous = process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION - process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION = '1' + delete process.env.FORGE_ACP_WORK_PACKAGE_EXECUTION const project = { id: 'project-1', localPath: '/workspace/project' } const task = { id: 'task-1', projectId: 'project-1', pmProviderConfigId: 'provider-task' } const workPackage = { id: 'pkg-1', assignedRole: 'backend' } diff --git a/web/__tests__/work-package-executor.test.ts b/web/__tests__/work-package-executor.test.ts index e5ab649..71940b5 100644 --- a/web/__tests__/work-package-executor.test.ts +++ b/web/__tests__/work-package-executor.test.ts @@ -155,6 +155,26 @@ describe('parseWorkPackageExecutionPlan', () => { expect(parsed.commands).toEqual([['npm', 'test']]) }) + it('normalizes package-script validation command aliases', () => { + const parsed = parseWorkPackageExecutionPlan(JSON.stringify({ + schemaVersion: 1, + summary: 'Built tracker', + files: [{ + path: 'package.json', + content: JSON.stringify({ + scripts: { + build: 'node build-check.js', + lint: 'node lint-check.js', + test: 'node tracker.test.js', + }, + }), + }], + commands: [['node', 'tracker.test.js'], ['node', 'build-check.js'], ['node', 'lint-check.js']], + })) + + expect(parsed.commands).toEqual([['npm', 'test'], ['npm', 'run', 'build'], ['npm', 'run', 'lint']]) + }) + it('rejects unsupported commands', () => { expect(() => parseWorkPackageExecutionPlan(JSON.stringify({ schemaVersion: 1, @@ -188,6 +208,62 @@ describe('parseWorkPackageExecutionPlan', () => { expect(parsed.summary).toBe('Built tracker') expect(parsed.files).toHaveLength(1) }) + + it('reports malformed execution JSON instead of an incidental object shape error', () => { + const raw = [ + 'Diagnostic metadata: {"provider":"local"}', + '```work_package_execution_json', + '{"schemaVersion":1,"summary":"Cut off"', + ].join('\n') + + expect(() => parseWorkPackageExecutionPlan(raw)).toThrow(/not valid JSON/i) + }) + + it('parses a one-line fenced execution JSON block', () => { + const payload = JSON.stringify({ + schemaVersion: 1, + summary: 'Built tracker', + files: [{ path: 'package.json', content: '{"scripts":{}}' }], + commands: [], + }) + + const parsed = parseWorkPackageExecutionPlan(`\`\`\`work_package_execution_json ${payload}\`\`\``) + + expect(parsed.summary).toBe('Built tracker') + expect(parsed.files).toHaveLength(1) + }) + + it('parses a JSON-encoded execution response string', () => { + const payload = JSON.stringify({ + schemaVersion: 1, + summary: 'Built tracker', + files: [{ path: 'package.json', content: '{"scripts":{}}' }], + commands: [], + }) + + const parsed = parseWorkPackageExecutionPlan(JSON.stringify(payload)) + + expect(parsed.summary).toBe('Built tracker') + expect(parsed.files).toHaveLength(1) + }) + + it('parses a JSON-encoded fenced execution response string', () => { + const payload = [ + '```work_package_execution_json', + JSON.stringify({ + schemaVersion: 1, + summary: 'Built tracker', + files: [{ path: 'package.json', content: '{"scripts":{}}' }], + commands: [], + }), + '```', + ].join('\n') + + const parsed = parseWorkPackageExecutionPlan(JSON.stringify(payload)) + + expect(parsed.summary).toBe('Built tracker') + expect(parsed.files).toHaveLength(1) + }) }) describe('hasLocalConflictCopyPathSegment', () => { @@ -319,7 +395,7 @@ describe('executeWorkPackage', () => { }, { path: 'index.test.js', - content: 'import test from "node:test"; import assert from "node:assert/strict"; test("ok", () => assert.equal(1, 1));\n', + content: 'const test = require("node:test"); const assert = require("node:assert/strict"); test("ok", () => assert.equal(1, 1));\n', }, ], commands: [['npm', 'test'], ['npm', 'run', 'build']], @@ -1131,7 +1207,7 @@ describe('executeWorkPackage', () => { }, { path: 'index.test.js', - content: 'import test from "node:test"; import assert from "node:assert/strict"; test("ok", () => assert.equal(1, 1));\n', + content: 'const test = require("node:test"); const assert = require("node:assert/strict"); test("ok", () => assert.equal(1, 1));\n', }, ], commands: [['npm', 'test']], @@ -1147,31 +1223,117 @@ describe('executeWorkPackage', () => { await expect(fs.stat(outsideFile)).rejects.toMatchObject({ code: 'ENOENT' }) }) - it('fails build validation when no JavaScript source files can be checked', async () => { + it('repairs build validation when no JavaScript source files can be checked', async () => { + mocks.generateText + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated unchecked TypeScript.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { build: 'tsc --noEmit' } }), + }, + { + path: 'src/app.tsx', + content: 'export const App = () =>
\n', + }, + ], + commands: [['npm', 'run', 'build']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated checkable JavaScript.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { build: 'node build-check.js' } }), + }, + { path: 'app.js', content: 'module.exports = { ready: true };\n' }, + { path: 'build-check.js', content: 'console.log("build validated");\n' }, + ], + commands: [['npm', 'run', 'build']], + }), + }) + + const result = await executeWorkPackage(context({ + task: { + ...context().task, + prompt: 'Build a tiny task tracker web app. Make sure it builds.', + }, + })) + + expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('at least one checkable JavaScript source file') + expect(result.summary).toBe('Generated checkable JavaScript.') + }) + + it.each([ + [ + 'comment-only calls', + [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + '// test("fake", () => assert.equal(1, 1));', + '', + ].join('\n'), + ], + [ + 'skipped and TODO tests', + [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + 'test.skip("skipped", () => assert.equal(1, 1));', + 'test.todo("todo");', + '', + ].join('\n'), + ], + [ + 'string-only calls', + [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + 'const example = "test(\\"fake\\", () => assert.equal(1, 1))";', + 'void example;', + '', + ].join('\n'), + ], + [ + 'string-only module references', + [ + 'const modules = `require("node:test") require("node:assert/strict")`;', + 'const test = (_name, callback) => callback();', + 'const assert = { equal: () => undefined };', + 'test("fake", () => assert.equal(1, 1));', + 'void modules;', + '', + ].join('\n'), + ], + ])('rejects %s as focused test coverage', async (_label, content) => { mocks.generateText.mockResolvedValue({ text: JSON.stringify({ schemaVersion: 1, - summary: 'Generated unchecked TypeScript.', + summary: 'Generated non-running tests.', files: [ { path: 'package.json', - content: JSON.stringify({ scripts: { build: 'tsc --noEmit' } }), - }, - { - path: 'src/app.tsx', - content: 'export const App = () => \n', + content: JSON.stringify({ scripts: { test: 'node --test' } }), }, + { path: 'tracker.test.js', content }, ], - commands: [['npm', 'run', 'build']], + commands: [['npm', 'test']], }), }) await expect(executeWorkPackage(context({ task: { ...context().task, - prompt: 'Build a tiny task tracker web app. Make sure it builds.', + prompt: 'Build a tiny task tracker web app with focused tests.', }, - }))).rejects.toThrow(/at least one checkable JavaScript source file/i) + }))).rejects.toThrow(/not a focused node:test assertion/i) + expect(mocks.generateText).toHaveBeenCalledTimes(3) }) it('fails lint validation when no JavaScript source files can be checked', async () => { @@ -1196,7 +1358,7 @@ describe('executeWorkPackage', () => { await expect(executeWorkPackage(context())).rejects.toThrow(/at least one checkable JavaScript source file/i) }) - it('throws validation failures with durable sandbox output metadata', async () => { + it('wraps command-selected generation validation failures with durable empty sandbox metadata', async () => { mocks.generateText.mockResolvedValue({ text: JSON.stringify({ schemaVersion: 1, @@ -1222,16 +1384,15 @@ describe('executeWorkPackage', () => { expect(err).toBeInstanceOf(WorkPackageExecutionError) const failure = (err as WorkPackageExecutionError).failureDetails expect(failure.sandboxPath).toBe(path.join(tempRoot, '.forge', 'task-runs', 'task-1', 'pkg-1', 'attempt-1')) - expect(failure.fileCount).toBe(1) + expect(failure.fileCount).toBe(0) expect(failure.artifactMetadata).toMatchObject({ - files: ['package.json'], + files: [], generatedBy: 'work-package-executor', repositoryWrites: false, - sandboxWrites: true, + sandboxWrites: false, validationStatus: 'failed', }) - expect(failure.commandResults).toHaveLength(1) - expect(failure.commandResults[0].exitCode).not.toBe(0) + expect(failure.commandResults).toHaveLength(0) } }) @@ -1261,7 +1422,7 @@ describe('executeWorkPackage', () => { validationStatus: 'failed', }) await expect(fs.stat(sandbox)).resolves.toMatchObject({}) - expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText).toHaveBeenCalledTimes(3) } }) @@ -1523,7 +1684,7 @@ describe('executeWorkPackage', () => { prompt: 'Build a tiny task tracker web app. Add focused tests and make sure the app builds.', }, }))).rejects.toThrow(/placeholder tests/i) - expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText).toHaveBeenCalledTimes(3) }) it('rejects invalid node:test shell scripts before command execution', async () => { @@ -1543,7 +1704,7 @@ describe('executeWorkPackage', () => { }, { path: 'tracker.test.js', - content: 'import test from "node:test"; import assert from "node:assert/strict"; test("adds item", () => assert.equal(1, 1));\n', + content: 'const test = require("node:test"); const assert = require("node:assert/strict"); test("adds item", () => assert.equal(1, 1));\n', }, { path: 'build-check.js', content: 'console.log("build ok");\n' }, ], @@ -1559,6 +1720,224 @@ describe('executeWorkPackage', () => { }))).rejects.toThrow(/test script is invalid/i) }) + it('accepts focused tests that use a localStorage stub and implementation placeholder text', async () => { + mocks.generateText.mockResolvedValue({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Built and tested the tracker.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ + scripts: { + build: 'node build-check.js', + test: 'node --test', + }, + }), + }, + { + path: 'tracker.js', + content: 'export function configure(input) { input.placeholder = "Add task"; }\n', + }, + { + path: 'tracker.test.js', + content: [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + "const quotePattern = /['\"]/;", + 'test("persists with a localStorage stub", () => {', + ' const localStorageStub = new Map([["tasks", "[]"]]);', + ' const input = { placeholder: "Add task" };', + ' assert.equal(quotePattern.test("\\\""), true);', + ' assert.equal(localStorageStub.get("tasks"), "[]");', + ' assert.equal(input.placeholder, "Add task");', + '});', + '', + ].join('\n'), + }, + { + path: 'build-check.js', + content: 'console.log("build validated");\n', + }, + ], + commands: [['node', '--test'], ['node', 'build-check.js']], + }), + }) + + const result = await executeWorkPackage(context({ + task: { + ...context().task, + prompt: 'Build a tiny task tracker web app. Add focused tests and make sure the app builds.', + }, + })) + + expect(result.summary).toBe('Built and tested the tracker.') + expect(result.commandResults.map((item) => item.command)).toEqual([ + ['npm', 'test'], + ['npm', 'run', 'build'], + ]) + }) + + it('repairs a response truncated at the output limit', async () => { + mocks.generateText + .mockResolvedValueOnce({ + finishReason: 'length', + text: '{"schemaVersion":1,"summary":"Cut off"', + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Returned a concise complete plan.', + files: [{ path: 'package.json', content: '{}' }], + commands: [], + }), + }) + + const result = await executeWorkPackage(context()) + + expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('configured output limit') + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('Keep the response concise') + expect(result.summary).toBe('Returned a concise complete plan.') + }) + + it('repairs selected test validation when the task prompt does not mention tests', async () => { + mocks.generateText + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated weak tests.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { test: 'node --test' } }), + }, + { + path: 'tracker.test.js', + content: 'const test = require("node:test"); test("adds", () => console.log("ok"));\n', + }, + ], + commands: [['npm', 'test']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated focused tests.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { test: 'node --test' } }), + }, + { + path: 'tracker.test.js', + content: 'const test = require("node:test"); const assert = require("node:assert/strict"); test("adds", () => assert.equal(1, 1));\n', + }, + ], + commands: [['npm', 'test']], + }), + }) + + const result = await executeWorkPackage(context()) + + expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('not a focused node:test assertion') + expect(result.summary).toBe('Generated focused tests.') + }) + + it('repairs a selected placeholder build when the task prompt does not mention builds', async () => { + mocks.generateText + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated a placeholder build.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { build: 'echo "build passed"' } }), + }, + { path: 'app.js', content: 'export const ready = true;\n' }, + ], + commands: [['npm', 'run', 'build']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated a meaningful build check.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { build: 'node build-check.js --strict true' } }), + }, + { path: 'app.js', content: 'export const ready = true;\n' }, + { path: 'build-check.js', content: 'console.log("build validated");\n' }, + ], + commands: [['npm', 'run', 'build']], + }), + }) + + const result = await executeWorkPackage(context()) + + expect(mocks.generateText).toHaveBeenCalledTimes(2) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('build script appears to be a placeholder') + expect(result.summary).toBe('Generated a meaningful build check.') + }) + + it('repairs selected lint validation before writing sandbox files', async () => { + mocks.generateText + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated a placeholder lint command.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { lint: 'echo "lint passed"' } }), + }, + { path: 'app.js', content: 'module.exports = { ready: true };\n' }, + ], + commands: [['npm', 'run', 'lint']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated unchecked TypeScript lint input.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { lint: 'node lint-check.js' } }), + }, + { path: 'app.tsx', content: 'export const App = () => ;\n' }, + ], + commands: [['npm', 'run', 'lint']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated checkable lint input.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ scripts: { lint: 'node lint-check.js' } }), + }, + { path: 'app.js', content: 'module.exports = { ready: true };\n' }, + { path: 'lint-check.js', content: 'console.log("lint validated");\n' }, + ], + commands: [['npm', 'run', 'lint']], + }), + }) + + const result = await executeWorkPackage(context()) + + expect(mocks.generateText).toHaveBeenCalledTimes(3) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('lint script appears to be a placeholder') + expect(mocks.generateText.mock.calls[2][0].prompt).toContain('at least one checkable JavaScript source file') + expect(result.summary).toBe('Generated checkable lint input.') + }) + it('reprompts once when validation rejects the first generated plan', async () => { mocks.generateText .mockResolvedValueOnce({ @@ -1596,7 +1975,7 @@ describe('executeWorkPackage', () => { }, { path: 'tracker.test.js', - content: 'import test from "node:test"; import assert from "node:assert/strict"; test("adds item", () => assert.equal(["a"].length, 1));\n', + content: 'const test = require("node:test"); const assert = require("node:assert/strict"); test("adds item", () => assert.equal(["a"].length, 1));\n', }, { path: 'build-check.js', @@ -1618,4 +1997,136 @@ describe('executeWorkPackage', () => { expect(result.summary).toBe('Built and tested the tracker.') expect(result.commandResults.map((item) => item.exitCode)).toEqual([0, 0]) }) + + it('reprompts through invalid JSON and bad generated tests for a tiny task tracker', async () => { + mocks.generateText + .mockResolvedValueOnce({ + text: '```work_package_execution_json\n{"schemaVersion":1,"summary":"Cut off"', + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Generated weak tracker tests.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ + scripts: { + build: 'node build-check.js', + test: 'node test.js', + }, + }), + }, + { + path: 'tracker.test.js', + content: [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + 'const runner = { test: (callback) => callback() };', + 'assert.equal(1, 1);', + 'runner.test(() => assert.equal(1, 1));', + 'test("adds item", () => {});', + '', + ].join('\n'), + }, + { + path: 'build-check.js', + content: 'console.log("build validated");\n', + }, + ], + commands: [['npm', 'test'], ['npm', 'run', 'build']], + }), + }) + .mockResolvedValueOnce({ + text: JSON.stringify({ + schemaVersion: 1, + summary: 'Built a dependency-free tiny task tracker.', + files: [ + { + path: 'package.json', + content: JSON.stringify({ + scripts: { + build: 'node build-check.js', + test: 'node --test', + }, + }), + }, + { + path: 'taskStore.js', + content: [ + 'function createState() { return { tasks: [] }; }', + 'function addTask(state, title) {', + ' const task = { id: String(state.tasks.length + 1), title, completed: false };', + ' return { tasks: [...state.tasks, task] };', + '}', + 'function toggleTask(state, id) {', + ' return { tasks: state.tasks.map((task) => task.id === id ? { ...task, completed: !task.completed } : task) };', + '}', + 'function deleteTask(state, id) { return { tasks: state.tasks.filter((task) => task.id !== id) }; }', + 'function filterTasks(state, filter) {', + ' if (filter === "active") return state.tasks.filter((task) => !task.completed);', + ' if (filter === "completed") return state.tasks.filter((task) => task.completed);', + ' return state.tasks;', + '}', + 'function hydrate(raw) {', + ' try {', + ' const parsed = JSON.parse(raw);', + ' return Array.isArray(parsed.tasks) ? parsed : createState();', + ' } catch {', + ' return createState();', + ' }', + '}', + 'module.exports = { createState, addTask, toggleTask, deleteTask, filterTasks, hydrate };', + '', + ].join('\n'), + }, + { + path: 'tracker.test.js', + content: [ + 'const test = require("node:test");', + 'const assert = require("node:assert/strict");', + 'const { createState, addTask, toggleTask, deleteTask, filterTasks, hydrate } = require("./taskStore.js");', + '', + 'test("add, complete, filter, delete, and malformed storage fallback", () => {', + ' let state = createState();', + ' state = addTask(state, "Ship the tracker");', + ' assert.equal(state.tasks.length, 1);', + ' state = toggleTask(state, state.tasks[0].id);', + ' assert.deepEqual(filterTasks(state, "completed").map((task) => task.title), ["Ship the tracker"]);', + ' assert.equal(filterTasks(state, "active").length, 0);', + ' state = deleteTask(state, state.tasks[0].id);', + ' assert.equal(filterTasks(state, "all").length, 0);', + ' assert.deepEqual(hydrate("{bad json"), createState());', + '});', + '', + ].join('\n'), + }, + { + path: 'build-check.js', + content: [ + 'const fs = require("node:fs");', + 'for (const file of ["taskStore.js", "tracker.test.js"]) {', + ' if (!fs.existsSync(file)) throw new Error(`${file} is missing`);', + '}', + '', + ].join('\n'), + }, + ], + commands: [['npm', 'test'], ['npm', 'run', 'build']], + }), + }) + + const result = await executeWorkPackage(context({ + task: { + ...context().task, + prompt: 'Build a tiny task tracker web app. Add focused tests and make sure the app builds.', + }, + })) + + expect(mocks.generateText).toHaveBeenCalledTimes(3) + expect(mocks.generateText.mock.calls[1][0].prompt).toContain('Execution response was not valid JSON.') + expect(mocks.generateText.mock.calls[2][0].prompt).toContain('Generated test file is not a focused node:test assertion: tracker.test.js') + expect(result.summary).toBe('Built a dependency-free tiny task tracker.') + expect(result.commandResults.map((item) => item.exitCode)).toEqual([0, 0]) + }) }) diff --git a/web/worker/work-package-executor.ts b/web/worker/work-package-executor.ts index ea5fb09..f08b3d8 100644 --- a/web/worker/work-package-executor.ts +++ b/web/worker/work-package-executor.ts @@ -27,6 +27,7 @@ import { sanitizeWorkerMessage } from './redaction' import { publishTaskEvent } from './events' import { recordTaskLogBestEffort } from './task-logs' import { shouldApplyHostRepositoryWrites } from './repository-edit-policy' +import { defaultOnFeatureFlagState } from './feature-flags' const execFile = promisify(execFileCallback) @@ -36,7 +37,7 @@ const MAX_FILE_BYTES = 512 * 1024 const MAX_COMMANDS = 5 const MAX_COMMAND_OUTPUT_BYTES = 16 * 1024 const COMMAND_TIMEOUT_MS = 120_000 -const MAX_GENERATION_ATTEMPTS = 2 +const MAX_GENERATION_ATTEMPTS = 3 const DEFAULT_GENERATION_TIMEOUT_MS = 120_000 const DEFAULT_GENERATION_MAX_OUTPUT_TOKENS = 8000 export const MAX_WORK_PACKAGE_EXECUTION_ATTEMPTS = 3 @@ -47,8 +48,6 @@ const ALLOWED_COMMANDS = new Set([ 'npm run lint', ]) -const ACP_EXECUTION_ENABLED_VALUES = new Set(['1', 'true', 'on', 'yes', 'enabled']) - type TaskRow = typeof tasks.$inferSelect type ProjectRow = typeof projects.$inferSelect type WorkPackageRow = typeof workPackages.$inferSelect @@ -256,8 +255,8 @@ function isAcpModel(model: LanguageModel): boolean { } function isAcpWorkPackageExecutionEnabled(env: Record