Skip to content

Commit 109e77b

Browse files
Sean SmithSean Smith
authored andcommitted
fix(task): gate answer filing on return eligibility and project terminal truth
B-1 and B-3 land as one slice under CP-032 R-11: enabling the async Scope.result consumer without status-faithful finalization would make a cancelled child return an earlier turn retained fallback as degraded evidence. B-3, in attachObservation: the lifetime waiter read waitHandle and then discarded it, calling finalizeOwnerScope(Exit.void). It now maps the authoritative BackgroundJob terminal onto the owner scope Exit -- cancelled to Exit.failCause(Cause.interrupt()), error to Exit.fail, otherwise Exit.void. The Exit is the carrier because AttachmentCoordinator.finalizeScope already maps interrupt to claimCancellation, failure to degrade, and success to close, so no new coordinator entry point is added and the projection cannot drift from the mapping every other finalization site obeys. The other nine finalizeOwnerScope callsites are deliberately unchanged: Exit.void is not neutral, it claims nothing and degrades nothing, so closeNow marks an unresolved scope degraded -- the truthful no-authoritative-terminal disposition. B-1, in detect and executeSupplement: a new eligible() gate is the single path both run shapes take. With no scope, or a scope that already resolved, it returns immediately eligible evidence. Otherwise it announces the sequence, awaits invocation.result(), and derives filing identity from the controlling selected Assistant. A cancelled resolution returns undefined so nothing files. detect calls eligible AFTER its owner Assistant-error and failed-ToolPart checks, so those exits announce nothing; executeSupplement reaches it without them. That asymmetry is preserved, not copied. A-1: renderAnswer now renders the retained selected record directly instead of rebuilding {fallback, degraded:false}, which had made candidate, observed and degraded selection unreachable on every observer route. controllingAssistant is exported from task-return.ts and reuses the private select rather than restating its precedence. The synchronous return consumes Info.output and the obsolete second ownerScopeHolder.scope.result call is deleted -- it would have latched a non-message as the retained fallback. AttachmentContract.Current gains a readonly resolved flag. The eligibility gate is one-shot, but R-23 keeps an opened scope live through descendants, so a second sequential run on the same session inherited a resolution computed for a different turn, filed at the earlier position, and had its answer swallowed by the filing guard. Four parentPrompts assertions in task-attachment.test.ts caught it. eligible() now pre-checks current().resolved and takes immediate eligibility on its own turn. T-032-3 adds the K14 oracle in attachment-coordinator.test.ts: a scope holding an earlier distinctive answer as its retained fallback returns cancellation with that text absent, while the control arm finalizing the same fixture as Exit.void reproduces the defect by replaying it as degraded evidence. T-032-1 in task-attachment-scope-lifetime.test.ts is re-framed from asserting the incident as correct to proving a yield files nothing until eligible. Ref: CP-ocp-032 R-01, R-02, R-03, R-04, R-11, R-12, C2, C3
1 parent 7d89206 commit 109e77b

6 files changed

Lines changed: 310 additions & 69 deletions

File tree

‎packages/opencode/src/session/attachment/contract.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,20 @@ export type Current = {
1010
readonly candidate: boolean
1111
readonly failed: boolean
1212
readonly cancelled: boolean
13+
/**
14+
* This scope has already published its return-eligibility resolution.
15+
*
16+
* The gate is ONE-SHOT: it resolves once and latches. A scope outlives the run that resolved it —
17+
* R-23 requires an opened scope to stay live through its descendants — so a later, sequential run
18+
* on the same session can find a scope that has already spoken. Its resolution was computed for a
19+
* different turn and cannot speak for this one; a run that consumed it anyway would file at the
20+
* earlier turn position, hit the filing guard, and lose its own answer silently.
21+
*
22+
* Exposed because the Task boundary is what has to make that distinction. `locateBorrowable`
23+
* covers the scope that is already resolved at LOOKUP; this covers the scope that resolves while
24+
* a borrowed run is still in flight.
25+
*/
26+
readonly resolved: boolean
1327
}
1428

1529
export type Scope = {

‎packages/opencode/src/session/attachment/coordinator.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ export const make = Effect.gen(function* () {
325325
candidate: state.candidate !== undefined,
326326
failed: state.degraded,
327327
cancelled: state.cancelled,
328+
resolved: state.resolution !== undefined,
328329
}))
329330

330331
const own: Scope["own"] = (messageID) =>

‎packages/opencode/src/session/task-return.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,22 @@ function select(input: Extract<TaskSelectedReturn, { type: "evidence" }>) {
188188
return { assistant: input.candidate.assistant, earlierObserved: input.observed.assistant }
189189
}
190190

