diff --git a/lib/campaign-new.js b/lib/campaign-new.js index 175f2b76b..2d095e781 100644 --- a/lib/campaign-new.js +++ b/lib/campaign-new.js @@ -8,7 +8,8 @@ const fs = require('fs'); const path = require('path'); -const { newCampaignId } = require('./campaign-store'); +const { newCampaignId, getCampaign } = require('./campaign-store'); +const { CAMPAIGN_ID } = require('./campaign-schema'); const DEFAULT_BUDGET = 20; const DEFAULT_OWNER = 'mission-lead'; @@ -106,12 +107,12 @@ async function askOwner(root, view, prompt) { return { text: (answer && answer.stdout) || '', engine, ok: Boolean(answer && answer.ok) }; } -function writeJudge(root, id, script) { - const rel = path.join('atris', 'campaigns', id, 'judge.sh'); +function writeJudge(root, id, script, attempt) { + const rel = path.join('atris', 'campaigns', id, attempt === 1 ? 'judge.sh' : `judge-${attempt}.sh`); const file = path.join(root, rel); fs.mkdirSync(path.dirname(file), { recursive: true }); const body = script.startsWith('#!') ? script : `#!/bin/sh\n${script}`; - fs.writeFileSync(file, body.endsWith('\n') ? body : `${body}\n`, { mode: 0o755 }); + fs.writeFileSync(file, body.endsWith('\n') ? body : `${body}\n`, { mode: 0o755, flag: 'wx' }); return rel.split(path.sep).join('/'); } @@ -123,12 +124,13 @@ function brokenReading(reading) { // Ask, write, run; one retry with the reason when the draft passes today, // cannot run, or is not a script. -async function draftJudge({ root, goal, owner, idFlag, timeout, ask, judge }) { +async function draftJudge({ root, goal, owner, idFlag, timeout, ask, judge, store }) { const members = teamMembers(root); const hints = workspaceHints(root); let previous = ''; let id = idFlag || ''; let last = null; + let madeDir = false; for (let attempt = 1; attempt <= ATTEMPTS; attempt++) { const prompt = judgePrompt({ goal, members, hints, previous }); const answer = await ask(root, { id: id || 'new', owner, title: goal }, prompt); @@ -140,7 +142,19 @@ async function draftJudge({ root, goal, owner, idFlag, timeout, ask, judge }) { } if (!draft.title) draft.title = goal.slice(0, 80); if (!id) id = newCampaignId(draft.title); - const judgeFile = writeJudge(root, id, draft.script); + if (store && getCampaign(store, id)) return { ok: false, error: `campaign ${id} already exists` }; + if (!madeDir) { + const campaigns = path.join(root, 'atris', 'campaigns'); + fs.mkdirSync(campaigns, { recursive: true }); + try { + fs.mkdirSync(path.join(campaigns, id)); + madeDir = true; + } catch (err) { + if (err.code === 'EEXIST') return { ok: false, error: `campaign draft directory already exists: ${id}` }; + throw err; + } + } + const judgeFile = writeJudge(root, id, draft.script, attempt); const command = `sh ${judgeFile}`; const reading = await judge(command, timeout); const result = { id, draft, judgeFile, command, reading, engine: answer.engine }; @@ -156,7 +170,7 @@ async function draftJudge({ root, goal, owner, idFlag, timeout, ask, judge }) { } return { ok: true, ...result }; } - if (id && !idFlag) fs.rmSync(path.join(root, 'atris', 'campaigns', id), { recursive: true, force: true }); + if (madeDir && last) last.error += `; draft files kept at atris/campaigns/${id}`; return { ok: false, ...last }; } @@ -206,6 +220,9 @@ async function draftAndStart(args, ctx, deps = {}) { const budgetText = readFlag(args, '--budget', ''); const budget = budgetText ? Number(budgetText) : DEFAULT_BUDGET; if (!(budget > 0)) return ctx.fail('--budget must be dollars above 0'); + const idFlag = readFlag(args, '--id', ''); + if (idFlag && !CAMPAIGN_ID.test(idFlag)) return ctx.fail('--id must look like cmp-, for example cmp-ship-billing'); + if (idFlag && ctx.store && getCampaign(ctx.store, idFlag)) return ctx.fail(`campaign ${idFlag} already exists`, 1); const owner = readFlag(args, '--owner', '') || DEFAULT_OWNER; const timeout = Number(readFlag(args, '--timeout', '')) || DRAFT_TIMEOUT_SECONDS; const options = { goal, budget, owner, run: args.includes('--run') }; @@ -213,9 +230,9 @@ async function draftAndStart(args, ctx, deps = {}) { const result = await draftJudge({ root: deps.root || process.cwd(), goal, owner, timeout, - idFlag: readFlag(args, '--id', ''), + idFlag, ask: deps.ask || askOwner, - judge: ctx.judge, + judge: ctx.judge, store: ctx.store, }); if (!result.ok) { if (ctx.asJson) { diff --git a/test/campaign-new--mut-L103-7.test.js b/test/campaign-new--mut-L103-7.test.js index db285a5e7..f229a3a48 100644 --- a/test/campaign-new--mut-L103-7.test.js +++ b/test/campaign-new--mut-L103-7.test.js @@ -25,7 +25,7 @@ test('drafting uses the owner engine and model instead of the default engine', a }) }]; }); let output; - const code = await draftAndStart(['--owner', 'builder', '--id', 'fixed-draft'], { + const code = await draftAndStart(['--owner', 'builder', '--id', 'cmp-fixed-draft'], { asJson: true, judge: async () => ({ passed: false, exit: 1, summary: 'feature is missing' }), fail: (message) => assert.fail(message), diff --git a/test/campaign-new--mut-L130-7.test.js b/test/campaign-new--mut-L130-7.test.js index 8a2d5573a..32e85b9bb 100644 --- a/test/campaign-new--mut-L130-7.test.js +++ b/test/campaign-new--mut-L130-7.test.js @@ -11,7 +11,7 @@ test('drafting preserves an explicit campaign id in its output', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'campaign-explicit-id-')); try { let output; - const code = await draftAndStart(['--id', 'chosen-campaign'], { + const code = await draftAndStart(['--id', 'cmp-chosen-campaign'], { asJson: true, judge: async () => ({ passed: false, exit: 1, summary: 'not ready' }), out: (_human, json) => { output = json; }, @@ -26,9 +26,9 @@ test('drafting preserves an explicit campaign id in its output', async () => { }); assert.equal(code, 0); - assert.equal(output.draft.id, 'chosen-campaign'); - assert.equal(output.draft.judge_file, 'atris/campaigns/chosen-campaign/judge.sh'); - assert.equal(output.draft.judge, 'sh atris/campaigns/chosen-campaign/judge.sh'); + assert.equal(output.draft.id, 'cmp-chosen-campaign'); + assert.equal(output.draft.judge_file, 'atris/campaigns/cmp-chosen-campaign/judge.sh'); + assert.equal(output.draft.judge, 'sh atris/campaigns/cmp-chosen-campaign/judge.sh'); } finally { fs.rmSync(root, { recursive: true, force: true }); } diff --git a/test/campaign-new.test.js b/test/campaign-new.test.js index 8a8f31913..e5388ce35 100644 --- a/test/campaign-new.test.js +++ b/test/campaign-new.test.js @@ -101,7 +101,9 @@ test('a drafted judge that passes today gets one retry with the reason, then is assert.equal(prompts.length, 2); assert.match(prompts[1], /passed today/); assert.equal(listCampaigns(store).length, 0); - assert.deepEqual(fs.readdirSync(path.join(dir, 'atris', 'campaigns')), [], 'the failed draft is cleaned up'); + const dirs = fs.readdirSync(path.join(dir, 'atris', 'campaigns')); + assert.equal(dirs.length, 1, 'failed drafts remain available for inspection'); + assert.deepEqual(fs.readdirSync(path.join(dir, 'atris', 'campaigns', dirs[0])).sort(), ['judge-2.sh', 'judge.sh']); }); }); @@ -190,3 +192,110 @@ test('a yes at the terminal starts the drafted campaign; a no starts nothing', a }); } }); + + +test('existing campaign directory survives a new draft with the same id', async () => { + await inWorkspace(async (dir) => { + const campaignDir = path.join(dir, 'atris', 'campaigns', 'cmp-existing'); + fs.mkdirSync(campaignDir, { recursive: true }); + fs.writeFileSync(path.join(campaignDir, 'judge.sh'), 'original judge\n'); + fs.writeFileSync(path.join(campaignDir, 'notes'), 'original notes\n'); + const result = await run(['goal', '--id', 'cmp-existing'], [reply(FAILING)]); + assert.equal(result.code, 1); + assert.match(result.json.error, /directory already exists/); + assert.equal(fs.readFileSync(path.join(campaignDir, 'judge.sh'), 'utf8'), 'original judge\n'); + assert.equal(fs.readFileSync(path.join(campaignDir, 'notes'), 'utf8'), 'original notes\n'); + }); +}); + +test('unsafe campaign id is rejected before asking or running a judge', async () => { + await inWorkspace(async () => { + const result = await run(['goal', '--id', 'cmp-x;touch-pwned'], [reply(FAILING)]); + assert.equal(result.code, 2); + assert.match(result.json.error, /--id must look/); + assert.equal(result.prompts.length, 0); + assert.equal(fs.existsSync('touch-pwned'), false); + }); +}); + +test('retry leaves a judge edited after the first reading intact', async () => { + const { draftAndStart } = require('../lib/campaign-new'); + await inWorkspace(async (dir) => { + let reads = 0; + let result; + const code = await draftAndStart(['--id', 'cmp-retry'], { + asJson: true, + fail: (message) => assert.fail(message), + out: (_human, json) => { result = json; }, + judge: async () => { + if (++reads === 1) { + fs.writeFileSync(path.join(dir, 'atris/campaigns/cmp-retry/judge.sh'), 'edited while running\n'); + return { passed: true, exit: 0 }; + } + return { passed: false, exit: 1 }; + }, + }, { root: dir, goal: 'test', ask: async () => ({ engine: 'mock', text: reply(FAILING) }) }); + assert.equal(code, 0); + assert.equal(fs.readFileSync(path.join(dir, 'atris/campaigns/cmp-retry/judge.sh'), 'utf8'), 'edited while running\n'); + assert.equal(result.draft.judge_file, 'atris/campaigns/cmp-retry/judge-2.sh'); + }); +}); + +test('existing SQLite campaign without its judge directory refuses drafting before engine or judge', async () => { + const { draftAndStart } = require('../lib/campaign-new'); + const { createSqliteStore } = require('../lib/campaign-store'); + await inWorkspace(async (dir) => { + const store = createSqliteStore(path.join(dir, 'campaigns.db')); + try { + const id = 'cmp-existing'; + const target = path.join(dir, 'atris/campaigns', id); + store.append({ campaign_id: id, type: 'campaign_started', actor: 'human:test', data: { + title: 'Original', judge: `sh atris/campaigns/${id}/judge.sh`, budget: 10, + } }); + let asks = 0; + let judges = 0; + let error; + const code = await draftAndStart(['--id', id], { + store, asJson: true, + fail: (message, status) => { error = message; return status || 2; }, + out: () => {}, + judge: async () => { judges++; return { passed: false, exit: 1 }; }, + }, { root: dir, goal: 'New goal', ask: async () => { asks++; return { text: reply(FAILING) }; } }); + assert.equal(code, 1); + assert.match(error, /already exists/); + assert.equal(asks, 0); + assert.equal(judges, 0); + assert.equal(fs.existsSync(target), false); + assert.equal(store.read(id).length, 1); + } finally { store.close(); } + }); +}); + +test('campaign created while the engine is answering is refused before writing or judging', async () => { + const { draftAndStart } = require('../lib/campaign-new'); + const { createSqliteStore } = require('../lib/campaign-store'); + await inWorkspace(async (dir) => { + const store = createSqliteStore(path.join(dir, 'campaigns.db')); + const id = 'cmp-race'; + let judges = 0; + let output; + const log = console.log; + console.log = (line) => { output = JSON.parse(line); }; + try { + const code = await draftAndStart(['--id', id], { + store, asJson: true, fail: (message) => assert.fail(message), out: () => {}, + judge: async () => { judges++; return { passed: false, exit: 1 }; }, + }, { root: dir, goal: 'New goal', ask: async () => { + store.append({ campaign_id: id, type: 'campaign_started', actor: 'human:test', data: { + title: 'Original', judge: `sh atris/campaigns/${id}/judge.sh`, budget: 10, + } }); + return { text: reply(FAILING) }; + } }); + assert.equal(code, 1); + assert.match(output.error, /already exists/); + assert.equal(judges, 0); + assert.equal(fs.existsSync(path.join(dir, 'atris/campaigns', id)), false); + assert.equal(store.read(id).length, 1); + } finally { console.log = log; store.close(); } + }); +});