From f85ffed90361f082e91bf1428a3721dcbc1c7b1f Mon Sep 17 00:00:00 2001 From: Its-My-Work Date: Tue, 22 Sep 2026 10:08:09 +0000 Subject: [PATCH] feat(plan): archive an imported task's file to plan/imported/, stamped with its card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A card's status is never written back into its source file (the file is a draft, the board is authoritative once a card exists) — so once a plan task lands a card, its file moves from plan/tasks/ to plan/imported/ instead of staying in plan/tasks/ looking like an open proposal. Why location, not just a status field: during a room turn a bot has no list_tasks tool (only a queued background task gets _ccs_task_manager), so the board is not something it can check from inside a room — the file's location is the only cheap, reliable "already done" signal available there. ROOM_RULES now says so explicitly for every participant. - plan-lib.js: stampImported (pure) inserts imported_card/imported_at into a file's front matter, given a card id and timestamp. - plan-import.js: archivePlanFiles (fs-injectable) moves each entry from plan/tasks/ to plan/imported/, stamped; one file's failure is reported and does not stop the rest. - server.js: applyPlanImport (now async) archives only tasks that actually landed a card this run (toCreate always; a toUpdate whose card was deleted from under us does not) — a SKIPPED task (not selected, or blocked by an unselected dependency) never loses its only copy. Runs strictly after the board transaction commits; a failed archive is logged and reported (archived/archiveErrors) but never fails or undoes the import itself. Committed via room-git.js the same way a room's own changes are (ROOM_GIT=off disables it here too). Tests: plan-lib (59), plan-import (31, fake fs incl. a missing file reported without stopping the rest), plans-api (24, real server + real files: archived files actually gone from plan/tasks/ and present in plan/imported/ stamped with the REAL card id; re-import only reviews a task whose file is back; the dependent-skip case now also asserts the skipped files are untouched on disk). Whole npm test suite green. docs/plan-kanban-import.md. Not verified: a live room run reading plan/imported/ and acting on the new rule. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Y2ZSHUdJHqVsvSNQ5ZeQ3f --- docs/plan-kanban-import.md | 10 ++++++++++ plan-import.js | 30 +++++++++++++++++++++++++++++- plan-lib.js | 18 +++++++++++++++++- server.js | 35 +++++++++++++++++++++++++++++------ test/plan-import.test.js | 24 +++++++++++++++++++++++- test/plan-lib.test.js | 14 +++++++++++++- test/plans-api.test.js | 15 ++++++++++++++- 7 files changed, 135 insertions(+), 11 deletions(-) diff --git a/docs/plan-kanban-import.md b/docs/plan-kanban-import.md index cb3e07e..375c639 100644 --- a/docs/plan-kanban-import.md +++ b/docs/plan-kanban-import.md @@ -47,6 +47,16 @@ Posted automatically the moment a room's closing step changes `plan/tasks/*.md` `POST /api/plans/review` is the dry run (no writes) — usable standalone, e.g. after hand-editing plan files, without re-running the room. +## After import: plan/imported/ + +A card's own status ('backlog' → 'todo' → 'done') is never written back into its source file — the file is a draft, the board is authoritative once a card exists. Instead, `applyPlanImport` moves each imported task's file from `plan/tasks/` to `plan/imported/`, stamping its front matter with `imported_card: ` and `imported_at: ` (`plan-lib.js: stampImported`, `plan-import.js: archivePlanFiles`). + +This is the answer to "how does an agent know not to re-propose a done task": during a room turn a bot has no `list_tasks` tool (only a queued background task gets `_ccs_task_manager` — see `TASK_MANAGER_INSTRUCTION`), so the board itself is not something it can check from inside a room. The file's *location* is the only cheap, reliable signal available there, and the room's own rules say so (`plan/imported/ holds ones already turned into Kanban cards... treat those as done`). + +Safety: only a plan id that actually landed a card (in `toCreate`/`toUpdate`, and — for an update — whose card still existed) is archived; a *skipped* task (not selected, or blocked by an unselected dependency) keeps its only copy in `plan/tasks/`, never touched. The move runs strictly after the board write commits — a failure to archive a file is logged and reported back (`archived`/`archiveErrors` in the response) but never undoes or fails the import itself, since the cards are already correctly on the board by that point. The move is committed via `room-git.js` the same way a room's own file changes are (`ROOM_GIT=off` disables it here too). + +Revising an already-imported task means writing a fresh file back into `plan/tasks/` (the planner can still read `plan/imported/T-00x.md` for context) — re-approving then updates the same card via `plan_task_id`, and archives it again. + ## Known gap No manual "review this plan again" button in the UI yet — only the automatic trigger right after a room's closing step and the two HTTP endpoints. A project without a `planner` bot, or a closing step that touches no `plan/tasks/*.md` file, never shows a card. diff --git a/plan-import.js b/plan-import.js index be1343b..1655221 100644 --- a/plan-import.js +++ b/plan-import.js @@ -88,4 +88,32 @@ async function reviewPlan({ workdir, botIds, existing = {}, fsImpl } = {}) { }; } -module.exports = { readPlanFiles, reviewPlan, isMissing }; +/** + * Moves each imported task's file from plan/tasks/ to plan/imported/, stamped with the card it + * became. Runs AFTER the board write, on tasks the caller already confirmed were created/updated + * — never on a skipped one, which has no card yet and would lose its only copy. Each entry is + * independent: one file that cannot be read/written is reported and does not stop the rest (the + * cards are already on the board by this point — a partial archive is a cosmetic loose end, not a + * reason to make the import look like it failed). `entries`: [{ file, taskId, cardId }]. + * -> { moved: [taskId], errors: [{ taskId, message }] } + */ +async function archivePlanFiles({ workdir, entries, fsImpl = require('fs') }) { + const moved = [], errors = []; + const at = new Date().toISOString(); + for (const { file, taskId, cardId } of entries || []) { + const from = path.join(workdir, file); + const to = path.join(workdir, 'plan', 'imported', path.basename(file)); + try { + const content = await fsImpl.promises.readFile(from, 'utf8'); + await fsImpl.promises.mkdir(path.dirname(to), { recursive: true }); + await fsImpl.promises.writeFile(to, planLib.stampImported(content, { cardId, at })); + await fsImpl.promises.unlink(from); + moved.push(taskId); + } catch (e) { + errors.push({ taskId, message: String((e && e.message) || e).slice(0, 160) }); + } + } + return { moved, errors }; +} + +module.exports = { readPlanFiles, reviewPlan, archivePlanFiles, isMissing }; diff --git a/plan-lib.js b/plan-lib.js index 749ba34..6d99582 100644 --- a/plan-lib.js +++ b/plan-lib.js @@ -190,8 +190,24 @@ function guessTzFile(names) { return hit[0] || null; } +/** + * A task file's text with two lines appended to its front matter, recording where it landed. + * A file in plan/imported/ (server.js moves it there once its card exists) is how an agent — + * with no other way to ask "is this already on the board?" during a room turn — tells a done + * proposal apart from an open one: the id-shaped card reference in the text is only for a human + * or another tool that opens the file directly; the LOCATION is the actual, cheap-to-check signal. + * A file with no front-matter block (should not happen; parseTaskFile would have rejected it + * earlier) is returned unchanged rather than corrupted. + */ +function stampImported(content, { cardId, at }) { + const m = FRONT_MATTER_RE.exec(String(content ?? '')); + if (!m) return content; + const stamp = `imported_card: ${cardId}\nimported_at: ${at}\n`; + return `---\n${m[1]}\n${stamp}---\n${m[2]}`; +} + module.exports = { CARD_DESCRIPTION_MAX, PLAN_TASK_ID_RE, FRONT_MATTER_RE, - parseScalar, parseTaskFile, extractHeadings, headingCovered, guessTzFile, + parseScalar, parseTaskFile, extractHeadings, headingCovered, guessTzFile, stampImported, lintPlan, buildImportPlan, pickReady, }; diff --git a/server.js b/server.js index aa5698e..0223511 100644 --- a/server.js +++ b/server.js @@ -5160,6 +5160,7 @@ async function runConversationRoom(p, { bots, prompt, rosterBots }) { + `- Read a file before you change it. Change the parts that need changing; do not rewrite a whole document from scratch, ` + `and do not make one shorter unless the user asked you to.\n`) + `- The project's working directory is ${workdir || WORKDIR}. Paths are relative to it; do not invent paths such as /workspace/....\n` + + `- plan/tasks/ holds proposed tasks awaiting approval; plan/imported/ holds ones already turned into Kanban cards (each stamped with which one) — treat those as done, not as open work, and do not re-propose them.\n` + `- You have about ${turnCap} steps (tool calls). Do the research you need, then answer: a run that ends on a tool call ` + `has no answer and is lost.\n` + `- Keep it short: about ten lines at most, unless you are writing a file. A report format from your role description is for written deliverables, not for a turn here. You are ${self}.`; @@ -8124,7 +8125,7 @@ const PLAN_IMPORT_MAX = 200; // a plan is user-approved, not agent-spawned — n * every card lands as 'backlog'. -> { created, updated, skipped: [{id,reason}] } or throws if the * review itself has lint errors (the caller must not have offered Approve in that case). */ -function applyPlanImport({ workdir, review, selected }) { +async function applyPlanImport({ workdir, review, selected }) { if (!review.canApprove) { const e = new Error('plan has lint errors'); e.code = 'PLAN_LINT'; throw e; } if (review.tasks.length > PLAN_IMPORT_MAX) { const e = new Error(`plan has ${review.tasks.length} tasks, over the ${PLAN_IMPORT_MAX}-task import limit`); e.code = 'PLAN_TOO_BIG'; throw e; } const existing = existingPlanTaskMap(workdir); @@ -8132,6 +8133,10 @@ function applyPlanImport({ workdir, review, selected }) { const graph = planLib.buildImportPlan({ tasks: review.tasks, errors: [], existing, selected }); const idMap = { ...Object.fromEntries(graph.toUpdate.map(t => [t.id, existing[t.id]])) }; + // Which plan ids really got a card this run — toCreate always does; a toUpdate whose card was + // deleted from under us does not, and its file (still the only copy of that proposal) must not + // be archived as if it had one. + const landed = new Set(); let created = 0, updated = 0; db.transaction(() => { for (const t of graph.toCreate) { @@ -8143,7 +8148,7 @@ function applyPlanImport({ workdir, review, selected }) { '', 'backlog', 0, null, workdir, sqlVal(full.model), 'auto', 'single', full.max_turns || UNATTENDED_MAX_TURNS, null, null /* depends_on: filled in the remap pass below */, null, null, null, null, null, null, null, full.bot, ); - created++; + created++; landed.add(t.id); } for (const t of graph.toUpdate) { const full = byId.get(t.id); @@ -8157,11 +8162,12 @@ function applyPlanImport({ workdir, review, selected }) { row.source_session_id, row.scheduled_at, row.recurrence, row.recurrence_end_at, row.effort, row.run_engine, full.bot, taskId, ); - updated++; + updated++; landed.add(t.id); } // Second pass: depends_on remapped to real Kanban ids, now that every id in this batch has one // (mirrors /api/tasks/dispatch's own two-pass id assignment, for the same forward-reference reason). for (const t of [...graph.toCreate, ...graph.toUpdate]) { + if (!landed.has(t.id)) continue; const full = byId.get(t.id); const taskId = idMap[t.id]; const deps = (full.depends_on || []).map(d => idMap[d]).filter(Boolean); @@ -8170,7 +8176,24 @@ function applyPlanImport({ workdir, review, selected }) { db.prepare(`UPDATE tasks SET plan_task_id=? WHERE id=?`).run(full.id, taskId); } })(); - return { created, updated, skipped: graph.toSkip }; + + // The board write is done and committed — everything below is best-effort. archivePlanFiles + // moves plan/tasks/T-00x.md to plan/imported/, stamped with the card it became: an agent has no + // other way to tell "already on the board" from "still a proposal" during a room turn (it has no + // list_tasks tool there — only the ones a queued background task gets), so the file's own + // location has to say it. Only files whose plan id actually landed a card; never a skipped one, + // which has no card yet and would lose its only copy. + const archiveEntries = [...graph.toCreate, ...graph.toUpdate].filter(t => landed.has(t.id)) + .map(t => ({ file: byId.get(t.id).file, taskId: t.id, cardId: idMap[t.id] })); + let archived = { moved: [], errors: [] }; + if (archiveEntries.length) { + try { + archived = await planImport.archivePlanFiles({ workdir, entries: archiveEntries }); + if (archived.moved.length && ROOM_GIT) roomGit.checkpoint(workdir, `plan: archived ${archived.moved.length} imported task file(s)`, WM); + if (archived.errors.length) log.warn('plan archive had errors', { workdir, errors: archived.errors }); + } catch (e) { log.warn('plan archive failed', { workdir, err: e.message }); } + } + return { created, updated, skipped: graph.toSkip, archived: archived.moved.length, archiveErrors: archived.errors }; } // Dry run: parse + lint the plan/ package of a workdir, no writes. Used by the approval card and @@ -8191,7 +8214,7 @@ app.post('/api/plans/import', express.json(), async (req, res) => { try { const review = await buildPlanReview(workdir); if (!review.canApprove) return res.status(409).json({ error: 'the plan has lint errors and cannot be approved', review }); - const result = applyPlanImport({ workdir, review, selected }); + const result = await applyPlanImport({ workdir, review, selected }); log.info('Plan imported', { workdir, ...result, skipped: result.skipped.length }); res.json(result); } catch (e) { @@ -13656,7 +13679,7 @@ wss.on('connection', (ws) => { ws.send(JSON.stringify({ type: 'plan_import_result', ok: false, error: 'the plan has lint errors and cannot be approved', review, ...(tabId ? { tabId } : {}) })); return; } - const result = applyPlanImport({ workdir: wd, review, selected: Array.isArray(selected) ? selected : null }); + const result = await applyPlanImport({ workdir: wd, review, selected: Array.isArray(selected) ? selected : null }); log.info('Plan imported via WS', { workdir: wd, ...result, skipped: result.skipped.length }); // The persisted card is patched with the outcome, so a page reload shows "imported", not an // approve button that would re-run the import a second time. diff --git a/test/plan-import.test.js b/test/plan-import.test.js index 9358770..6b4f9d3 100644 --- a/test/plan-import.test.js +++ b/test/plan-import.test.js @@ -3,7 +3,7 @@ 'use strict'; const assert = require('assert'); const path = require('path'); -const { readPlanFiles, reviewPlan } = require('../plan-import'); +const { readPlanFiles, reviewPlan, archivePlanFiles } = require('../plan-import'); let pass = 0, fail = 0; function check(label, actual, expected) { @@ -29,9 +29,13 @@ function fakeFs(files) { } const enoent = (p) => Object.assign(new Error(`ENOENT: ${p}`), { code: 'ENOENT' }); return { + files, dirs, promises: { async readFile(p) { if (!(p in files)) throw enoent(p); return files[p]; }, async readdir(p) { if (!dirs.has(p)) throw enoent(p); return [...dirs.get(p)]; }, + async mkdir(p) { let d = p; while (!dirs.has(d)) { dirs.set(d, new Set()); const parent = path.dirname(d); if (parent === d) break; dirs.get(parent).add(path.basename(d)); d = parent; } }, + async writeFile(p, content) { files[p] = content; const d = path.dirname(p); if (!dirs.has(d)) dirs.set(d, new Set()); dirs.get(d).add(path.basename(p)); }, + async unlink(p) { if (!(p in files)) throw enoent(p); delete files[p]; }, }, }; } @@ -137,6 +141,24 @@ console.log('readPlanFiles:'); check('a long task body is capped for the card description, the full text stays in context', [r.tasks[0].description.length <= 2000, r.tasks[0].context.length > 2000], [true, true]); } + +console.log('archivePlanFiles:'); + { + const fs = fakeFs({ '/wd/plan/tasks/T-001.md': T1, '/wd/plan/tasks/T-002.md': T2 }); + const r = await archivePlanFiles({ workdir: '/wd', entries: [{ file: 'plan/tasks/T-001.md', taskId: 'T-001', cardId: 'card-1' }], fsImpl: fs }); + check('the file moved, none left behind', [r.moved, r.errors, '/wd/plan/tasks/T-001.md' in fs.files, '/wd/plan/imported/T-001.md' in fs.files], [['T-001'], [], false, true]); + check('the moved file is stamped', fs.files['/wd/plan/imported/T-001.md'].includes('imported_card: card-1'), true); + check('T-002 was never touched', '/wd/plan/tasks/T-002.md' in fs.files, true); + } + { + const fs = fakeFs({ '/wd/plan/tasks/T-001.md': T1 }); + const r = await archivePlanFiles({ workdir: '/wd', entries: [{ file: 'plan/tasks/T-001.md', taskId: 'T-001', cardId: 'card-1' }, { file: 'plan/tasks/GONE.md', taskId: 'T-999', cardId: 'card-2' }], fsImpl: fs }); + check('one missing file is reported by id, the other still archives', [r.moved, r.errors.map(e => e.taskId)], [['T-001'], ['T-999']]); + } + { + const r = await archivePlanFiles({ workdir: '/wd', entries: [], fsImpl: fakeFs({}) }); + check('nothing to archive: no crash, empty result', r, { moved: [], errors: [] }); + } console.log(`\n${pass} passed, ${fail} failed`); process.exit(fail ? 1 : 0); })(); diff --git a/test/plan-lib.test.js b/test/plan-lib.test.js index d6f40d9..93092a2 100644 --- a/test/plan-lib.test.js +++ b/test/plan-lib.test.js @@ -3,7 +3,7 @@ const assert = require('assert'); const { parseScalar, parseTaskFile, extractHeadings, headingCovered, - lintPlan, buildImportPlan, pickReady, PLAN_TASK_ID_RE, + lintPlan, buildImportPlan, pickReady, PLAN_TASK_ID_RE, stampImported, } = require('../plan-lib'); let pass = 0, fail = 0; @@ -143,5 +143,17 @@ console.log('buildImportPlan:'); { toCreate: [], toUpdate: [], toSkip: [{ id: 'A', reason: 'plan has lint errors' }, { id: 'B', reason: 'plan has lint errors' }, { id: 'C', reason: 'plan has lint errors' }] }); } +console.log('stampImported:'); +{ + const src = '---\nid: T-001\ntitle: x\nbot: kolya-prohramist\n---\nbody text\n'; + const out = stampImported(src, { cardId: 'mucabc123', at: '2026-09-22T10:00:00.000Z' }); + check('the two new lines are inside the front matter, before its closing ---', out, + '---\nid: T-001\ntitle: x\nbot: kolya-prohramist\nimported_card: mucabc123\nimported_at: 2026-09-22T10:00:00.000Z\n---\nbody text\n'); + check('the body is untouched', out.endsWith('---\nbody text\n'), true); + check('re-parsing the stamped file still works and still gets the original fields', parseTaskFile(out, 'x.md').task.id, 'T-001'); + check('a file with no front matter is returned unchanged, not corrupted', stampImported('no front matter', { cardId: 'x', at: 'y' }), 'no front matter'); + check('empty input does not throw', stampImported('', { cardId: 'x', at: 'y' }), ''); +} + console.log(`\n${pass} passed, ${fail} failed`); process.exit(fail ? 1 : 0); diff --git a/test/plans-api.test.js b/test/plans-api.test.js index 542766c..c301c4d 100644 --- a/test/plans-api.test.js +++ b/test/plans-api.test.js @@ -102,16 +102,25 @@ function clearPlan() { try { fs.rmSync(path.join(WORKDIR, 'plan'), { recursive: check('titles round-trip', new Set(board.map(t => t.title)), new Set(['Задача'])); const t2 = board.find(t => t.bot_id === 'taras-qa'); check('depends_on is remapped to a real Kanban id (T-002 -> T-001s real id)', JSON.parse(t2.depends_on || '[]'), [board.find(t => t.bot_id === 'kolya-prohramist').id]); + check('the response says both were archived, no errors', [r.json.archived, r.json.archiveErrors], [2, []]); + check('the source files are gone from plan/tasks/', [fs.existsSync(path.join(WORKDIR, 'plan', 'tasks', 'T-001.md')), fs.existsSync(path.join(WORKDIR, 'plan', 'tasks', 'T-002.md'))], [false, false]); + check('...and moved to plan/imported/, stamped with the real card id', [ + fs.readFileSync(path.join(WORKDIR, 'plan', 'imported', 'T-001.md'), 'utf8').includes(`imported_card: ${board.find(t => t.bot_id === 'kolya-prohramist').id}`), + fs.readFileSync(path.join(WORKDIR, 'plan', 'imported', 'T-002.md'), 'utf8').includes(`imported_card: ${t2.id}`), + ], [true, true]); } console.log('re-import: updates in place, no duplicates:'); + // T-002's file was archived away by the first import; only a task actively re-proposed (its file + // put back in plan/tasks/) is reviewed again — the archived one is not touched by this import. writeTask('T-001', { covers: ['1. Введение'], title: 'Задача (переписана)' }); { const r = await api('POST', '/api/plans/import', { workdir: WORKDIR }); - check('T-001 updates, T-002 updates too (title text unchanged but still an update, not a dup)', [r.status, r.json.created, r.json.updated], [200, 0, 2]); + check('only T-001 (its file is back) updates; T-002 (still archived) is not part of this review', [r.status, r.json.created, r.json.updated], [200, 0, 1]); const board = (await api('GET', `/api/tasks?workdir=${encodeURIComponent(WORKDIR)}`)).json; check('still exactly two cards — no duplicate created', board.length, 2); check('the title change reached the card', board.some(t => t.title === 'Задача (переписана)'), true); + check('T-001 is archived again (overwriting the earlier copy), still stamped', fs.readFileSync(path.join(WORKDIR, 'plan', 'imported', 'T-001.md'), 'utf8').includes('Задача (переписана)'), true); } console.log('a card already moved along keeps its status:'); @@ -135,6 +144,10 @@ function clearPlan() { try { fs.rmSync(path.join(WORKDIR, 'plan'), { recursive: check('only C is created; A and B (not selected) are skipped, B named why', [r.json.created, r.json.skipped.sort((x, y) => x.id.localeCompare(y.id))], [1, [{ id: 'A', reason: 'not selected' }, { id: 'B', reason: 'not selected' }]]); check('board grew by exactly one', (await api('GET', `/api/tasks?workdir=${encodeURIComponent(WORKDIR)}`)).json.length, before + 1); + check('C (imported) is archived; A and B (skipped, not on the board yet) keep their only copy in plan/tasks/', [ + fs.existsSync(path.join(WORKDIR, 'plan', 'tasks', 'C.md')), fs.existsSync(path.join(WORKDIR, 'plan', 'imported', 'C.md')), + fs.existsSync(path.join(WORKDIR, 'plan', 'tasks', 'A.md')), fs.existsSync(path.join(WORKDIR, 'plan', 'tasks', 'B.md')), + ], [false, true, true, true]); } { // deselecting A (a dependency of B) while selecting B: B must not import with a dangling depends_on