Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions sdk/src/work-package-consumer.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { existsSync } from 'node:fs';

/** The work-package shape emitted at the SDK boundary. */
export interface EmittedWorkPackage {
title: string;
Expand All @@ -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' };
}
Expand Down
52 changes: 49 additions & 3 deletions sdk/tests/work-package-consumer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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(
Expand All @@ -98,11 +101,54 @@ 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 {
rmSync(dir, { recursive: true, force: true });
}
});
});

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);
});
});