implement-loop: guard the plan-extract against silent truncation on big plans #22

New Issue

2026-07-06T13:42:55+02:00

Brummel commented

2026-07-06 13:42:55 +02:00

Filed autonomously by the /boss orchestrator while running on
project "aura". No human was in the loop; this records a
skill-system deficiency the orchestrator hit mid-run.

Motivation

A plan iteration silently ships only part of itself. On a large plan the
plan-extract step returns fewer tasks than requested, and the loop accepts the
short list and runs to a DONE verdict over the truncated subset — the missing
tasks are never attempted, yet the end-report reads as a clean completion. The
positive-evidence precondition on DONE (issue #12) does not catch this: the
truncated subset does land real edits, so the git-tree count is non-zero and the
run looks successful. The orchestrator only discovers the shortfall by manually
diffing tasks_total against the range it asked for; a run that skips that
manual check commits an incomplete iteration.

Problem

implement-loop.js extracts every requested task's verbatim block through a
single schema-bound agent() call:

// implement-loop.js:285-294
const extracted = await agent(
  `${STANDING}\n\nRead the plan at ${plan_path}. Extract ` +
    (task_range ? `tasks ${task_range[0]}..${task_range[1]} (inclusive, 1-based)` : 'every task') +
    ', each as its VERBATIM block ...',
  { model: 'sonnet', effort: 'medium', label: 'plan-extract', ... schema: TASKS_SCHEMA },
)
if (!extracted || !extracted.tasks || extracted.tasks.length === 0) {
  return { status: 'NEEDS_CONTEXT', ... }
}
tasks = extracted.tasks

Two gaps compound:

Unbounded single-shot verbatim extraction. All requested tasks' full text
is returned in one agent response. A large plan (many tasks, or a few tasks
carrying long verbatim code blocks) overflows that one response's output
budget, so the agent returns a prefix of the tasks — commonly just task 1.
Observed on a ~5200-line plan with task_range: [1, 10] yielding
tasks.length === 1.
No range-cardinality guard. The acceptance check at :291 only rejects an
empty list. A truncated non-empty list (e.g. 1 of 10) passes, and the loop
proceeds over it. Nothing compares extracted.tasks.length (or the returned
ids) to the requested [from, to] cardinality, so the truncation is silent.

Suggested directions (either or both):

Add a cardinality guard after :290: when task_range is set, require
extracted.tasks.length === task_range[1] - task_range[0] + 1 (and/or that the
returned ids cover the range); on a mismatch, BLOCKED (or NEEDS_CONTEXT)
with a "plan-extract truncated: got N of M" reason instead of proceeding.
Extract per-task rather than all-at-once — one agent() call per task id in the
range — so no single response has to carry every task's verbatim block. This
removes the output-budget ceiling entirely and makes the loop robust to plan
size.

Current workaround (orchestrator-side, not a fix): copy the workflow script,
hardcode a [from, to] slice of at most ~2 tasks, run per slice, and manually
assert tasks_total matches the slice before committing. Large single tasks are
run one per slice. This is the discipline a fix would make unnecessary.

> Filed autonomously by the `/boss` orchestrator while running on > project "aura". No human was in the loop; this records a > skill-system deficiency the orchestrator hit mid-run. ## Motivation A plan iteration silently ships only part of itself. On a large plan the `plan-extract` step returns fewer tasks than requested, and the loop accepts the short list and runs to a `DONE` verdict over the truncated subset — the missing tasks are never attempted, yet the end-report reads as a clean completion. The positive-evidence precondition on `DONE` (issue #12) does not catch this: the truncated subset does land real edits, so the git-tree count is non-zero and the run looks successful. The orchestrator only discovers the shortfall by manually diffing `tasks_total` against the range it asked for; a run that skips that manual check commits an incomplete iteration. ## Problem `implement-loop.js` extracts every requested task's verbatim block through a single schema-bound `agent()` call: ``` // implement-loop.js:285-294 const extracted = await agent( `${STANDING}\n\nRead the plan at ${plan_path}. Extract ` + (task_range ? `tasks ${task_range[0]}..${task_range[1]} (inclusive, 1-based)` : 'every task') + ', each as its VERBATIM block ...', { model: 'sonnet', effort: 'medium', label: 'plan-extract', ... schema: TASKS_SCHEMA }, ) if (!extracted || !extracted.tasks || extracted.tasks.length === 0) { return { status: 'NEEDS_CONTEXT', ... } } tasks = extracted.tasks ``` Two gaps compound: 1. **Unbounded single-shot verbatim extraction.** All requested tasks' full text is returned in one agent response. A large plan (many tasks, or a few tasks carrying long verbatim code blocks) overflows that one response's output budget, so the agent returns a *prefix* of the tasks — commonly just task 1. Observed on a ~5200-line plan with `task_range: [1, 10]` yielding `tasks.length === 1`. 2. **No range-cardinality guard.** The acceptance check at `:291` only rejects an *empty* list. A truncated non-empty list (e.g. 1 of 10) passes, and the loop proceeds over it. Nothing compares `extracted.tasks.length` (or the returned ids) to the requested `[from, to]` cardinality, so the truncation is silent. Suggested directions (either or both): - Add a cardinality guard after `:290`: when `task_range` is set, require `extracted.tasks.length === task_range[1] - task_range[0] + 1` (and/or that the returned ids cover the range); on a mismatch, `BLOCKED` (or `NEEDS_CONTEXT`) with a "plan-extract truncated: got N of M" reason instead of proceeding. - Extract per-task rather than all-at-once — one `agent()` call per task id in the range — so no single response has to carry every task's verbatim block. This removes the output-budget ceiling entirely and makes the loop robust to plan size. Current workaround (orchestrator-side, not a fix): copy the workflow script, hardcode a `[from, to]` slice of at most ~2 tasks, run per slice, and manually assert `tasks_total` matches the slice before committing. Large single tasks are run one per slice. This is the discipline a fix would make unnecessary.

Brummel added the bug label 2026-07-06 13:42:55 +02:00

Brummel referenced this issue from a commit

2026-07-06 14:14:50 +02:00

fix(implement-loop): guard plan-extract against silent truncation on big plans

Brummel closed this issue

2026-07-06 14:14:50 +02:00

Brummel commented

2026-07-06 14:15:19 +02:00

Fixed in a75a821 (main).

Design decisions (both suggested directions taken, plus one the issue did not scope):

Both directions, not either/or. Per-task extraction removes the across-all-tasks output ceiling (the root cause), and a cardinality guard is the cheap backstop on top of it — defense in depth, matching the file's existing layered-guard style.
Extraction restructured as enumerate → per-task. A plan-index pass returns only ids + one-line titles (bounded output, self-limits far later than a verbatim dump); one plan-extract:<id> call then carries each task's verbatim block.
total_tasks self-consistency backstop for the whole-plan path. The issue's direction-1 scoped the cardinality check to the ranged [from,to] case. But the default invocation has no range, so the enumerate would be the sole authority with nothing to check it against — the exact #22 symptom relocated to the (bounded) enumerate pass. plan-index now also reports total_tasks (counted from the fully-readable plan text, independent of the emitted list); enumerated ≠ total is a hard BLOCKED. This closes the asymmetry.
parallel() kept over a sequential loop. Extraction is read-only and order-independent, so the calls fan out — and parallel() (unlike Promise.all) honours the concurrency cap, which matters precisely on the big plans this issue targets. Association is made robust by binding the requested id inside each thunk and re-associating by it — never by array position, never by the agent's self-reported id — so the mapping is correct regardless of resolution order or a mislabelled response.
Well-formed-range guard. A degenerate/inverted task_range (from > to, non-integer, < 1) is now its own explicit BLOCKED instead of being swallowed into an empty task list that later mis-reports as a no-op iteration.
Documented residual (not closed): a single task whose own verbatim body overflows its dedicated response still returns an accepted prefix — there is no per-task round-trip oracle. This is far narrower than the closed across-tasks vector and is bounded by the planner's bite-sized-task invariant; recorded as an in-code comment rather than over-engineered away.

Verification: an adversarial three-lens review of the diff (correctness / contract / completeness) plus a stubbed harness driving the real script through 11 extraction scenarios (happy range, truncation → got N of M, range overshoot, unordered enumerate, big-plan ceiling removed, empty → NEEDS_CONTEXT, agent-death, enumerate self-limit 8 of 40, inverted range, from<1, single-task range) and a dedicated order- and self-id-independence association test.

Note: the review's headline objection — that parallel() is an undefined global — was a false alarm (it read only the repo's own docs); parallel() is a documented Workflow-substrate primitive and was exercised live by the review workflow itself. The valid findings (id-association robustness, the no-range backstop, the inverted-range guard, a comment overclaim) were all incorporated above.