191+
/**
192+
* The assistant a selected evidence result actually speaks for, by the same precedence the renderer
193+
* uses. CP-032 files an answer at the CONTROLLING assistant position rather than at the run-final
194+
* one, because eligibility can select an earlier message: a degraded resolution falls through to the
195+
* retained fallback, and an observed non-clean turn outranks a later candidate. Filing at the
196+
* run-final position would then key the answer on a message the caller never receives.
197+
*
198+
* This reuses `select` rather than restating it. A second copy of the precedence would be a second
199+
* thing to keep in step, and the two disagreeing is exactly the defect it would be introduced to
200+
* prevent. Cancellation has no controlling assistant: it carries no evidence and is not filed.
201+
*/
202+
export function controllingAssistant(selected: TaskSelectedReturn): SessionV1.WithParts | undefined {
203+
if (selected.type !== "evidence") return undefined
204+
return select(selected).assistant
205+
}
206+
191207
function classify(sessionID: SessionID, selected: ReturnType<typeof select>) {
192208
const assistant = selected.assistant
193209
if (assistant.info.role !== "assistant") throw new Error("Task return does not contain an Assistant message")

‎packages/opencode/src/tool/task.ts‎

Lines changed: 122 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ import type { SessionMutation } from "../session/closure/mutation"
1414
import type { SessionPhysical } from "../session/physical-interrupt"
1515
import { SessionAdmission } from "../session/closure/admission"
1616
import { AttachmentCoordinator } from "@/session/attachment/coordinator"
17-
import { renderCancelledTask, renderNotices, renderOutput, renderSelectedTask } from "@/session/task-return"
17+
import {
18+
controllingAssistant,
19+
renderCancelledTask,
20+
renderNotices,
21+
renderOutput,
22+
renderSelectedTask,
23+
type TaskSelectedReturn,
24+
} from "@/session/task-return"
1825
import { Config } from "@/config/config"
1926
import { Cause, Deferred, Effect, Exit, Schema, Scope } from "effect"
2027
import { EffectBridge } from "@/effect/bridge"
@@ -321,28 +328,76 @@ export const TaskTool = Tool.define(
321328
})
322329

323330
/**
324-
* The detected value is the message itself, because that is what each delivery surface needs
325-
* in order to classify and render at the moment it delivers.
331+
* The filed value is the SELECTED structural result, because that is what each delivery
332+
* surface needs in order to classify and render at the moment it delivers — and both surfaces
333+
* now consume the same one rather than the observer rebuilding fallback-only evidence.
326334
*
327-
* `at` is the final assistant message's creation time — the chronology key that keeps ordering
328-
* correct across the message-id wrap. ORDERING RESTS ON THAT KEY ALONE: the answer log inserts
329-
* by `(at, position)`, and filing-ARRIVAL order is in-order too, for a reason outside this
330-
* file — runs of one child session are runner-serialized (`effect/runner.ts`), and this
331-
* detect-to-file span contains no await on an external event, so an earlier position cannot
332-
* still be unfiled when a later one arrives. There is deliberately no detection-time announce.
333-
* Putting a real await into this span would reopen the ordering window with no test failing,
334-
* and would be the condition for reinstating an ordering mechanism.
335+
* `position` and `at` come from the CONTROLLING selected assistant, not from the run-final
336+
* message. Eligibility can select an earlier one: a degraded resolution falls through to the
337+
* retained fallback, and an observed non-clean turn outranks a later candidate. `at` is still
338+
* the creation-time chronology key that keeps `(at, position)` ordering correct across the
339+
* message-id wrap.
340+
*
341+
* ORDERING NO LONGER RESTS ON THAT KEY ALONE. `eligible` below puts a real await into the
342+
* detect-to-file span, which is exactly the condition recorded for reinstating an ordering
343+
* mechanism. It is reinstated: a scoped run announces its unresolved sequence before parking,
344+
* and the answer log withholds later sequences until that announcement clears.
335345
*/
336-
const toDetected = (result: SessionV1.WithParts) =>
346+
const toDetected = (selected: TaskSelectedReturn, controlling: SessionV1.WithParts) =>
337347
({
338-
position: result.info.id,
339-
at: result.info.time.created,
340-
detected: result,
348+
position: controlling.info.id,
349+
at: controlling.info.time.created,
350+
detected: selected,
341351
}) satisfies BackgroundJob.Detected
342352

