Skip to content

fix: bound task-deferred goal continuation - #42

Merged
danyel117 merged 2 commits into
prevalentWare:mainfrom
spencer2211:fix/task-deferral-latch-ceiling
Sep 7, 2026
Merged

fix: bound task-deferred goal continuation#42
danyel117 merged 2 commits into
prevalentWare:mainfrom
spencer2211:fix/task-deferral-latch-ceiling

Conversation

@spencer2211

Copy link
Copy Markdown
Contributor

Summary

  • always re-arm after a task deferral, so a goal deferred by a Task child no longer depends on a further idle event to resume
  • add max_task_block_seconds (default 900, 0 disables) as a wall-clock ceiling on how long one child session may defer goal continuation
  • apply both changes to the V1 and V2 continuation paths, which carry the identical block
  • document the new option and the bounded deferral in README.md

Why

runAutoContinue has one deferral path that writes nothing, records nothing and schedules nothing. When a task block is active and taskBlockStatus reports no retryAt, it returns after only adding the session to an in-memory set:

const taskStatus = await taskBlockStatus(sessionID)
if (taskStatus && taskStatus.blocked) {
  taskDeferredSessions.add(sessionID)
  if (taskStatus.retryAt != null) {
    scheduleSettledContinuation(sessionID, taskStatus.retryAt - Date.now(), scheduled != null)
  }
  return
}

retryAt is non-null only while a snapshot-idle hold exists, and markSnapshotIdle is reachable only for a tracked running child that is absent from the live child list, or for a live idle child that is not tracked at all. A child that is tracked as running and still listed by session.children() matches neither, so retryAt is null and nothing is scheduled. Because nothing is scheduled, nothing calls taskBlockStatus again — and refreshLiveChildren only runs from inside taskBlockStatus, so the plugin also stops re-observing live children. One such child ends auto-continuation for that goal permanently.

Nothing surfaces it. The goal keeps reading status: "active", stopReason: null, continuationFailures: 0, pendingAttempt: null, with no history entry and no lastStatus change — indistinguishable from a healthy goal that simply has not been prompted yet. The only symptom is that autoTurns stops advancing in wall-clock time.

This is the complement of the case fixed in #6. #6 handles the child that disappears from live child status; this is the child that is still present and listed, so markAbsentRunningChildren never fires for it and the pruning path #6 added is never reached. #6's own rationale already describes the deadlock class — "Since there is no terminal event to reconcile, the old task can block continuation forever." This is the same deadlock through the other door. The same reasoning covers a terminalUnreconciled record whose orchestrator turn never arrives.

The block appears twice, identically — once in the V1 runAutoContinue and once in the V2 copy — so a fix that patches only one site lands in only half the plugin. This change patches both.

The max_turn_time watchdog added in #18 does not rescue this. It is unset by default, so in a default configuration it never arms. More importantly, when it is configured it re-runs the same block check and returns on it, and README.md:162 documents that as intended: "Idle, built-in retry, session deletion, active Task children, and restricted agents suppress the retry." Active Task children are excluded from the watchdog by design, so it is not even a partial mitigation for this path. #20's bounded continuation retries do not apply either: they key off a pendingAttempt or a counted continuationFailures, and the block check runs before reserveContinuation, so this path creates neither.

The deferral itself is not the bug — #4's reasoning still holds, and a goal should not burn continuations prompting a parent that is waiting on a subagent. The bug is that this one deferral has no way back, and it is on by default.

Related issue

None filed; the analysis is in this PR. Happy to open one if you would rather track it separately.

