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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ dist/
.rustup/
.relayflows-toolchain/
.workflow-env
.relayflow/
73 changes: 73 additions & 0 deletions sdk/src/backlog-picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ export interface BacklogEntry {
body: string;
}

export interface ValidatedWorkPackage {
title: string;
files_in_scope: string[];
definition_of_done: string[];
description?: string;
gate?: number | null;
}

export type WorkPackageValidationReason =
| 'missing_title'
| 'missing_scope'
| 'missing_definition_of_done';

export type WorkPackageValidation =
| { accepted: true; work: ValidatedWorkPackage }
| { accepted: false; reason: WorkPackageValidationReason };

/**
* Returns the selected entry, or null when the backlog holds no actionable
* one. Null is a real answer — "nothing to do" — not a failure.
Expand Down Expand Up @@ -50,3 +67,59 @@ export function renderWorkPackage(entry: BacklogEntry): string {
'',
].join('\n');
}

/** Accept a complete emitted package, or name the first missing requirement. */
export function validateWorkPackage(input: unknown): WorkPackageValidation {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wire validation into the backlog flow

When testdata/backlog-picker.flow.yaml processes a malformed entry such as - **Title only**, emit-package never invokes this function and still exits successfully with empty files_in_scope and definition_of_done arrays. The new validator is referenced only by its tests and the SDK export, so the actual picker continues emitting the same half-formed package this change is meant to prevent; integrate validation into the emitting path and its canonical spec.

AGENTS.md reference: AGENTS.md:L22-L23

Useful? React with 👍 / 👎.

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['definition_of_done'])) {
return { accepted: false, reason: 'missing_definition_of_done' };
}
return { accepted: true, work: input as unknown as ValidatedWorkPackage };
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}

function isNonEmptyString(value: unknown): value is string {
return typeof value === 'string' && value.trim().length > 0;
}

function isNonEmptyStringArray(value: unknown): value is string[] {
return Array.isArray(value) && value.length > 0 && value.every(isNonEmptyString);
}

