Skip to content
Closed
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
99 changes: 63 additions & 36 deletions ops/NEXT.md
Original file line number Diff line number Diff line change
@@ -1,49 +1,76 @@
# NEXT — Gate 3: Build the work package consumer

**Scope:** Gate 3 — close the Garden's loop. CODE task, SDK-side. The picker EMITS a work package (sdk/src/backlog-picker.ts + testdata/backlog-picker.flow.yaml, merged, four tested properties) and NOTHING consumes it — that is the missing half. Build the consumer: an SDK entrypoint taking an emitted package and turning it into something runnable, validating it has a title, a non-empty scope and a definition of done, and REFUSING with a typed reason when it does not, because a package that cannot be verified must not become work. NOTE: two previous attempts (ee5c9b3e, 06c0d6ab) did this correctly and their files were LOST before delivery by a platform fault — the build sandbox's .git points at a directory that does not exist, so writes cannot be captured. You are not duplicating live work.
# NEXT — Gate 3: File existence validation in work package consumer

## Objective

Build the SDK entrypoint that takes an emitted work package and turns it into something runnable. The consumer must validate that the package has:
- A title (non-empty string)
- A non-empty scope
- A definition of done
This run is pinned to **gate 3**. The target from ops/TARGET.md states:

> **Scope:** Harden the Garden's loop with a case it does not yet handle. CODE task, SDK-side.
>
> On main now, all merged and tested:
> - `sdk/src/backlog-picker.ts` — proposes a work package from ops/BACKLOG.md
> - `sdk/src/work-package-consumer.ts` — judges one, refusing with a typed
> reason (missing_title / missing_scope / missing_definition_of_done)
> - `testdata/backlog-picker.flow.yaml` — the flow, with its canonical spec
> - a test running the flow's real emit-package output through the consumer,
> proving the two halves interoperate in both directions
>
> So propose -> judge -> accept/refuse works end to end. What it does NOT do is
> survive a hostile or malformed backlog. Pick ONE of these and do it properly:
>
> (a) The picker reads whatever ops/BACKLOG.md contains. A malformed entry — a
> bold title with no body, an unterminated backtick, a bullet nested under
> another — should produce a typed refusal, never a crash and never a
> half-formed package. Add the handling and the tests.
>
> (b) The consumer accepts any package whose fields are present. It does not
> check that files_in_scope names paths that EXIST, so a package can be
> accepted while scoping files that are not there. Add that check as a new
> typed refusal reason, with tests.

**I choose option (b)**: add file existence checking to the work package consumer.

When any of these is missing or invalid, the consumer REFUSES with a typed reason. A package that cannot be verified must not become work.
The consumer currently validates that `files_in_scope` is a non-empty string array, but it does NOT verify that those paths actually exist. A package scoping nonexistent files can be accepted and will fail later when work begins. Add a typed refusal reason `nonexistent_files` with tests.

## Files in scope