343-
// A run detects its answer and hands it back. No selection, no rendering and no comparison
344-
// happen here: a parked attachment scope affects the owner's render moment at the delivery
345-
// surface, never the filing.
353+
/**
354+
* RETURN ELIGIBILITY — the one gate every run shape passes through (CP-032 B-1).
355+
*
356+
* A run-final assistant is turn evidence, not by itself an answer. With no attachment scope it
357+
* is immediately eligible. With one — owner or supplemental alike — the scope decides, and
358+
* while it is parked this run files NOTHING. That parked turn is the CP-021 yield, and filing
359+
* it is what lost a child's real return while its attached grandchildren were still running:
360+
* the yield was published as a completed answer, the lifetime terminalized on it, and the
361+
* answer that actually came back had no observer left to reach.
362+
*
363+
* The announcement is ordering authority only and never becomes output. It is taken
364+
* immediately after detection with NO await in between, which is what keeps the delivery floor
365+
* sound: runs of one child session are runner-serialized, so an earlier sequence has always
366+
* detected — and therefore announced — before a later one can resolve and try to deliver.
367+
*
368+
* A cancelled resolution files nothing. Cancellation carries no answer payload and travels the
369+
* terminal route; filing it merely to satisfy the log shape would hand the caller a completed
370+
* envelope for a cancelled task.
371+
*/
372+
const eligible = (invocation: AttachmentCoordinator.Scope | undefined, result: SessionV1.WithParts) =>
373+
Effect.gen(function* () {
374+
// No scope, or a scope that has ALREADY published its resolution, means nothing is pending
375+
// on this turn and it is immediately eligible.
376+
//
377+
// The gate is one-shot, and a scope outlives the run that resolved it (R-23 keeps an
378+
// opened scope live through its descendants). So a second, sequential run on the same
379+
// session can reach a scope that has already spoken for an EARLIER turn. Consuming that
380+
// resolution would key this answer to the earlier turn position, where the filing guard
381+
// would swallow it — the run would execute, produce a real answer, and deliver nothing.
382+
//
383+
// `locateBorrowable` refuses a scope already resolved at LOOKUP; this covers the scope
384+
// that resolves while a borrowed run is still in flight. Immediate eligibility rather than
385+
// refusal, because unlike the admission boundary this run has already produced a genuine
386+
// answer: refusing here would discard it. Truthful too — a resolved scope accepts no new
387+
// attachments, so this turn has no outstanding work on it to wait for.
388+
if (!invocation || invocation.current().resolved) {
389+
return toDetected({ type: "evidence", fallback: result, degraded: false }, result)
390+
}
391+
const announce = yield* BackgroundJob.Announce
392+
yield* announce()
393+
const selected = yield* invocation.result(result)
394+
const controlling = controllingAssistant(selected)
395+
if (!controlling) return undefined
396+
return toDetected(selected, controlling)
397+
})
398+
399+
// A run detects its turn result, then passes it through return eligibility before handing back
400+
// anything fileable. No rendering and no comparison happen here.
346401
const detect = (input: { invocation?: AttachmentCoordinator.Scope; onAdmitted?: Effect.Effect<void> }) =>
347402
Effect.gen(function* () {
348403
const parts = yield* ops.resolvePromptParts(params.prompt)
@@ -363,7 +418,10 @@ export const TaskTool = Tool.define(
363418
if (failed?.type === "tool" && failed.state.status === "error") {
364419
return yield* Effect.fail(new Error(`Subagent failed (task_id: ${nextSession.id}): ${failed.state.error}`))
365420
}
366-
return toDetected(result)
421+
// Eligibility runs AFTER the owner error checks, so those exits announce nothing and leave
422+
// no floor to clear. `executeSupplement` deliberately has no equivalent checks and reaches
423+
// the same gate directly; that asymmetry is preserved, not copied.
424+
return yield* eligible(input.invocation, result)
367425
})
368426

369427
const causeReason = (cause: Cause.Cause<unknown>) => {
@@ -458,7 +516,10 @@ export const TaskTool = Tool.define(
458516
),
459517
)
460518
if (outcome._tag === "note") return { note: outcome.note } satisfies BackgroundJob.SequenceOutcome
461-
return toDetected(outcome.result)
519+
// Same gate as the owner path. A supplement borrowing a live scope shares its
520+
// resolution, so both waiters select one answer and the filing guard makes the second
521+
// a no-op; a supplement that opened its own scope holds it through its descendants.
522+
return yield* eligible(invocation, outcome.result)
462523
})
463524
if (!flags.experimentalBackgroundSubagents) return yield* attempt()
464525
// CP-032 R-08: BORROW, so this asks `locateBorrowable`, not raw `locate`. A scope that has
@@ -546,15 +607,18 @@ export const TaskTool = Tool.define(
546607
}
547608

