Skip to content

Commit aeb6699

Browse files
Sean SmithSean Smith
authored andcommitted
refactor(core): remove the detection-time announce/floor mechanism
The `Announce` capability and the `AnswerLog` delivery floor guarded an ordering inversion that is not constructible: a run announced its detected position the instant its prompt resolved, and the answer log withheld any later entry that an announced-but-unfiled detection preceded. Ordering now rests on the chronology key alone. The log inserts by `(at, position)` -- final assistant message creation time, with the message id breaking ties because ids wrap -- and filing-ARRIVAL order is in-order too, for a reason outside this module: runs of one child session are runner-serialized (`effect/runner.ts` `ensureRunning` -> `awaitDone`) and a run's detect-to-file span contains no await on an external event. The `(at, position)` key, `compare` and both sorted inserts are PRESERVED and are now the sole ordering guarantee. Removed: the `Announce` service; `LifetimeLedger.announced`; `AnswerLog`'s `Observe` floor field, its check in `transition` and the floor computation in `waitAnswer`; the four clear-sites (settle's `not_running` arm, settle's main arm, the success terminal, the cancellation terminal); the `fork` wrapper's `announce` closure, `clearAnnounce`, `provideService` and `ensuring`; `Announce` in the requirements channel of `StartInput`, `ExtendInput` and `ExtendExactInput`; and the `background/job.ts` re-export. `fork`'s `ledger` parameter became dead with those closures and is removed with both call sites. In the task tool, `announceDetection` becomes the pure `toDetected`. `Announce` sat in `run`'s REQUIREMENTS channel, so its presence was permissive rather than breaking for out-of-tree callers; removing it eliminates an undocumented constraint on three published signatures either way. The invariant this rests on is DOCUMENTED, NOT ENFORCED: a future edit putting a real await into the detect-to-file span would silently reopen the ordering window with no test failing. The condition is recorded at the detection site in task.ts. Tests: the two cases that existed only to exercise the floor are deleted -- the `AnswerLog.transition` unit case for the `floor` action field, and the live case driving an in-flight detection through `BackgroundJob.Announce`. Neither has a subject after this change. background-job goes 24 -> 22 tests, exactly those two; the task blast radius in packages/opencode is unchanged at 51. Before removing, the floor was made behaviourally inert in the fork this was ported from while its source text was left intact, and the suites were run: only the live floor test failed and the task blast radius was wholly unaffected. Nothing depended on the mechanism except the tests written to exercise it. Ported from the ACF fork of opencode.
1 parent 9ee5a66 commit aeb6699

4 files changed

Lines changed: 34 additions & 167 deletions

File tree

‎packages/core/src/background-job.ts‎

Lines changed: 16 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,6 @@ export type SequenceNote = { readonly note: string }
3737
/** What a run returns: a detected answer, a notice, or nothing — never a rendered string. */
3838
export type SequenceOutcome = Detected | SequenceNote | undefined
3939

40-
/**
41-
* Per-run announcement capability, provided to each run effect by the registry's own fork.
42-
*
43-
* A run announces the instant its prompt resolves, before doing anything else, so the registry
44-
* knows a detection is in flight and withholds later positions from delivery until this one
45-
* settles. Without it, two runs whose filings arrive in the opposite order to their positions
46-
* would deliver out of conversation order.
47-
*
48-
* The capability closes over the announcing run's own ledger, so a run belonging to a replaced
49-
* lifetime can only ever mark its own ledger, never a replacement's. Announcing is advisory and
50-
* self-limiting: a run that never announces is simply never withheld for, and both `settle` and
51-
* the fork's finalizer clear the mark, so a stale announcement cannot stall delivery.
52-
*/
53-
export class Announce extends Context.Service<
54-
Announce,
55-
(input: { readonly position: string; readonly at: number }) => Effect.Effect<void>
56-
>()("@opencode/BackgroundJob/Announce") {}
57-
5840
/** One filed answer at a position. */
5941
export type Answer = { readonly index: number; readonly detected: DetectedAnswer; readonly notes: readonly string[] }
6042

