diff --git a/.changeset/business-knowledge-537.md b/.changeset/business-knowledge-537.md new file mode 100644 index 00000000..85704cc3 --- /dev/null +++ b/.changeset/business-knowledge-537.md @@ -0,0 +1,5 @@ +--- +"@gemstack/framework": minor +--- + +Collect business knowledge in the repo (#537). Every run now puts `.the-framework/README.md`, `.the-framework/DECISIONS.md` and `.the-framework/KNOWLEDGE-BASE.md` on the `Context:` line, so the agent reads whatever the project already knows about itself, and the post-merge prompt gained a `## Business knowledge` section asking it to fold back what the session taught that the code cannot show. The docs go with the built-in system prompt: `--vanilla` still injects nothing but the user's own dirs. `--eco-auto-maintenance` now drops the post-merge prompt's `## Maintenance` section instead of skipping the whole run, which would have taken business knowledge with it. diff --git a/packages/framework/prompts/post_merge_prompt.md b/packages/framework/prompts/post_merge_prompt.md index 2015c98b..b88c6bd5 100644 --- a/packages/framework/prompts/post_merge_prompt.md +++ b/packages/framework/prompts/post_merge_prompt.md @@ -7,3 +7,13 @@ If the changes introduced by ${{ tf.session_name }} aren't trivial and have refa ${{ tf.settings.technical_control ? '- "Apply preset `readability` on the changes introduced by ' + tf.session_name + '"\n' : '' }} If the changes introduced by ${{ tf.session_name }} can potentially lead to security issues, add the following to - "Apply preset `security_audit` on the changes introduced by ${{ tf.session_name }}" + + +## Business knowledge + +Consider whether the changes introduced by ${{ tf.session_name }} taught you something that belongs in these documents, and update them if so (create one if it doesn't exist yet): +- `.the-framework/README.md` (whole repo overview) +- `.the-framework/DECISIONS.md` (decisions taken, and why) +- `.the-framework/KNOWLEDGE-BASE.md` (business knowledge about the project) + +Only write what a future agent would need and cannot get from the code itself. diff --git a/packages/framework/src/cli.test.ts b/packages/framework/src/cli.test.ts index bad5a109..30bc11da 100644 --- a/packages/framework/src/cli.test.ts +++ b/packages/framework/src/cli.test.ts @@ -95,7 +95,7 @@ test('runPostMerge queues the follow-ups in ONE run instead of running the prese seen.push(prompt) return Promise.resolve(true) } - await runPostMerge('/work/app', '/bin/framework', io, { session_name: 'add-oauth' }, undefined, run) + await runPostMerge('/work/app', '/bin/framework', io, { session_name: 'add-oauth' }, undefined, undefined, run) // One child run, not three: it asks for TODO entries rather than doing the passes. assert.equal(seen.length, 1) const prompt = seen[0]! @@ -117,6 +117,7 @@ test('runPostMerge gates the readability entry on technical_control (#326)', asy io, { session_name: 'add-oauth', settings: { technical_control } }, undefined, + undefined, p => { seen.push(p) return Promise.resolve(true) @@ -130,7 +131,7 @@ test('runPostMerge gates the readability entry on technical_control (#326)', asy test('runPostMerge is best-effort: a failed queueing run is reported, never thrown (#326)', async () => { const { io, out } = capture() - await runPostMerge('/work/app', '/bin/framework', io, { session_name: 'add-oauth' }, undefined, () => + await runPostMerge('/work/app', '/bin/framework', io, { session_name: 'add-oauth' }, undefined, undefined, () => Promise.resolve(false), ) assert.ok(out.some(l => /post-merge queueing did not complete/.test(l))) diff --git a/packages/framework/src/cli.ts b/packages/framework/src/cli.ts index 137896ef..894abb2d 100644 --- a/packages/framework/src/cli.ts +++ b/packages/framework/src/cli.ts @@ -957,10 +957,9 @@ export async function runCli(argv: string[], io: CliIO = defaultIO): Promise => { if (!opts.postMerge || !sawReadyForMerge || stoppedCleanly || fake) return - // The post-merge prompt is exactly the maintenance section, so --eco-auto-maintenance - // (#314) leaves nothing to queue. This is the flag's target now that #326 moved that - // section out of the system prompt, where it had gone inert (#555). - if (opts.eco.autoMaintenance) return + // --eco-auto-maintenance (#314) no longer skips the whole run: since #537 this prompt + // also carries `## Business knowledge`, which the flag does not name. It drops just + // `## Maintenance` inside renderPostMergePrompt() instead. // Every line of the prompt names the session, so there is nothing to queue without one. // An agent that made changes has one; this is the agent that ignored the instruction. if (!sessionName) { @@ -975,6 +974,7 @@ export async function runCli(argv: string[], io: CliIO = defaultIO): Promise { io.out(`\nā—† post-merge: queueing quality follow-ups for ${tf.session_name}`) - const ok = await run(renderPostMergePrompt(tf), cwd, binPath, maxCost) + const ok = await run(renderPostMergePrompt(tf, eco), cwd, binPath, maxCost) if (!ok) io.out(` ! post-merge queueing did not complete cleanly.`) } diff --git a/packages/framework/src/post-merge-prompt.test.ts b/packages/framework/src/post-merge-prompt.test.ts index ff2d9ff8..778f0665 100644 --- a/packages/framework/src/post-merge-prompt.test.ts +++ b/packages/framework/src/post-merge-prompt.test.ts @@ -2,6 +2,7 @@ import { strict as assert } from 'node:assert' import { test } from 'node:test' import { renderPostMergePrompt, POST_MERGE_PROMPT_TEMPLATE } from './post-merge-prompt.js' import { TemplateFragmentError } from './prompt-template.js' +import { KNOWLEDGE_DOCS } from './system-prompt.js' test('POST_MERGE_PROMPT_TEMPLATE carries the #326 post-merge block', () => { assert.ok(POST_MERGE_PROMPT_TEMPLATE.includes('TODO_FILE: `TODO_.agent.md`')) @@ -11,6 +12,29 @@ test('POST_MERGE_PROMPT_TEMPLATE carries the #326 post-merge block', () => { } }) +test('the business-knowledge section names every knowledge doc (#537)', () => { + // The two halves of #537 are authored apart: `## Context` lists the docs from the + // KNOWLEDGE_DOCS const, this prompt names them as markdown. Pin them together, or the + // agent gets told to read one set of files and update another. + assert.ok(POST_MERGE_PROMPT_TEMPLATE.includes('## Business knowledge')) + for (const doc of KNOWLEDGE_DOCS) { + assert.ok(POST_MERGE_PROMPT_TEMPLATE.includes(doc), `missing ${doc}`) + } +}) + +test('eco.autoMaintenance drops the maintenance section and keeps the rest (#314/#537)', () => { + const prompt = renderPostMergePrompt({ session_name: 'add-oauth' }, { autoMaintenance: true }) + assert.ok(!prompt.includes('## Maintenance')) + assert.ok(!prompt.includes('Apply preset'), 'the preset entries go with the section') + // The flag names maintenance only, so business knowledge must survive it. + assert.ok(prompt.includes('## Business knowledge')) + assert.ok(prompt.includes('add-oauth')) + assert.ok(!prompt.includes('${{'), 'the dropped section takes its fragments with it') + // Absent/off eco leaves the prompt whole. + assert.ok(renderPostMergePrompt({ session_name: 'x' }, {}).includes('## Maintenance')) + assert.ok(renderPostMergePrompt({ session_name: 'x' }).includes('## Maintenance')) +}) + test('the template never nests a fragment inside another (#556)', () => { // The one place this prompt departs from the doc, and the reason why. `renderTemplate`'s // fragment regex is non-greedy, so an inner `${{ ... }}` closes the outer fragment early and diff --git a/packages/framework/src/post-merge-prompt.ts b/packages/framework/src/post-merge-prompt.ts index f110eb78..18ba7725 100644 --- a/packages/framework/src/post-merge-prompt.ts +++ b/packages/framework/src/post-merge-prompt.ts @@ -1,6 +1,6 @@ import { renderTemplate } from './prompt-template.js' import { POST_MERGE_PROMPT } from './prompts.generated.js' -import type { TfContext } from './system-prompt.js' +import { dropSection, type EcoOptions, type TfContext } from './system-prompt.js' /** * The post-merge prompt (#326), in `prompts/post_merge_prompt.md` (#551). @@ -16,9 +16,15 @@ import type { TfContext } from './system-prompt.js' * inside a backtick template literal. {@link renderTemplate}'s fragment regex is * non-greedy, so the outer fragment closes on the inner `}}` and the remainder is not * valid JS. Same branch, same output, one fragment. + * + * Two sections: `## Maintenance` queues the quality presets, and `## Business knowledge` + * (#537) asks the agent to fold what it learned back into {@link KNOWLEDGE_DOCS}. */ export const POST_MERGE_PROMPT_TEMPLATE = POST_MERGE_PROMPT +/** The section `EcoOptions.autoMaintenance` drops (#314). */ +const MAINTENANCE_HEADING = '## Maintenance' + /** What the post-merge prompt's fragments read. A subset of {@link TfContext}. */ export interface PostMergeContext { /** The session the finished run named via setSessionName(). Every line of the prompt names it. */ @@ -31,7 +37,15 @@ export interface PostMergeContext { * Render the post-merge prompt for a finished session. `settings` is defaulted rather than * left absent: the template reads `tf.settings.technical_control`, so a missing `settings` * throws a {@link TemplateFragmentError} instead of reading as off. + * + * `eco.autoMaintenance` (#314) drops `## Maintenance` here rather than skipping the whole + * run: since #537 the prompt also carries `## Business knowledge`, which the flag does not + * name and must not silently take with it. Dropped before rendering, so the dropped + * section's fragments never evaluate. */ -export function renderPostMergePrompt(tf: PostMergeContext): string { - return renderTemplate(POST_MERGE_PROMPT_TEMPLATE, { tf: { ...tf, settings: tf.settings ?? {} } }) +export function renderPostMergePrompt(tf: PostMergeContext, eco?: EcoOptions | undefined): string { + const template = eco?.autoMaintenance + ? dropSection(POST_MERGE_PROMPT_TEMPLATE, MAINTENANCE_HEADING) + : POST_MERGE_PROMPT_TEMPLATE + return renderTemplate(template, { tf: { ...tf, settings: tf.settings ?? {} } }) } diff --git a/packages/framework/src/system-prompt.test.ts b/packages/framework/src/system-prompt.test.ts index 63bc4479..e3ec86d0 100644 --- a/packages/framework/src/system-prompt.test.ts +++ b/packages/framework/src/system-prompt.test.ts @@ -5,11 +5,15 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { composeRunSystem, + KNOWLEDGE_DOCS, renderSystemPrompt, systemPromptBlock, SYSTEM_PROMPT_TEMPLATE, } from './system-prompt.js' import { loadUserSystemPrompt, SYSTEM_PROMPT_FILE } from './system-prompt-file.js' + +/** The `Context:` line the #537 knowledge docs stand up on their own, with no dirs picked. */ +const KNOWLEDGE_CONTEXT = `Context: ${KNOWLEDGE_DOCS.join(', ')}` import { AWAIT_PROTOCOL, SIGNAL_PROTOCOL } from './turn-gate.js' test('loadUserSystemPrompt reads and trims SYSTEM.md', async () => { @@ -80,13 +84,13 @@ test('renderSystemPrompt is not confused by a user prompt containing the heading assert.equal(user, sneaky) }) -test('systemPromptBlock defaults to the built-in #326 prompt alone', () => { - assert.equal(systemPromptBlock(), renderSystemPrompt().system) +test('systemPromptBlock defaults to the knowledge-doc context line + the built-in #326 prompt', () => { + assert.equal(systemPromptBlock(), [KNOWLEDGE_CONTEXT, renderSystemPrompt().system].join('\n\n')) }) test('systemPromptBlock appends the user prompt after the built-in one', () => { const block = systemPromptBlock({ user: 'Ship small PRs.' }) - assert.ok(block.startsWith('# System prompt')) + assert.ok(block.startsWith(`${KNOWLEDGE_CONTEXT}\n\n# System prompt`)) assert.ok(block.endsWith('Ship small PRs.')) assert.match(block, /AWAIT[\s\S]*Ship small PRs\./) // built-in first, then user }) @@ -103,16 +107,32 @@ test('systemPromptBlock prepends a Context line for the selected directories (#4 assert.equal(systemPromptBlock({ antiLazyPill: false, user: 'x', context: [' '] }), 'x') // blank entries dropped }) +test('systemPromptBlock puts the knowledge docs in context, after the user dirs (#537)', () => { + const block = systemPromptBlock({ user: 'Only mine.', context: ['/work/api'] }) + assert.ok(block.startsWith(`Context: /work/api, ${KNOWLEDGE_DOCS.join(', ')}\n\n`)) + // No dirs picked: the docs still stand up a Context line of their own. + assert.ok(systemPromptBlock({}).startsWith(`Context: ${KNOWLEDGE_DOCS.join(', ')}\n\n`)) +}) + +test('systemPromptBlock adds no knowledge docs when antiLazyPill is false (#537/#547)', () => { + // `--vanilla` is "Disable system prompt": the docs are framework-authored context, so + // they go with the built-in prompt. Only the user's own dirs survive it. + assert.equal(systemPromptBlock({ antiLazyPill: false }), '') + assert.equal(systemPromptBlock({ antiLazyPill: false, context: ['/work/api'] }), 'Context: /work/api') +}) + test('systemPromptBlock is the #326 prompt and the user prompt, in that order, and nothing else (#457)', () => { // The bootstrap preamble was the last text here that was neither the #326 doc nor the // user's own. Measured on four live runs: #326 alone already stops an empty-dir build // for a plan, so the override earned nothing and outranked the doc. + // The knowledge docs (#537) join the Context line, which is paths, not prompt text. const block = systemPromptBlock({ user: 'Ship small PRs.', context: ['/work/api'] }) - assert.equal(block, ['Context: /work/api', renderSystemPrompt().system, 'Ship small PRs.'].join('\n\n')) + const context = `Context: ${['/work/api', ...KNOWLEDGE_DOCS].join(', ')}` + assert.equal(block, [context, renderSystemPrompt().system, 'Ship small PRs.'].join('\n\n')) }) test('systemPromptBlock ignores a whitespace-only user prompt', () => { - assert.equal(systemPromptBlock({ user: ' ' }), renderSystemPrompt().system) + assert.equal(systemPromptBlock({ user: ' ' }), [KNOWLEDGE_CONTEXT, renderSystemPrompt().system].join('\n\n')) assert.equal(systemPromptBlock({ antiLazyPill: false, user: ' \n ' }), '') }) @@ -198,9 +218,10 @@ test('vanilla (antiLazyPill false) wins over eco: no built-in prompt at all (#31 test('composeRunSystem is exactly the #326 block + both emit protocols, and nothing else (#547)', () => { // The one assembly path both runFramework and runPrompt go through. Exact equality is - // the point: no persona, skill, or memory framing may ever be appended again. + // the point: no persona, skill, or memory framing may ever be appended again. The #537 + // knowledge docs are in front of that, on the #439 context line: paths, not prompt text. const system = composeRunSystem() - assert.equal(system, [renderSystemPrompt().system, AWAIT_PROTOCOL, SIGNAL_PROTOCOL].join('\n\n')) + assert.equal(system, [KNOWLEDGE_CONTEXT, renderSystemPrompt().system, AWAIT_PROTOCOL, SIGNAL_PROTOCOL].join('\n\n')) }) test('composeRunSystem appends nothing after the protocols, whatever the options (#547)', () => { diff --git a/packages/framework/src/system-prompt.ts b/packages/framework/src/system-prompt.ts index b7d808b3..d0a485dd 100644 --- a/packages/framework/src/system-prompt.ts +++ b/packages/framework/src/system-prompt.ts @@ -42,10 +42,10 @@ export interface EcoOptions { /** Drop `### Alternatives` (the variability-rating research section). */ autoResearch?: boolean | undefined /** - * Drop the maintenance section. Nothing to drop *here*: #326 moved that section out of - * the system prompt and into the post-merge prompt, so this flag acts on that prompt - * instead (#556) and the CLI skips it wholesale, the post-merge prompt being exactly the - * maintenance section. Listed here because it is still an {@link EcoOptions} flag. + * Drop `## Maintenance`. Nothing to drop *here*: #326 moved that section out of the + * system prompt and into the post-merge prompt, so this flag acts on that prompt + * instead (#556) ā€” see {@link ./post-merge-prompt.renderPostMergePrompt}. Listed here + * because it is still an {@link EcoOptions} flag. */ autoMaintenance?: boolean | undefined } @@ -88,6 +88,18 @@ export interface TfContext { /** The neutral context used when a caller has none: empty prompt, no modes. */ const DEFAULT_TF: TfContext = { prompt: '', params: {} } +/** + * The project-knowledge documents (#537): what the repo knows about itself, as markdown + * that travels with the code. {@link systemPromptBlock} puts them in front of every run + * as in-context paths; the post-merge prompt asks the agent to update them, which is what + * keeps them current. Workspace-relative, because that is the agent's cwd. + */ +export const KNOWLEDGE_DOCS: readonly string[] = [ + '.the-framework/README.md', + '.the-framework/DECISIONS.md', + '.the-framework/KNOWLEDGE-BASE.md', +] + /** The two halves of the rendered {@link SYSTEM_PROMPT_TEMPLATE}. */ export interface RenderedSystemPrompt { /** The `# System prompt` half: frames the session's system channel. */ @@ -108,7 +120,7 @@ const USER_PROMPT_HEADING = '\n# User prompt\n' * `### Scope` has to stop at the next `###` sibling rather than run on to the next `##` * and swallow it. */ -function dropSection(md: string, heading: string): string { +export function dropSection(md: string, heading: string): string { const at = md.indexOf(`\n${heading}`) if (at === -1) return md const level = /^#+/.exec(heading)?.[0].length ?? 2 @@ -183,8 +195,11 @@ export function systemPromptBlock(opts: SystemPromptOptions = {}): string { const parts: string[] = [] // The #439 context line goes first, so it frames whatever prompt follows (or stands // alone under `--vanilla`, where there is no built-in prompt to frame). - const context = opts.context?.map(d => d.trim()).filter(Boolean) - if (context && context.length) parts.push(`Context: ${context.join(', ')}`) + const context = opts.context?.map(d => d.trim()).filter(Boolean) ?? [] + // The knowledge docs ride with the built-in prompt, not with the user's dirs: they are + // ours, and `--vanilla` means no framework-authored prompt at all (#547 rule 3). + const inContext = opts.antiLazyPill === false ? context : [...context, ...KNOWLEDGE_DOCS] + if (inContext.length) parts.push(`Context: ${inContext.join(', ')}`) if (opts.antiLazyPill !== false) parts.push(renderSystemPrompt(opts.tf).system) const user = opts.user?.trim() if (user) parts.push(user)