548609
// Each delivery is rendered from the retained answer at the moment it is delivered: the filed
549-
// position carries the message, never a rendered form.
610+
// position carries the selected structural result, never a rendered form.
611+
//
612+
// A-1: this used to REBUILD evidence as `{ fallback: <the message>, degraded: false }`, which
613+
// discarded whatever the coordinator had actually resolved. Candidate/observed selection and
614+
// the degraded warning were therefore unreachable on every observer route — an async child
615+
// that degraded, or whose clean final turn followed an earlier observed error, was delivered
616+
// fallback-only and silently lost that evidence. The filed record is now the selected result
617+
// itself, so both delivery surfaces render the same facts.
550618
const renderAnswer = (answer: BackgroundJob.Answer) =>
551619
renderSelectedTask({
552620
sessionID: nextSession.id,
553-
selected: {
554-
type: "evidence",
555-
fallback: answer.detected as SessionV1.WithParts,
556-
degraded: false,
557-
},
621+
selected: answer.detected as TaskSelectedReturn,
558622
notes: answer.notes,
559623
})
560624

@@ -772,11 +836,34 @@ export const TaskTool = Tool.define(
772836
// OWNER-SCOPE INVOCATIONS ONLY. Extensions never open one, and a synchronous non-promoted
773837
// caller still needs its scope after the terminal for the render moment at its delivery
774838
// surface — that window contains no parent turn, so it does not gate resumption.
839+
// THE TERMINAL IS PROJECTED, NOT DISCARDED (CP-032 B-3).
840+
//
841+
// The lifetime waiter holds the one authoritative statement of how this child ended, so it
842+
// finalizes the owner scope WITH that outcome. Finalizing every terminal as `Exit.void`
843+
// closed a cancelled child as a mere degradation, and a degraded close resolves through the
844+
// evidence gate: `complete()` then reattaches the retained `state.fallback` — an earlier
845+
// successful turn — and `select()` returns it. A cancelled task would answer with stale text
846+
// from before its cancellation (CP-023 K14).
847+
//
848+
// The Exit IS the carrier, because `AttachmentCoordinator.finalizeScope` already maps one:
849+
// interrupts claim cancellation, other failures degrade, success closes normally. So this
850+
// needs no new coordinator entry point (§3.4 allows either) and cannot drift from the
851+
// mapping every other finalization site already obeys.
852+
//
853+
// No authoritative terminal means exactly that: `Exit.void` leaves an unresolved scope to
854+
// degrade through `closeNow`, and never infers completion or cancellation.
775855
if (ownerScopeHolder.scope) {
776856
yield* Effect.gen(function* () {
777857
const handle = yield* handleSource
778-
if (handle) yield* background.waitHandle({ handle })
779-
yield* finalizeOwnerScope(Exit.void)
858+
const waited = handle ? yield* background.waitHandle({ handle }) : undefined
859+
const status = waited?.info?.status
860+
yield* finalizeOwnerScope(
861+
status === "cancelled"
862+
? Exit.failCause(Cause.interrupt())
863+
: status === "error"
864+
? Exit.fail(new Error(`Task lifetime ended in error (task_id: ${nextSession.id})`))
865+
: Exit.void,
866+
)
780867
}).pipe(Effect.forkIn(scope, { startImmediately: true }))
781868
}
782869
if (!parentScope || !reservation) {
@@ -1130,13 +1217,13 @@ export const TaskTool = Tool.define(
11301217
// call's own prompt produced - and it is rendered here, at the moment of delivery, in
11311218
// owner context: the selection that may park for attached children, then classification.
11321219
// Presence is the check rather than truthiness, because an empty answer is a real one.
1133-
const detected =
1134-
result && Object.hasOwn(result, "output") ? (result.output as SessionV1.WithParts) : undefined
1135-
const selected = detected
1136-
? ownerScopeHolder.scope && !ownerScopeHolder.finalized
1137-
? yield* ownerScopeHolder.scope.result(detected)
1138-
: ({ type: "evidence", fallback: detected, degraded: false } as const)
1139-
: undefined
1220+
// Eligibility and CP-028 selection already happened INSIDE the run, before filing
1221+
// (CP-032 B-1), so this consumes the selected record rather than resolving the scope a
1222+
// second time. Resolving again here would also be actively wrong now: `Scope.result`
1223+
// latches its argument as the retained fallback, and the argument available here is no
1224+
// longer a message.
1225+
const selected =
1226+
result && Object.hasOwn(result, "output") ? (result.output as TaskSelectedReturn) : undefined
11401227
const rendered = selected
11411228
? renderSelectedTask({ sessionID: nextSession.id, selected, notes: result?.notes })
11421229
: ""

0 commit comments

Comments
 (0)