diff --git a/sdk/src/work-package-consumer.ts b/sdk/src/work-package-consumer.ts index 1611a1b7c..1f68b10fa 100644 --- a/sdk/src/work-package-consumer.ts +++ b/sdk/src/work-package-consumer.ts @@ -1,3 +1,5 @@ +import { existsSync } from 'node:fs'; + /** The work-package shape emitted at the SDK boundary. */ export interface EmittedWorkPackage { title: string; @@ -10,23 +12,48 @@ export interface EmittedWorkPackage { export type WorkPackageRefusalReason = | 'missing_title' | 'missing_scope' - | 'missing_definition_of_done'; + | 'missing_definition_of_done' + | 'nonexistent_files'; export type WorkPackageConsumption = | { accepted: true; work: EmittedWorkPackage } | { accepted: false; reason: WorkPackageRefusalReason }; +/** Resolve a scoped path; used to check that scoped files are really there. */ +export type PathExists = (path: string) => boolean; + +/** Default: ask the filesystem. Injectable so the check is testable. */ +const defaultPathExists: PathExists = (path) => existsSync(path); + /** * Validate an emitted package before admitting it as runnable work. * Refusals are data so callers must handle an unverifiable package explicitly. + * + * The existence check is ON by default — review rejected making it opt-in + * (PR #28, P1): a caller using the one-argument API would silently skip it, so + * the guard would not guard. `pathExists` defaults to the real filesystem and + * is injectable purely so the behaviour can be tested without one. A package + * scoping files that are not there is refused: the picker derives files_in_scope from prose + * in ops/BACKLOG.md, so a stale or mistyped entry produces a package that reads + * as actionable and sends whoever picks it up looking for something that does + * not exist. Checking is cheap; a wrong scope is not. */ -export function consumeWorkPackage(input: unknown): WorkPackageConsumption { +export function consumeWorkPackage( + input: unknown, + pathExists: PathExists = defaultPathExists, +): WorkPackageConsumption { if (!isRecord(input) || !isNonEmptyString(input['title'])) { return { accepted: false, reason: 'missing_title' }; } if (!isNonEmptyStringArray(input['files_in_scope'])) { return { accepted: false, reason: 'missing_scope' }; } + if (isNonEmptyStringArray(input['files_in_scope'])) { + const missing = input['files_in_scope'].filter((p) => !pathExists(p)); + if (missing.length > 0) { + return { accepted: false, reason: 'nonexistent_files' }; + } + } if (!isNonEmptyStringArray(input['definition_of_done'])) { return { accepted: false, reason: 'missing_definition_of_done' }; } diff --git a/sdk/tests/work-package-consumer.test.ts b/sdk/tests/work-package-consumer.test.ts index 4b8d48846..9dcef21c4 100644 --- a/sdk/tests/work-package-consumer.test.ts +++ b/sdk/tests/work-package-consumer.test.ts @@ -9,7 +9,10 @@ const validPackage = { async function consume(input: unknown) { const module = await import('../src/work-package-consumer.js'); - return module.consumeWorkPackage(input); + // Inject a permissive existence check: these tests exercise the OTHER + // refusal reasons, and should not also depend on whether fixture paths + // happen to exist on disk. + return module.consumeWorkPackage(input, () => true); } describe('work package consumer', () => { @@ -88,7 +91,7 @@ describe('the Garden join: picker output feeds the consumer', () => { run(step('read-backlog'), dir); run(step('select-entry'), dir); const accepted = JSON.parse(run(step('emit-package'), dir)); - expect(consumeWorkPackage(accepted).accepted, JSON.stringify(accepted)).toBe(true); + expect(consumeWorkPackage(accepted, () => true).accepted, JSON.stringify(accepted)).toBe(true); // An entry with no command names no way to verify itself. writeFileSync( @@ -98,7 +101,7 @@ describe('the Garden join: picker output feeds the consumer', () => { run(step('read-backlog'), dir); run(step('select-entry'), dir); const refused = JSON.parse(run(step('emit-package'), dir)); - const verdict = consumeWorkPackage(refused); + const verdict = consumeWorkPackage(refused, () => true); expect(verdict.accepted).toBe(false); expect(verdict.accepted === false && verdict.reason).toBe('missing_definition_of_done'); } finally { @@ -106,3 +109,46 @@ describe('the Garden join: picker output feeds the consumer', () => { } }); }); + +describe('scope existence', () => { + const scoped = { + title: 'Real entry', + files_in_scope: ['sdk/src/a.ts', 'sdk/src/b.ts'], + definition_of_done: ['npm test'], + }; + + it('refuses a package scoping files that do not exist', () => { + // The picker derives files_in_scope from prose in ops/BACKLOG.md, so a + // stale or mistyped entry yields a package that reads as actionable and + // sends whoever picks it up hunting for a file that was never there. + const verdict = consumeWorkPackage(scoped, () => false); + expect(verdict.accepted).toBe(false); + expect(verdict.accepted === false && verdict.reason).toBe('nonexistent_files'); + }); + + it('refuses when only some scoped files exist', () => { + const verdict = consumeWorkPackage(scoped, (p) => p === 'sdk/src/a.ts'); + expect(verdict.accepted).toBe(false); + expect(verdict.accepted === false && verdict.reason).toBe('nonexistent_files'); + }); + + it('accepts when every scoped file exists', () => { + expect(consumeWorkPackage(scoped, () => true).accepted).toBe(true); + }); + + it('checks the real filesystem when no checker is injected', () => { + // Review rejected making this opt-in (PR #28, P1): a caller using the + // one-argument API would silently skip the check, so the guard would not + // guard. The default must reach the filesystem — these paths do not exist, + // so the package is refused without anyone passing a checker. + const verdict = consumeWorkPackage(scoped); + expect(verdict.accepted).toBe(false); + expect(verdict.accepted === false && verdict.reason).toBe('nonexistent_files'); + }); + + it('accepts a package scoping files that really are present', () => { + // Injection exists to make the check testable, not to disable it. + const real = { ...scoped, files_in_scope: ['package.json'] }; + expect(consumeWorkPackage(real).accepted).toBe(true); + }); +});