- `sdk/src/` (new consumer code)
- `sdk/tests/` (new consumer tests)
- NO changes to `kernel/` (PR #19 is open)
- NO changes to `sdk/src/demo-hn-monitor.ts` (PR #19 is open)
- `sdk/src/work-package-consumer.ts` — add file existence validation
- `sdk/tests/work-package-consumer.test.ts` — add tests for the new behavior
- `sdk/src/index.ts` — if needed to export new types

## Definition of done

1. **SDK code exists** that consumes an emitted work package
2. **Validation tests exist** for:
- Missing title → typed refusal
- Empty title → typed refusal
- Missing scope → typed refusal
- Empty scope → typed refusal
- Missing definition of done → typed refusal
- Empty definition of done → typed refusal
- Valid package → accepted
3. **Every new test is confirmed to FAIL against current code** with literal output pasted
4. **SDK test suite passes:**
```
cd sdk && npm test
```
Paste the literal output showing all tests pass, 0 failed
5. **Final verification** — as the LAST action, run:
```
git status --porcelain
```
And paste the output to make lost writes visible immediately
All of the following must hold, per the target:

1. **Code in sdk/src** that checks every path in `files_in_scope` exists
2. A new typed refusal reason `nonexistent_files` added to `WorkPackageRefusalReason`
3. **Tests covering the new behavior AND existing behavior still passing**:
- A package with all existing files → accepted
- A package with one nonexistent file → refused with `nonexistent_files`
- A package with multiple nonexistent files → refused with `nonexistent_files`
- A package mixing existing and nonexistent files → refused with `nonexistent_files`
4. **Every new test confirmed to FAIL against current code**, with the literal failing output quoted
5. The following command passes:

```bash
cd sdk && npm test
```

Paste the complete literal output showing green tests.

6. **As the LAST action**, run and paste:

```bash
git status --porcelain
```

This makes any silent file loss visible immediately in the log.

## Out of scope

- Kernel changes (different PR)
- Integration with hn-monitor (different PR)
- Any changes to the backlog picker itself (already merged in PRs #20, #21, #22)
- Changes to flow execution or scheduling
- Do NOT touch kernel/
- Do NOT touch sdk/src/demo-hn-monitor.ts
- Do NOT touch anything under ops/
- Do NOT implement option (a) from the target (malformed backlog handling)
- Do NOT work on any other gate

The target says: "ONE cycle, about ten minutes. Small and true beats large and aspirational."
6 changes: 6 additions & 0 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,6 +12,7 @@ export interface EmittedWorkPackage {
export type WorkPackageRefusalReason =
| 'missing_title'
| 'missing_scope'
| 'nonexistent_files'
| 'missing_definition_of_done';

export type WorkPackageConsumption =
Expand All @@ -30,6 +33,9 @@ export function consumeWorkPackage(input: unknown): WorkPackageConsumption {
if (!isNonEmptyStringArray(input['definition_of_done'])) {
return { accepted: false, reason: 'missing_definition_of_done' };
}
if (!input['files_in_scope'].every((path) => existsSync(path))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Resolve scopes against the package workspace

existsSync(path) resolves relative paths against the consumer process's ambient CWD, not the workspace that emitted or will execute the package. This already makes the join test a false positive: the emitter runs in a temporary directory containing no scoped source file, but the package is accepted because the same relative path exists under the SDK test CWD. When Garden stages differ in CWD or sandbox, valid packages will be refused and invalid target-workspace packages can be accepted; pass an explicit workspace root and resolve every scope path against it.

Useful? React with 👍 / 👎.

return { accepted: false, reason: 'nonexistent_files' };
}
return { accepted: true, work: input as unknown as EmittedWorkPackage };
}

Expand Down
28 changes: 26 additions & 2 deletions sdk/tests/work-package-consumer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { consumeWorkPackage } from '../src/work-package-consumer.js';

const validPackage = {
title: 'Build the work package consumer',
files_in_scope: ['sdk/src/', 'sdk/tests/'],
files_in_scope: ['src/work-package-consumer.ts', 'tests/work-package-consumer.test.ts'],
definition_of_done: ['cd sdk && npm test'],
};

Expand Down Expand Up @@ -55,6 +55,30 @@ describe('work package consumer', () => {
it('accepts a valid package as runnable work', async () => {
expect(await consume(validPackage)).toEqual({ accepted: true, work: validPackage });
});

it('refuses one nonexistent file with a typed reason', async () => {
expect(
await consume({ ...validPackage, files_in_scope: ['does-not-exist.ts'] }),
).toEqual({ accepted: false, reason: 'nonexistent_files' });
});

it('refuses multiple nonexistent files with a typed reason', async () => {
expect(
await consume({
...validPackage,
files_in_scope: ['does-not-exist.ts', 'also-does-not-exist.ts'],
}),
).toEqual({ accepted: false, reason: 'nonexistent_files' });
});

it('refuses a mix of existing and nonexistent files with a typed reason', async () => {
expect(
await consume({
...validPackage,
files_in_scope: ['src/work-package-consumer.ts', 'does-not-exist.ts'],
}),
).toEqual({ accepted: false, reason: 'nonexistent_files' });
});
});

describe('the Garden join: picker output feeds the consumer', () => {
Expand Down Expand Up @@ -83,7 +107,7 @@ describe('the Garden join: picker output feeds the consumer', () => {
// An entry carrying a runnable command: that IS its definition of done.
writeFileSync(
join(dir, 'ops', 'BACKLOG.md'),
'# Backlog\n\n- **Actionable entry** touches `sdk/src/x.ts`, verified by `npm test --silent`\n',
'# Backlog\n\n- **Actionable entry** touches `src/work-package-consumer.ts`, verified by `npm test --silent`\n',
);
run(step('read-backlog'), dir);
run(step('select-entry'), dir);
Expand Down