Fixed in a75a821 (main). **Design decisions** (both suggested directions taken, plus one the issue did not scope): 1. **Both directions, not either/or.** Per-task extraction removes the *across-all-tasks* output ceiling (the root cause), and a cardinality guard is the cheap backstop on top of it — defense in depth, matching the file's existing layered-guard style. 2. **Extraction restructured as enumerate → per-task.** A `plan-index` pass returns only ids + one-line titles (bounded output, self-limits far later than a verbatim dump); one `plan-extract:<id>` call then carries each task's verbatim block. 3. **`total_tasks` self-consistency backstop for the whole-plan path.** The issue's direction-1 scoped the cardinality check to the ranged `[from,to]` case. But the *default* invocation has no range, so the enumerate would be the sole authority with nothing to check it against — the exact #22 symptom relocated to the (bounded) enumerate pass. `plan-index` now also reports `total_tasks` (counted from the fully-readable plan text, independent of the emitted list); `enumerated ≠ total` is a hard BLOCKED. This closes the asymmetry. 4. **`parallel()` kept over a sequential loop.** Extraction is read-only and order-independent, so the calls fan out — and `parallel()` (unlike `Promise.all`) honours the concurrency cap, which matters precisely on the big plans this issue targets. Association is made robust by binding the requested id *inside each thunk* and re-associating by it — never by array position, never by the agent's self-reported id — so the mapping is correct regardless of resolution order or a mislabelled response. 5. **Well-formed-range guard.** A degenerate/inverted `task_range` (from > to, non-integer, < 1) is now its own explicit BLOCKED instead of being swallowed into an empty task list that later mis-reports as a no-op iteration. 6. **Documented residual (not closed):** a *single* task whose own verbatim body overflows its dedicated response still returns an accepted prefix — there is no per-task round-trip oracle. This is far narrower than the closed across-tasks vector and is bounded by the planner's bite-sized-task invariant; recorded as an in-code comment rather than over-engineered away. **Verification:** an adversarial three-lens review of the diff (correctness / contract / completeness) plus a stubbed harness driving the real script through 11 extraction scenarios (happy range, truncation → `got N of M`, range overshoot, unordered enumerate, big-plan ceiling removed, empty → NEEDS_CONTEXT, agent-death, enumerate self-limit `8 of 40`, inverted range, from<1, single-task range) and a dedicated order- and self-id-independence association test. Note: the review's headline objection — that `parallel()` is an undefined global — was a false alarm (it read only the repo's own docs); `parallel()` is a documented Workflow-substrate primitive and was exercised live by the review workflow itself. The valid findings (id-association robustness, the no-range backstop, the inverted-range guard, a comment overclaim) were all incorporated above.

claude referenced this issue

2026-07-17 15:20:05 +02:00

Cut fixed per-iteration dispatch overhead in implement-loop; add the missing standard-mode suite gate #29

Brummel referenced this issue from a commit

2026-07-17 16:01:20 +02:00

feat(implement-loop): adaptive plan extraction — batch small plans, guarded

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Brummel/Skills#22