Changes

  • src/server.ts — the task-deferral branch in both runAutoContinue copies now always calls scheduleSettledContinuation, falling back to a short bounded retry (TASK_BLOCK_RETRY_MS = 1_000) when there is no snapshot-idle hold to wait on. The retry re-enters with fromTaskDeferral = true, which the existing guard already expects, and it restores periodic refreshLiveChildren so fix: prune missing task sessions #6's pruning can still fire for a child that later vanishes.
  • src/server.tsTaskRecord gains runningSince, set by markRunning and preserved across consecutive running marks so a child that keeps reporting busy accumulates real age instead of resetting on every refresh; markTerminal clears it. A taskBlockExpired predicate bounds a running task by runningSince and a terminalUnreconciled one by the existing terminalAt, and hasBlockingTasks skips expired records. TaskTracker is shared, so this reaches both surfaces through one edit.
  • src/server.ts — new max_task_block_seconds option, wired through the existing timeoutMillisecondsFromSeconds helper exactly like max_turn_time. Defaulted on at 900 deliberately: an opt-in ceiling would leave the default configuration as wedgeable as it is today. 0 removes the ceiling.
  • test/server.test.ts — a test that a task deferral re-polls live children with no further idle event, and a test that a permanently-listed busy child stops blocking after the ceiling.
  • test/server-v2.test.ts — the V2 equivalent of the ceiling test, modelled on the existing "V2 idle continuation waits for a running child session" test with its session.deleted rescue removed. waitFor gained an optional deadline argument.
  • README.md — document max_task_block_seconds, note that a deferral now re-checks children on a short timer, and mention the ceiling in the behavior overview.
  • dist/server.js — rebuilt.

Verification

Bun 1.4.0, branched from main at 95df754:

  • bun run lint
  • bun run typecheck
  • bun run test — 215 pass / 1 fail locally. The suite is not fully green on this machine, and was not before this change either. The one failure is the timing-sensitive V2 watchdog no-response counts a failure on idle even with auto_continue false, which fails the same way on unpatched main here (3/3 runs in isolation) while the Publish workflow is green on main at 95df754 — so it is a local timing flake on this machine, not something this change introduces.
  • bun run builddist/server.js is byte-identical to the committed bundle
  • bun run pack:dry-run — 5 files, unchanged file set
  • the three new tests fail against unpatched src/server.ts (each times out at the wait), so they do exercise the fix

Reproduced independently on a second machine: the same suite is noisy there in both directions (patched and unpatched baseline both fail several tests), and the three new tests still fail against unpatched source and pass against patched. Full-suite counts on any one host are therefore not a clean signal; the focused tests are.

Separately verified against the published dist/server.js with a mocked OpenCode event bus: a control run with no child sessions delivered 8 continuations over 9 turns, while an identical run that emits one session.created with a parentID and keeps that child listed froze at autoTurns: 2 for the rest of the run with status: "active", stopReason: null, continuationFailures: 0. The only difference between the two runs is the child session.

AI attribution

Written with Claude Code (model: Claude Opus 5) and reviewed by a human before submission.

Checklist

  • bun run test — new behavior has regression coverage; the suite is green apart from one pre-existing timing-sensitive failure that reproduces identically on unpatched main (see Verification)
  • bun run lint passes
  • bun run typecheck passes
  • bun run build passes and dist/server.js is committed if server code changed
  • README/docs updated if behavior or options changed

Note: merging to main automatically publishes a new patch release to npm.

@danyel117 danyel117 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tackling the task-deferral latch. I found two blocking lifecycle issues before this is safe to merge:

  1. In the V1 path, the new ceiling never expires a listed terminal child whose result is unreconciled. refreshLiveChildren() calls markTerminal() for the same idle child on every poll (src/server.ts:632-634). Because the existing record has terminalUnreconciled === true, markTerminal() falls through and rewrites terminalAt with Date.now() (src/server.ts:664-680). taskBlockExpired() therefore always sees a fresh timestamp, so the documented terminal-unreconciled case remains blocked forever. Please preserve the original terminal timestamp and add a V1 regression test with a child that remains listed as idle and unreconciled.

  2. Both continuation paths check/re-arm task deferral before loading the current goal (src/server.ts:1117-1137 and src/server.ts:1752-1788). The scheduled task-deferral timer uses the default settle purpose, whose callback does not validate goal state. As a result, completing, clearing, or pausing a goal while a child blocks leaves a 1 Hz polling loop running until the ceiling; with max_task_block_seconds: 0, it runs indefinitely. Please validate that the goal still exists and is continuable before re-arming, cancel/stop polling for closed goals, and cover this lifecycle in both V1 and V2 tests.

The existing 216 tests, lint, and typecheck pass locally, but the new tests only exercise continuously busy children and do not catch these cases.

spencer2211 added a commit to spencer2211/opencode-goal-plugin that referenced this pull request Sep 5, 2026
… goals

Addresses both review findings on prevalentWare#42.

