-
Notifications
You must be signed in to change notification settings - Fork 0
drive: cloud run b754df4d #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,49 +1,73 @@ | ||
| # NEXT — Gate 3: Build the work package consumer | ||
| # NEXT — Gate 3: Sharpen backlog-picker actionability | ||
|
|
||
| **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. | ||
| **Scope:** Gate 3 — Improve how the Garden decides what is WORTH working on. CODE task, SDK-side. | ||
|
|
||
| ## Objective | ||
| On main now, all merged and tested: | ||
| - `sdk/src/backlog-picker.ts` — proposes a work package from ops/BACKLOG.md; exports selectBacklogEntry / packageFromEntry / validateWorkPackage | ||
| - `sdk/src/work-package-consumer.ts` — judges one, refusing with a typed reason (missing_title / missing_scope / missing_definition_of_done / nonexistent_files) | ||
| - `testdata/backlog-picker.flow.yaml` — the flow. Its `select-entry` step now scans for the first ACTIONABLE entry, validating candidates and skipping the ones that fail, and exits nonzero with NO_ACTIONABLE_BACKLOG_ENTRY when nothing qualifies. | ||
|
|
||
| Do NOT re-do any of the above. Malformed-backlog handling (PR #30) and the nonexistent-files check (PR #28) are DONE and merged. | ||
|
|
||
| ## The actual defect | ||
|
|
||
| Run `select-entry` against the real ops/BACKLOG.md. It prints: | ||
|
|
||
| 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 | ||
| SKIPPED_UNACTIONABLE=10 ... | ||
|
|
||
| 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. | ||
| and then selects a dated notes blob ("Upstream issues (2026-08-27):") as the work package. Ten genuine engineering tasks were skipped in favour of a list of links. | ||
|
|
||
| The cause: `validateWorkPackage` decides "actionable" using only two shallow signals — does the text contain a backticked path, and does it contain a multi-word backticked phrase. A notes blob full of backticked identifiers passes both. A real task written in prose ("Refuse a path-like deterministic command word when that path does not exist") fails both. | ||
|
|
||
| The guard is correct. The SELECTION is poor. That is what to fix. | ||
|
|
||
| ## Objective | ||
|
|
||
| Implement a sharper notion of actionability in `sdk/src/backlog-picker.ts` so that the backlog picker selects real engineering tasks and does NOT select notes entries. | ||
|
|
||
| ## 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/backlog-picker.ts` — improve actionability detection | ||
| - `sdk/src/index.ts` — wire in new export if it is needed | ||
| - Tests for the new behavior | ||
| - `testdata/backlog-picker.flow.yaml` — ONLY if changes needed | ||
| - `testdata/backlog-picker.spec.canonical.json` — regenerate ONLY if yaml changes | ||
|
|
||
| ## 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:** | ||
| All of the following must hold: | ||
|
|
||
| 1. **Improved actionability logic** in `sdk/src/backlog-picker.ts` that distinguishes real engineering tasks from notes blobs | ||
|
|
||
| 2. **Literal before/after evidence:** | ||
| - Quote the literal `select-entry` output BEFORE the change showing it selected "Upstream issues" | ||
| - Quote the literal `select-entry` output AFTER the change showing it selected a real engineering task | ||
|
|
||
| 3. **Test coverage:** | ||
| - Tests covering the new behavior | ||
| - EVERY new test confirmed to FAIL against current code (quote the literal failing output) | ||
| - All existing tests still passing | ||
|
|
||
| 4. **Green test suites:** | ||
| ``` | ||
| cd sdk && npm test | ||
| cd kernel && sh ../ops/cargo.sh test | ||
| ``` | ||
| Paste the literal output showing all tests pass, 0 failed | ||
| 5. **Final verification** — as the LAST action, run: | ||
| Both must pass with output quoted. | ||
|
|
||
| 5. **If testdata/backlog-picker.flow.yaml is modified:** | ||
| - Regenerate `testdata/backlog-picker.spec.canonical.json` | ||
|
|
||
| 6. **Final verification** — as the LAST action, run: | ||
| ``` | ||
| git status --porcelain | ||
| ``` | ||
| And paste the output to make lost writes visible immediately | ||
| And paste the output | ||
|
|
||
| ## 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 re-implement malformed-backlog handling** (PR #30, merged) | ||
| - **DO NOT re-implement nonexistent-files check** (PR #28, merged) | ||
| - Any work on other gates (1, 2, 4, 5, 6, 7, 8, 9) | ||
| - Any changes to the consumer logic beyond what's needed for this specific defect | ||
| - Performance optimizations unrelated to the selection problem |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,9 @@ | |
|
|
||
| /** First bold top-level bullet: `- **Title** rest`. */ | ||
| const ENTRY = /^- \*\*(.+?)\*\*\s*(.*(?:\n .*)*)/m; | ||
| const ACTION_TITLE = | ||
| /^(?:add|build|change|close|create|document|fix|implement|persist|refuse|release|remove|rename|replace|sharpen|update|validate|wire)\b/i; | ||
| const NOTES_TITLE = /^(?:notes?|release notes|upstream issues)\s*(?:\(|:|$)/i; | ||
|
|
||
| export interface BacklogEntry { | ||
| title: string; | ||
|
|
@@ -109,17 +112,28 @@ export function packageFromEntry(entry: BacklogEntry): Record<string, unknown> { | |
| ...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)), | ||
| .filter( | ||
| (candidate): candidate is string => | ||
| candidate !== undefined && !/^\/|\/\//.test(candidate), | ||
| ), | ||
| ), | ||
| ]; | ||
| const gate = blob.match(/\bgate[ -]?(\d+)\b/i); | ||
| const explicitChecks = (entry.body.match(/`[^`]+`/g) || []) | ||
| .map((candidate) => candidate.slice(1, -1)) | ||
| .filter((candidate) => /\s/.test(candidate)); | ||
| const definitionOfDone = NOTES_TITLE.test(entry.title) | ||
| ? [] | ||
|
Comment on lines
+125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a legitimate task is titled Useful? React with 👍 / 👎. |
||
| : explicitChecks.length > 0 | ||
| ? explicitChecks | ||
| : ACTION_TITLE.test(entry.title) | ||
| ? [entry.title.replace(/[.:]\s*$/, '')] | ||
| : []; | ||
| 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)), | ||
| definition_of_done: definitionOfDone, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a notes entry is titled
Notes from run 123and its body mentions both a scoped path and a quoted command, this expression does not match becausenotes?must be followed immediately by(,:, or the end of the title. The existingexplicitCheckspath then accepts the entry, soselect-entrycan still choose the same class of notes blob this change is intended to filter.Useful? React with 👍 / 👎.