/**
* Build a work package from a backlog entry.
*
* This lives in the SDK because two flow steps need it: `select-entry` has to
* build a candidate package to know whether an entry is actionable at all, and
* `emit-package` has to build the package it emits. When the logic was inlined
* in both, the two could drift silently — the flow would select an entry on one
* rule and describe it by another.
*/
export function packageFromEntry(entry: BacklogEntry): Record<string, unknown> {
const blob = `${entry.title} ${entry.body}`;
const files = [
...new Set(
[...blob.matchAll(/`([A-Za-z_][A-Za-z0-9._-]*(?:\/[A-Za-z0-9._-]*)+)`/g)]
.map((match) => match[1])
.filter((candidate): candidate is string => candidate !== undefined && !/^\/|\/\//.test(candidate)),
),
];
const gate = blob.match(/\bgate[ -]?(\d+)\b/i);
return {
title: entry.title,
description: entry.body,
files_in_scope: files,
gate: gate ? Number(gate[1]) : null,
definition_of_done: (entry.body.match(/`[^`]+`/g) || [])
.map((candidate) => candidate.slice(1, -1))
.filter((candidate) => /\s/.test(candidate)),
};
}
8 changes: 8 additions & 0 deletions sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ export { JOURNAL_WRITE_FAILED, PROTOCOL_VERSION } from './protocol.js';

export { JournalClient, type JournalClientOptions } from './journal-client.js';

export {
validateWorkPackage,
packageFromEntry,
type ValidatedWorkPackage,
type WorkPackageValidation,
type WorkPackageValidationReason,
} from './backlog-picker.js';

export {
consumeWorkPackage,
type EmittedWorkPackage,
Expand Down
14 changes: 10 additions & 4 deletions sdk/tests/backlog-picker-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ function stepCommands(): Record<string, string> {
}

function run(command: string, cwd: string): string {
return execFileSync('sh', ['-c', command], { cwd, encoding: 'utf8' });
// The steps run in a throwaway cwd, so point them at the real built SDK
// rather than making them hunt for one that is not there.
return execFileSync('sh', ['-c', command], {
cwd,
encoding: 'utf8',
env: { ...process.env, RELAYFLOWS_SDK_DIST: join(__dirname, '..', 'dist') },
});
}

describe('backlog-picker flow', () => {
Expand All @@ -34,7 +40,7 @@ describe('backlog-picker flow', () => {
mkdirSync(join(dir, 'ops'), { recursive: true });
writeFileSync(
join(dir, 'ops', 'BACKLOG.md'),
'# Backlog\n\n- **Original entry** the one that must win\n',
'# Backlog\n\n- **Original entry** the one that must win, touching `sdk/src/a.ts` with `the picker still selects it`\n',
);

const steps = stepCommands();
Expand Down Expand Up @@ -69,7 +75,7 @@ describe('backlog-picker flow', () => {
const steps = stepCommands();

// Run one: a real entry, which populates the shared state.
writeFileSync(join(dir, 'ops', 'BACKLOG.md'), '# Backlog\n\n- **Yesterday entry** old\n');
writeFileSync(join(dir, 'ops', 'BACKLOG.md'), '# Backlog\n\n- **Yesterday entry** old, touching `sdk/src/a.ts` with `it must not be reused`\n');
run(steps['read-backlog'], dir);
run(steps['select-entry'], dir);
expect(run(steps['emit-package'], dir)).toContain('Yesterday entry');
Expand Down Expand Up @@ -126,7 +132,7 @@ describe('backlog-picker canonical spec', () => {
writeFileSync(
join(dir, 'ops', 'BACKLOG.md'),
'# Backlog\n\n- **Scope entry** touches `regressions/` and `src/Dockerfile` and `ops/BACKLOG.md`,\n' +
' but a path that merely contains `/` is prose, not a file.\n',
' but a path that merely contains `/` is prose, not a file, and `the scope is parsed` is the goal.\n',
);
run(steps['read-backlog'], dir);
run(steps['select-entry'], dir);
Expand Down
74 changes: 72 additions & 2 deletions sdk/tests/backlog-picker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ Some preamble that is not an entry.
- **Second entry** should not be chosen
`;

async function validate(input: unknown) {
const picker = (await import('../src/backlog-picker.js')) as Record<string, unknown>;
return (picker['validateWorkPackage'] as (value: unknown) => unknown)(input);
}

describe('backlog picker', () => {
it('selects the first bold top-level bullet', () => {
const entry = selectBacklogEntry(BACKLOG);
Expand Down Expand Up @@ -65,15 +70,80 @@ describe('backlog picker', () => {
join(dir, '.relayflow', 'backlog-picker-entry.json'),
JSON.stringify({
title: 'Fix gate 3',
body: 'Update `src/file.ts`; prose that contains `/` is not a path.',
body: 'Update `src/file.ts`; prose that contains `/` is not a path, and `the scope is parsed` is the goal.',
}),
);

const output = execFileSync('sh', ['-c', command!], { cwd: dir, encoding: 'utf8' });
const output = execFileSync('sh', ['-c', command!], {
cwd: dir,
encoding: 'utf8',
// Steps run in a throwaway cwd; point them at the real built SDK.
env: { ...process.env, RELAYFLOWS_SDK_DIST: join(__dirname, '..', 'dist') },
});
const workPackage = JSON.parse(output) as { files_in_scope: string[] };
expect(workPackage.files_in_scope).toEqual(['src/file.ts']);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
});

describe('work package validation', () => {
it('accepts a package yielded by an actionable backlog', async () => {
const entry = selectBacklogEntry(
'# Backlog\n\n- **Validate packages** edit `sdk/src/backlog-picker.ts`; run `npm test`\n',
);

expect(
await validate({
title: entry?.title,
files_in_scope: ['sdk/src/backlog-picker.ts'],
definition_of_done: ['npm test'],
}),
).toEqual({
accepted: true,
work: {
title: 'Validate packages',
files_in_scope: ['sdk/src/backlog-picker.ts'],
definition_of_done: ['npm test'],
},
});
});

it('refuses a package yielded by an unverifiable backlog', async () => {
const entry = selectBacklogEntry('# Backlog\n\n- **Vague package** improve the SDK\n');

expect(
await validate({ files_in_scope: ['sdk/src/'], definition_of_done: ['npm test'] }),
).toEqual({ accepted: false, reason: 'missing_title' });
expect(
await validate({
title: ' ',
files_in_scope: ['sdk/src/'],
definition_of_done: ['npm test'],
}),
).toEqual({ accepted: false, reason: 'missing_title' });
expect(
await validate({ title: entry?.title, definition_of_done: ['npm test'] }),
).toEqual({ accepted: false, reason: 'missing_scope' });
expect(
await validate({ title: entry?.title, files_in_scope: [], definition_of_done: [] }),
).toEqual({ accepted: false, reason: 'missing_scope' });
expect(
await validate({ title: entry?.title, files_in_scope: ['sdk/src/'] }),
).toEqual({ accepted: false, reason: 'missing_definition_of_done' });
expect(
await validate({
title: entry?.title,
files_in_scope: ['sdk/src/'],
definition_of_done: [],
}),
).toEqual({ accepted: false, reason: 'missing_definition_of_done' });
});

it('refuses an empty backlog with a typed reason', async () => {
const entry = selectBacklogEntry('# Backlog\n');

expect(await validate(entry)).toEqual({ accepted: false, reason: 'missing_title' });
});
});
33 changes: 28 additions & 5 deletions sdk/tests/work-package-consumer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('the Garden join: picker output feeds the consumer', () => {
const flowPath = join(__dirname, '..', '..', 'testdata', 'backlog-picker.flow.yaml');
const flow = load(readFileSync(flowPath, 'utf8')) as { steps: Array<{ id: string; command: string }> };
const step = (id: string) => flow.steps.find((s) => s.id === id)!.command;
const run = (cmd: string, cwd: string) => execFileSync('sh', ['-c', cmd], { cwd, encoding: 'utf8' });
const run = (cmd: string, cwd: string) => execFileSync('sh', ['-c', cmd], { cwd, encoding: 'utf8', env: { ...process.env, RELAYFLOWS_SDK_DIST: join(__dirname, '..', 'dist') }, });

const dir = mkdtempSync(join(tmpdir(), 'garden-join-'));
try {
Expand All @@ -93,15 +93,38 @@ describe('the Garden join: picker output feeds the consumer', () => {
const accepted = JSON.parse(run(step('emit-package'), dir));
expect(consumeWorkPackage(accepted, () => true).accepted, JSON.stringify(accepted)).toBe(true);

// An entry with no command names no way to verify itself.
// An entry with no command names no way to verify itself. The refusal
// now happens EARLIER than it used to: PR #30 review required the flow
// to actually call the validator, so `select-entry` rejects an
// unactionable entry rather than letting emit-package hand a package
// nobody can act on to the consumer. Assert where the guard now lives.
writeFileSync(
join(dir, 'ops', 'BACKLOG.md'),
'# Backlog\n\n- **Scoped but unverifiable** touches `sdk/src/y.ts` but names no command\n',
);
run(step('read-backlog'), dir);
run(step('select-entry'), dir);
const refused = JSON.parse(run(step('emit-package'), dir));
const verdict = consumeWorkPackage(refused, () => true);
let selectionRefused = '';
try {
run(step('select-entry'), dir);
} catch (error) {
selectionRefused = String((error as { stderr?: Buffer }).stderr ?? error);
}
expect(selectionRefused, 'select-entry must refuse an entry with no definition of done').toContain(
'missing_definition_of_done',
);

// The consumer remains the second line of defence: if such a package
// ever reaches it by another route, it still refuses with the same
// typed reason.
const verdict = consumeWorkPackage(
{
title: 'Scoped but unverifiable',
description: 'touches `sdk/src/y.ts` but names no command',
files_in_scope: ['sdk/src/y.ts'],
definition_of_done: [],
},
() => true,
);
expect(verdict.accepted).toBe(false);
expect(verdict.accepted === false && verdict.reason).toBe('missing_definition_of_done');
} finally {
Expand Down
10 changes: 7 additions & 3 deletions testdata/backlog-picker.flow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ steps:
- id: read-backlog
type: deterministic
command: "mkdir -p .relayflow && cat ops/BACKLOG.md > .relayflow/backlog-picker-source.md"
- id: select-entry
- id: build-sdk
type: deterministic
dependsOn: [read-backlog]
command: "cd sdk && npm ci --silent --no-audit --no-fund && npm run build --silent"
- id: select-entry
type: deterministic
dependsOn: [read-backlog, build-sdk]
command: >-
node -e 'const fs=require("node:fs");fs.rmSync(".relayflow/backlog-picker-entry.json",{force:true});const text=fs.readFileSync(".relayflow/backlog-picker-source.md","utf8");const match=text.match(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m);if(!match)process.exit(1);const entry={title:match[1],body:match[2].replace(/\s+/g," ").trim()};fs.writeFileSync(".relayflow/backlog-picker-entry.json",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry))'
node -e 'const fs=require("node:fs");const P=require("node:path"),F=require("node:fs");const SDK=(()=>{if(process.env.RELAYFLOWS_SDK_DIST)return P.resolve(process.env.RELAYFLOWS_SDK_DIST,"backlog-picker.js");let c=process.cwd();for(;;){const f=P.join(c,"sdk","dist","backlog-picker.js");if(F.existsSync(f))return f;const up=P.dirname(c);if(up===c)throw new Error("SDK_DIST_NOT_FOUND: build the sdk or set RELAYFLOWS_SDK_DIST");c=up}})();const {validateWorkPackage,packageFromEntry:pack}=require(SDK);fs.rmSync(".relayflow/backlog-picker-entry.json",{force:true});const text=fs.readFileSync(".relayflow/backlog-picker-source.md","utf8");const entries=[...text.matchAll(/^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/gm)].map(m=>({title:m[1],body:m[2].replace(/\s+/g," ").trim()}));const skipped=[];for(const entry of entries){const verdict=validateWorkPackage(pack(entry));if(verdict.accepted){if(skipped.length)process.stderr.write("SKIPPED_UNACTIONABLE="+skipped.length+" "+skipped.join("; ")+"\n");fs.writeFileSync(".relayflow/backlog-picker-entry.json",JSON.stringify(entry));process.stdout.write(JSON.stringify(entry));process.exit(0)}skipped.push(entry.title.slice(0,40)+"["+verdict.reason+"]")}process.stderr.write("NO_ACTIONABLE_BACKLOG_ENTRY scanned="+entries.length+" "+skipped.join("; ")+"\n");process.exit(1)'
- id: emit-package
type: deterministic
dependsOn: [select-entry]
command: >-
node -e 'const fs=require("node:fs");const entry=JSON.parse(fs.readFileSync(".relayflow/backlog-picker-entry.json","utf8"));const text=entry.title+" "+entry.body;const files=[...new Set([...text.matchAll(/`([A-Za-z_][A-Za-z0-9._-]*(?:\/[A-Za-z0-9._-]*)+)`/g)].map(match=>match[1]).filter(candidate=>!/^\/|\/\//.test(candidate)))];const gate=text.match(/\bgate[ -]?(\d+)\b/i);process.stdout.write(JSON.stringify({title:entry.title,description:entry.body,files_in_scope:files,gate:gate?Number(gate[1]):null,definition_of_done:(entry.body.match(/`[^`]+`/g)||[]).map(c=>c.slice(1,-1)).filter(c=>/\s/.test(c))}))'
node -e 'const fs=require("node:fs");const P=require("node:path"),F=require("node:fs");const SDK=(()=>{if(process.env.RELAYFLOWS_SDK_DIST)return P.resolve(process.env.RELAYFLOWS_SDK_DIST,"backlog-picker.js");let c=process.cwd();for(;;){const f=P.join(c,"sdk","dist","backlog-picker.js");if(F.existsSync(f))return f;const up=P.dirname(c);if(up===c)throw new Error("SDK_DIST_NOT_FOUND: build the sdk or set RELAYFLOWS_SDK_DIST");c=up}})();const sdk=require(SDK);const entry=JSON.parse(fs.readFileSync(".relayflow/backlog-picker-entry.json","utf8"));const pkg=sdk.packageFromEntry(entry);const verdict=sdk.validateWorkPackage(pkg);if(!verdict.accepted){process.stderr.write("REFUSED_MALFORMED_BACKLOG_ENTRY reason="+verdict.reason+"\n");process.exit(1)}process.stdout.write(JSON.stringify(pkg))'
Loading