1. markTerminal rewrote terminalAt on every poll. refreshLiveChildren re-marks
   a listed idle child each pass, and once the record is terminal-unreconciled
   the early-return guard no longer fires, so taskBlockExpired always saw a
   fresh timestamp and the ceiling could never expire the documented
   terminal-unreconciled case. Carry the original terminalAt - and with it the
   assistant marker captured when the child first went terminal, so
   reconciliation also measures from that moment - across repeat marks of the
   same state. A genuine state change or an explicit resetReconciled starts a
   new clock.

2. Both continuation paths re-armed task deferral before loading the goal. The
   settle timer's callback does not validate goal state, so completing,
   clearing, or pausing a goal while a child blocked left a 1 Hz poll running
   until the ceiling, and indefinitely with max_task_block_seconds: 0. Both
   paths now load the goal first and stop - releasing the deferral and
   cancelling the scheduled continuation - when it is missing, closed, or
   paused. budgetLimited/usageLimited still re-arm, matching
   reserveContinuation, which serves those a wrap-up continuation.

Regressions: one V1 test per finding plus a paused variant, and a V2 lifecycle
test. Each was checked against a reverted fix; see the PR comment for the
vacuity control output.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UpT3nRyp8RkxD5FpcxLQ7
@spencer2211

Copy link
Copy Markdown
Contributor Author

Both findings are fixed in aba7497. I verified each against the source before changing anything, and both are real — thanks for catching them, particularly the first, which made the ceiling I added dead code for the exact case it was written for.

[1] The ceiling never expired a terminal-unreconciled child. Confirmed. refreshLiveChildren calls markTerminal(childID, "completed", parentSessionID) for the same listed idle child on every poll (src/server.ts:632-634). The early-return guard in markTerminal only fires when !existing.terminalUnreconciled, so once the record is unreconciled every poll fell through and rewrote terminalAt with Date.now(). taskBlockExpired therefore always compared against a fresh timestamp and the documented case stayed blocked forever — my runningSince branch worked, the terminalAt branch could not.

markTerminal now carries the original terminalAt when it is re-marking the same terminal state that is still unreconciled. A genuine state change, or an explicit resetReconciled: true from a real task-status event, still starts a new clock.

One thing I extended beyond the literal finding: the same rewrite also reset lastAssistantMessageIDAtTerminal to the current latest assistant on every poll. That field is the baseline reconciliation compares against (:742), so it should mean "the assistant message at the moment this child went terminal", not "at the moment of the most recent poll". It is preserved alongside terminalAt under the same condition. Say the word if you would rather I split that into its own commit.

[2] The deferral outlived the goal. Confirmed, in both paths. The task-block check and re-arm ran before getGoalInternal (:1117-1137 and :1752-1788), and the settle timer's callback does not validate goal state, so completing, clearing, or pausing a goal while a child blocked left a 1 Hz poll running to the ceiling — indefinitely with max_task_block_seconds: 0.

Both paths now load the goal first and, when it is missing, closed, or paused, release the deferral and cancelScheduledContinuation instead of re-arming. budgetLimited / usageLimited still re-arm deliberately: reserveContinuation serves those a wrap-up continuation rather than refusing them (src/state.ts:993), so they are still continuable and stopping the poll would strand them.

Regressions — four tests, each checked against a reverted fix.

Test Reverting Result
listed idle child whose result is never reconciled stops blocking after the task block ceiling (V1) fix 1 fails
task deferral stops polling when the goal is cleared while a child still blocks (V1) fix 2 fails
task deferral stops polling when the goal is paused while a child still blocks (V1) fix 2 fails
V2 task deferral stops re-arming when the goal is cleared while a child still blocks fix 2 fails

The V1 lifecycle tests assert on session.children() call counts, which is direct evidence the poll stopped rather than evidence a prompt was not sent.

A note on the V2 test, because my first version of it was vacuous and I want that on the record. V2's taskBlockStatus is entirely in-memory — it touches nothing on the mock context — so "no prompt was sent" cannot distinguish a stopped loop from a running one: a cleared goal is refused later in runAutoContinue either way, and the test passed with the fix reverted. It now counts the re-arm timers themselves via a setTimeout spy, which is the resource the fix is actually about, and it fails when fix 2 is reverted.

Verification (bun 1.4.0): bun run typecheck clean, bun run lint clean, dist/ rebuilt via bun run build. Full suite 219/220.