@@ -88,8 +70,10 @@ export type Info = {
8870
*
8971
* The log is conversation-ordered: entries insert by `(at, position)` — the run's final assistant
9072
* message creation time, with the message id breaking ties — because message ids wrap and cannot
91-
* serve as chronology alone. Delivery order is therefore conversation order even when filings
92-
* arrive inverted by run scheduling, provided the earlier detection announced first.
73+
* serve as chronology alone. Log order is therefore `(at, position)` by construction. Filing-ARRIVAL
74+
* order is in-order too, but for a reason outside this module: runs of one child session are
75+
* runner-serialized and a run's detect-to-file span contains no await, so an earlier position
76+
* cannot still be unfiled when a later one arrives.
9377
*
9478
* `baseIndex` plus dense `entries`: log index i is `entries[i - baseIndex]`, and observation is
9579
* the only thing that releases an entry.
@@ -100,7 +84,7 @@ export namespace AnswerLog {
10084
export type State = { readonly baseIndex: number; readonly entries: readonly Entry[] }
10185

10286
export type Action =
103-
| { readonly _tag: "Observe"; readonly after: number; readonly floor?: Key }
87+
| { readonly _tag: "Observe"; readonly after: number }
10488
| {
10589
readonly _tag: "Publish"
10690
readonly position: string
@@ -126,11 +110,6 @@ export namespace AnswerLog {
126110
const offset = clamped - state.baseIndex
127111
if (offset < state.entries.length) {
128112
const entry = state.entries[offset]
129-
// Withhold while an announced, still-unfiled detection precedes this entry: delivering it
130-
// would put a later position ahead of an earlier one a run has detected but not yet filed.
131-
if (action.floor !== undefined && compare(action.floor, { position: entry.position, at: entry.at }) < 0) {
132-
return { _tag: "miss", state }
133-
}
134113
// Returning answer N advances baseIndex to N+1 — the only release. Entries at or below the
135114
// observed offset leave the queue.
136115
return {
@@ -325,13 +304,6 @@ type LifetimeLedger = {
325304
/** Position identities already filed — the run-final assistant message ids. */
326305
readonly filed: Set<string>
327306
buffered: Buffered[]
328-
/**
329-
* Detections announced but not yet filed, keyed by sequence. The log withholds any entry such a
330-
* detection precedes, so a live observer cannot deliver a later position ahead of an earlier
331-
* detected one. Cleared by that run's settle and, as a net, by the fork's finalizer. An announce
332-
* on a position that is already filed is ignored when the floor is computed.
333-
*/
334-
readonly announced: Map<number, AnswerLog.Key>
335307
/** Replaced on each log append; the old one is completed after the lock by its publisher. */
336308
gate: Deferred.Deferred<void>
337309
/** Undelivered notice lines, drained into the next published answer or into the terminal Info. */
@@ -433,8 +405,7 @@ export type HandleObservation = Omit<Observation, "accepted"> & {
433405
* registry occupant, the same construction token, still running.
434406
*
435407
* `foreign_token` is the replaced-lifetime arm — the filing or failure is dropped without touching
436-
* the current occupant. `not_running` is the terminalization race — nothing files, but the run's
437-
* in-flight announcement still has to clear.
408+
* the current occupant. `not_running` is the terminalization race — nothing files.
438409
*/
439410
export type SettleAdmissibility = "admit" | "foreign_token" | "not_running"
440411
export const settleAdmissibility = (
@@ -459,8 +430,7 @@ export type StartInput = {
459430
* a second filed answer that the blocked caller will not receive.
460431
*/
461432
outstanding?: { readonly observer: string; readonly inline: string }
462-
/** May require the announcement capability; the registry's fork provides it. */
463-
run: Effect.Effect<SequenceOutcome, unknown, Announce>
433+
run: Effect.Effect<SequenceOutcome, unknown>
464434
/** Relayed to the binder for sequence zero; omission is handled by the configured binder. */
465435
admission?: Admission
466436
}
@@ -469,16 +439,14 @@ export type StartExactResult = ArmOutcome
469439

470440
export type ExtendInput = {
471441
id: string
472-
/** May require the announcement capability; the registry's fork provides it. */
473-
run: Effect.Effect<SequenceOutcome, unknown, Announce>
442+
run: Effect.Effect<SequenceOutcome, unknown>
474443
/** Each extension carries separate admission; sequence zero's consumed permit cannot be reused. */
475444
admission?: Admission
476445
}
477446

478447
export type ExtendExactInput = {
479448
lifetime: Lifetime
480-
/** May require the announcement capability; the registry's fork provides it. */
481-
run: Effect.Effect<SequenceOutcome, unknown, Announce>
449+
run: Effect.Effect<SequenceOutcome, unknown>
482450
/** Relayed to the binder for the reserved sequence. Optional - see `ExtendInput.admission`. */
483451
admission?: Admission
484452
}
@@ -636,19 +604,12 @@ export const makeWith = (binder: Binder) =>
636604
const admissibility = settleAdmissibility({ token: job.token, status: job.info.status }, token)
637605
if (admissibility === "foreign_token") return [{}, jobs]
638606
if (admissibility === "not_running") {
639-
// A late settle against a terminalized lifetime files nothing, but its in-flight
640-
// announcement must still clear so retained entries stay deliverable. The fork's
641-
// finalizer is the net for the replaced-lifetime arm, where this ledger is no longer
642-
// reachable from the map.
643-
job.ledger.announced.delete(sequence)
607+
// A late settle against a terminalized lifetime files nothing.
644608
return [{ info: snapshot(job) }, jobs]
645609
}
646610
const ledger = job.ledger
647611
const pending = job.pending - 1
648-
// This run's detection is no longer in flight: its filing, if any, commits in this same
649-
// modification, so the announcement clears first. The gate swaps on every settle so
650-
// parked observers re-evaluate the floor.
651-
ledger.announced.delete(sequence)
612+
// The gate swaps on every settle so parked observers re-evaluate the log.
652613
const gate: Deferred.Deferred<void> | undefined = ledger.gate
653614
ledger.gate = freshGate
654615

@@ -720,11 +681,8 @@ export const makeWith = (binder: Binder) =>
720681
if (retainsSecond && ledger.outstanding) ledger.notes.push(ledger.outstanding.inline)
721682
const notes = ledger.notes.splice(0)
722683
// Filed identities are retained only until every accepted run settles, and filings are
723-
// impossible past the status guard, so both identity sets clear here. Clearing the
724-
// announcements also unblocks the floor: an in-flight detection can never file after
725-
// this commit, so withholding for it would stall retained-answer delivery forever.
684+
// impossible past the status guard, so the identity set clears here.
726685
ledger.filed.clear()
727-
ledger.announced.clear()
728686
const next = {
729687
...job,
730688
onPromote: undefined,
@@ -777,41 +735,13 @@ export const makeWith = (binder: Binder) =>
777735
id: string,
778736
token: object,
779737
sequence: number,
780-
ledger: LifetimeLedger,
781-
run: Effect.Effect<SequenceOutcome, unknown, Announce>,
738+
run: Effect.Effect<SequenceOutcome, unknown>,
782739
) {
783-
// Closed over this run's own ledger, so a run belonging to a replaced lifetime can only ever
784-
// mark its own floor. Announcing is advisory: it withholds later positions from delivery until
785-
// this run settles, `settle` clears it in the same modification that files, and the finalizer
786-
// below covers every path that cannot reach `settle` — a replaced lifetime, or interruption
787-
// before settling — so a stale announcement can never stall delivery.
788-
const announce = (input: { readonly position: string; readonly at: number }) =>
789-
SynchronizedRef.update(state.jobs, (jobs) => {
790-
if (ledger.state !== "terminal") ledger.announced.set(sequence, { position: input.position, at: input.at })
791-
return jobs
792-
})
793-
const clearAnnounce = Effect.gen(function* () {
794-
const freshGate = yield* Deferred.make<void>()
795-
const gate = yield* SynchronizedRef.modify(
796-
state.jobs,
797-
(jobs): readonly [Deferred.Deferred<void> | undefined, Map<string, Active>] => {
798-
if (!ledger.announced.delete(sequence)) return [undefined, jobs]
799-
// Clearing an announcement can unblock the floor, so swap the gate to make parked
800-
// observers re-evaluate. A spurious wake costs only one full retry.
801-
const old = ledger.gate
802-
ledger.gate = freshGate
803-
return [old, jobs]
804-
},
805-
)
806-
if (gate) yield* Deferred.succeed(gate, undefined).pipe(Effect.ignore)
807-
})
808740
return yield* run.pipe(
809-
Effect.provideService(Announce, announce),
810741
Effect.matchCauseEffect({
811742
onSuccess: (outcome) => settle(id, token, sequence, Exit.succeed(outcome)),
812743
onFailure: (cause) => settle(id, token, sequence, Exit.failCause(cause)),
813744
}),
814-
Effect.ensuring(clearAnnounce),
815745
Effect.asVoid,
816746
Effect.forkIn(scope, { startImmediately: true }),
817747
)
@@ -955,7 +885,6 @@ export const makeWith = (binder: Binder) =>
955885
log: AnswerLog.empty,
956886
filed: new Set(),
957887
buffered: [],
958-
announced: new Map(),
959888
gate,
960889
notes: [],
961890
...(input.outstanding ? { outstanding: input.outstanding } : {}),
@@ -1034,7 +963,7 @@ export const makeWith = (binder: Binder) =>
1034963

1035964
if (!armed) return yield* restore(Deferred.await(registration.arm))
1036965

1037-
yield* fork(registration.scope, id, registration.token, 0, registration.ledger, restore(input.run))
966+
yield* fork(registration.scope, id, registration.token, 0, restore(input.run))
1038967
const outcome: ArmOutcome = { info: armed.info, lifetime, handle: armed.handle }
1039968
yield* Deferred.succeed(registration.arm, outcome).pipe(Effect.ignore)
1040969
return outcome
@@ -1116,7 +1045,7 @@ export const makeWith = (binder: Binder) =>
11161045
// An accepted supplemental run registers and runs without waiting for any previous run's
11171046
// tail: the serial hold is gone, so several runs on one lifetime can be in flight at once
11181047
// and their answers are ordered by position at delivery rather than by execution.
1119-
yield* fork(result.scope, input.lifetime.id, result.token, result.sequence, result.ledger, restore(input.run))
1048+
yield* fork(result.scope, input.lifetime.id, result.token, result.sequence, restore(input.run))
11201049
return { extended: true as const, sequence: result.sequence, handle: accepted }
11211050
}),
11221051
)
@@ -1281,10 +1210,9 @@ export const makeWith = (binder: Binder) =>
12811210
// A cancellation terminal carries no answer payload: filed answers stay retained and remain
12821211
// retrievable by task id. Undelivered notice lines drain into the terminal Info. Identity
12831212
// state clears at the terminal boundary, because filings are impossible past the status
1284-
// guard and a stale in-flight announcement must not withhold retained-answer delivery.
1213+
// guard.
12851214
const notes = job.ledger.notes.splice(0)
12861215
job.ledger.filed.clear()
1287-
job.ledger.announced.clear()
12881216
const next: Active = {
12891217
...job,
12901218
onPromote: undefined,
@@ -1368,18 +1296,9 @@ export const makeWith = (binder: Binder) =>
13681296
const binding = bindings.get(input.handle)
13691297
if (!binding) return [{}, jobs]
13701298
const ledger = binding.ledger
1371-
// The floor is the earliest announced, still-unfiled detection. Entries at or beyond it
1372-
// are withheld, because a run has detected an earlier position whose filing is still in
1373-
// flight. An announce on an already-filed position is inert here.
1374-
let floor: AnswerLog.Key | undefined
1375-
for (const key of ledger.announced.values()) {
1376-
if (ledger.filed.has(key.position)) continue
1377-
if (floor === undefined || AnswerLog.compare(key, floor) < 0) floor = key
1378-
}
13791299
const observed = AnswerLog.transition(ledger.log, {
13801300
_tag: "Observe",
13811301
after: input.after,
1382-
...(floor !== undefined ? { floor } : {}),
13831302
})
13841303
if (observed._tag === "answer") {
13851304
ledger.log = observed.state

‎packages/core/test/background-job.test.ts‎

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, test } from "bun:test"
22
import { BackgroundJob } from "@opencode-ai/core/background-job"
33
import { LayerNode } from "@opencode-ai/core/effect/layer-node"
4-
import { Deferred, Effect, Exit, Fiber, Scope } from "effect"
4+
import { Deferred, Effect, Exit, Scope } from "effect"
55
import { it } from "./lib/effect"
66

77
const jobsLayer = LayerNode.compile(BackgroundJob.node)
@@ -58,23 +58,6 @@ describe("BackgroundJob.AnswerLog", () => {
5858
expect(second.answer.detected).toBe("m2")
5959
})
6060

61-
test("withholds an entry that an announced, still-unfiled earlier detection precedes", () => {
62-
const state = publish(BackgroundJob.AnswerLog.empty, "m2", 200).state
63-
const withheld = BackgroundJob.AnswerLog.transition(state, {
64-
_tag: "Observe",
65-
after: 0,
66-
floor: { position: "m1", at: 100 },
67-
})
68-
expect(withheld._tag).toBe("miss")
69-
70-
// A floor at or after the entry does not withhold it.
71-
const released = BackgroundJob.AnswerLog.transition(state, {
72-
_tag: "Observe",
73-
after: 0,
74-
floor: { position: "m3", at: 300 },
75-
})
76-
expect(released._tag).toBe("answer")
77-
})
7861
})
7962

8063
describe("BackgroundJob.settleAdmissibility", () => {
@@ -247,39 +230,6 @@ describe("BackgroundJob", () => {
247230
}).pipe(Effect.provide(jobsLayer)),
248231
)
249232

250-
it.live("withholds a later answer from a live observer while an earlier detection is still filing", () =>
251-
Effect.gen(function* () {
252-
const jobs = yield* BackgroundJob.Service
253-
const release = yield* Deferred.make<void>()
254-
// The owner detects the EARLIER position, announces it, then parks before returning - so its
255-
// filing is in flight while the supplement's later answer is already filed.
256-
const started = yield* jobs.startExact({
257-
id: "job_answer_floor",
258-
type: "test",
259-
metadata: { background: true },
260-
run: Effect.gen(function* () {
261-
const announce = yield* BackgroundJob.Announce
262-
yield* announce({ position: "m1", at: 100 })
263-
yield* Deferred.await(release)
264-
return answered("m1", 100, "owner")
265-
}),
266-
})
267-
const handle = started.handle
268-
expect(handle).toBeDefined()
269-
if (!handle) return
270-
271-
// Park an observer first, so the only way it can return the supplement is a failed floor.
272-
const observer = yield* Effect.forkScoped(jobs.waitAnswer({ handle, after: 0 }))
273-
yield* jobs.extend({ id: "job_answer_floor", run: Effect.succeed(answered("m2", 200, "supplement")) })
274-
275-
yield* Deferred.succeed(release, undefined)
276-
const first = yield* Fiber.join(observer)
277-
expect(first.answer?.detected).toBe("owner")
278-
const second = yield* jobs.waitAnswer({ handle, after: 1 })
279-
expect(second.answer?.detected).toBe("supplement")
280-
}).pipe(Effect.provide(jobsLayer)),
281-
)
282-
283233
it.live("carries a run's notice to the terminal when it produced no answer", () =>
284234
Effect.gen(function* () {
285235
const jobs = yield* BackgroundJob.Service

‎packages/opencode/src/background/job.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { BackgroundJobBinder } from "./binder"
88
export {
99
Service,
1010
AnswerLog,
11-
Announce,
1211
type Admission,
1312
type Answer,
1413
type ArmPermit,

0 commit comments

Comments
 (0)