The one failure is V2 watchdog no-response counts a failure on idle even with auto_continue false, and it is pre-existing and unrelated to this branch — it fails the same way on the stock PR head with none of my new tests present (git stash + full run), and passes in isolation. It looks like a load-sensitive timing flake in the V2 suite; happy to open a separate issue for it if that is useful, but I did not want to touch it inside this PR.

@danyel117 danyel117 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the follow-up. The original terminal timestamp fix now looks correct, and I verified the branch merges cleanly with current main; lint, typecheck, 241 tests, build, and pack dry-run all pass on that integrated tree.

Two blocking items remain:

  1. taskDeferralGoalContinuable() still treats every budgetLimited / usageLimited goal as continuable. That is only true until the one wrap-up is reserved: reserveWrapup() returns null once budgetWrapupSent is true. If a child remains blocking after the wrap-up has been sent, the 1 Hz task-deferral loop keeps re-arming despite no possible continuation (indefinitely when max_task_block_seconds: 0). Please make this predicate match the actual reservation rules—limited goals should only continue while their wrap-up is still unsent—and add V1/V2 regressions for the post-wrap-up blocked-child case.

  2. The lifecycle coverage requested in the prior review is still asymmetric. V1 covers clear + pause, V2 only clear, and neither path exercises a completed/unmet goal. Please add a V2 paused case and closed-goal coverage for both paths so the new isClosedGoal branch and both continuation implementations are proven.

I approved the fork workflow so CI can run on aba7497; these findings are independent of its result.

spencer2211 and others added 2 commits September 6, 2026 22:32
A Task child session that never reports a terminal state, or whose terminal
result is never reconciled by an orchestrator turn, could defer goal
continuation forever. Bound it with a `max_task_block_seconds` ceiling
(default 900, `0` to disable), measured from `runningSince` for a listed
child and from `terminalAt` for an unreconciled terminal one.

`markTerminal` now carries the original `terminalAt` when it re-marks the same
still-unreconciled terminal state, along with the assistant marker captured at
that moment. `refreshLiveChildren` re-marks a listed idle child on every poll,
so rewriting the timestamp each time meant the terminal branch of the ceiling
could never fire for the case it exists to bound.

The deferral also validates the goal before re-arming. The retry runs at 1 Hz
and writes nothing to the goal, so a goal closed, cleared, or paused while a
child still blocks would otherwise keep polling until the ceiling - and
forever with `max_task_block_seconds: 0`.

Rebased onto b7e185c (prevalentWare#46).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MLdQ88tCqSgfEsaGR978UF
`taskDeferralGoalContinuable` treated every `budgetLimited` / `usageLimited`
goal as continuable. That holds only until the single wrap-up is reserved:
`reserveWrapup` returns null once `budgetWrapupSent` is set, so a child still
blocking after the wrap-up had been sent kept the 1 Hz poll re-arming with no
possible continuation - indefinitely with `max_task_block_seconds: 0`.

The predicate now mirrors the reservation rules instead of restating them: a
limited goal is continuable only while its wrap-up is unspent, and every other
non-active status fails `canContinue` outright.

Lifecycle coverage is now symmetric. V1 and V2 each cover cleared, paused,
completed, and the two-legged wrap-up case. Leg A of that pair is the control:
a predicate that simply refused every non-active status would pass leg B while
silently dropping the wrap-up a blocked limited goal is still owed. Both wrap-up
tests also assert the deferral returns after `update_goal_status active` clears
`budgetWrapupSent`, proving the loop stayed reachable and only the predicate
was holding it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MLdQ88tCqSgfEsaGR978UF
@spencer2211
spencer2211 force-pushed the fix/task-deferral-latch-ceiling branch from aba7497 to 963d18c Compare September 7, 2026 03:32
@spencer2211

Copy link
Copy Markdown
Contributor Author

Pushed 963d18c, rebased onto b7e185c (#46). Both findings were correct; here is what changed and how each new test was checked.

Rebase first

#46 landed on main about two hours after your review and touched the same V2 continuation path, so the branch went CONFLICTING in the meantime. src/server.ts auto-merged — your new if (!goal) branch after reserveContinuation sits well below the task-block check, and the two changes are independent. Two real conflicts:

[1] The limited-goal predicate

You are right, and the failure mode is exactly as you describe. The predicate now mirrors reserveContinuation's rules rather than restating them:

function taskDeferralGoalContinuable(goal: GoalSnapshot | null | undefined) {
  if (!goal) return false
  if (goal.status === "budgetLimited" || goal.status === "usageLimited") return !goal.budgetWrapupSent
  return goal.status === "active"
}

That is a direct read of reserveContinuation (state.ts:989-997): a limited goal routes to reserveWrapup, which returns null once budgetWrapupSent is set, and every other non-active status fails canContinue. Writing it this way also drops the isClosedGoal / !== "paused" enumeration, which was the thing that had drifted from the reservation rules in the first place.

[2] Lifecycle coverage

Now symmetric — cleared, paused, completed, and the wrap-up case on both paths:

case V1 V2
cleared existing existing
paused existing new
completed (closed goal) new new
limited, wrap-up unspent → still polls new new
limited, wrap-up spent → stops new new

The two wrap-up tests are one regression with two legs. Leg A is the control: a predicate that simply refused every non-active status would pass leg B while silently dropping the one wrap-up a blocked limited goal is still owed, so the test first proves the deferral survives budgetLimited while budgetWrapupSent is false. Then it releases the child, lets the poll reach reserveContinuation and spend the wrap-up, re-blocks, and asserts the loop stops.

I also lifted the re-arm counter out of the one V2 test that had it inline into a countTaskBlockRearms helper, since four tests now need it and it restores the global in a finally.

The first version of leg B passed for the wrong reason

Worth flagging, because it is the kind of thing a green check hides. My first draft asserted only "the poll count stops growing" after re-blocking, and it passed with the fix reverted. Instrumenting runAutoContinue showed why: the wrap-up continuation was still in flight, so activeContinuations.has(sessionID) short-circuited the entry before taskBlockStatus ever ran. The count was frozen because nothing was running at all, not because the predicate stopped it.

Both wrap-up tests now settle for the in-flight continuation first, and each carries a positive control that the loop was genuinely reachable:

  • V1 waits for the idle's own taskBlockStatus poll to land before measuring, so a frozen count afterwards means stopped rather than never started.
  • Both finish by resuming the goal with update_goal_status active — which clears budgetWrapupSent — and assert the deferral comes straight back on the same session and the same blocked child. Nothing else changed, so only the predicate was ever holding it.

Mutation checks

Every new test was run against a deliberately broken tree, not just a green one:

  • Predicate reverted to the previous form (isClosedGoal / !== "paused"): both wrap-up tests fail (V1 Expected: 7, Received: 9 poll count; V2 re-arm count 7 where 0 new re-arms are required). The other six lifecycle tests still pass, which is correct — that mutation only breaks the limited case.
  • Goal-validation guard removed entirely at both call sites (the state before the previous round): all eight lifecycle tests fail, 4/4 on each path.

Gates on 963d18c

tsc --noEmit clean, eslint . clean, npm pack --dry-run clean, bun run build regenerated dist/.

bun test is 259 pass / 1 fail. The failure is V2 watchdog no-response counts a failure on idle even with auto_continue false, and it is pre-existing and not from this branch — I checked it against a detached worktree at b7e185c with no changes: it fails 3/3 in isolation there, and stock b7e185c's own full run is 246 pass / 2 fail (that test plus restart resolves a persisted started pending attempt at the next idle). Both look load-sensitive at their 5s deadlines on this machine; this branch has strictly fewer failures than the base it sits on. Happy to be told they are green on your runners, in which case it is local noise either way.

Thanks for the two rounds — the wrap-up reservation rule is not something the deferral site makes visible, and I would not have found it from that call site.

@danyel117 danyel117 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed head 963d18c after the rebase and corrections. The limited-goal predicate now matches reserveContinuation, V1/V2 lifecycle coverage is symmetric, and no spec or blocking standards findings remain. Independent local gate: lint, typecheck, 260 tests, build, and pack dry-run all pass. The only non-blocking note is duplicated V1/V2 deferral orchestration, which is pre-existing structural symmetry rather than a correctness issue.

@danyel117
danyel117 merged commit 479af21 into prevalentWare:main Sep 7, 2026
4 checks passed
@spencer2211
spencer2211 deleted the fix/task-deferral-latch-ceiling branch September 7, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants