luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

docs(adr): write prompt-async-gate ADR

Browse Source 
Documents the reservation-based duplicate-injection guard introduced in
v4.2.0. Covers context (Issue #4012 race window), decision (Symbol token,
post-dispatch hold, dispatch timeout, shared runner, prefix-tightened
release), consequences (caller-side release discipline, AST audit
strengthens enforcement), and references.

Closes M11
This commit is contained in:
YeonGyu-Kim
2026-05-16 01:29:56 +09:00
parent 38732b426e
commit 0c27ecb17d
1 changed files with 147 additions and 268 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/reference/prompt-async-gate-rfc.md
+147 -268
View File
@@ -1,332 +1,211 @@
# ADR: Prompt Async Gate # ADR: prompt-async-gate - reservation-based duplicate-injection guard
## Status ## Status
Accepted for v4.2.0. Proposed (introduced in v4.2.0)
This decision applies to every production route that sends an internal message
through an OpenCode session API.
The mandated implementation is `src/shared/prompt-async-gate.ts`.
The root `AGENTS.md` invariant named "Internal message injection is dangerous"
is the policy authority for this ADR.
The static audit `src/shared/prompt-async-route-audit.test.ts` enforces the
production side of this decision.
Route-specific tests must still prove behavior for each internal message path.
## Context ## Context
Issue 4012 reported duplicate streaming output after internal message injection. Issue #4012 reported duplicate streaming output after OMO injected an
internal message into a live OpenCode session.
The visible symptom was repeated assistant output in a live parent session. The user-visible failure was two assistant bubbles streaming the same
continuation.
The underlying failure mode was a race between OpenCode session state and OMO The root race was not one hook making one bad decision. Multiple internal
continuation hooks. routes could observe the same idle, completion, or error edge and each decide
that the parent session needed a wake or recovery prompt.
OMO has several routes that can decide to wake or continue a session: The most important race window was:
- background task completion notifications 1. OpenCode emitted a `session.idle` event.
2. OMO started an `isSessionActive` HTTP poll.
3. OpenCode was still pacing the streaming animation for the previous answer.
4. The poll observed an inactive or idle-looking session.
5. OMO injected a continuation prompt.
6. A second hook observed the same edge and injected again.
7. The user saw two assistant bubbles.
The historical race site was visible in the built bundle at
`dist/index.js:69665-69680`. That code checked session activity before sending
an internal prompt, but the check and the prompt were not protected by a
shared reservation.
OpenCode's `prompt_async` route contributed to the failure mode because it has
fire-and-forget semantics. `session.promptAsync` can resolve before the prompt
is durably accepted by the target session. A later `session.error` event can
still arrive for the same attempt, so the caller can believe dispatch finished
while a recovery hook still treats the session as eligible for retry.
OMO has 13+ internal hook callers that can inject prompts, including:
- background task parent wakes
- runtime fallback retries - runtime fallback retries
- team mailbox delivery - model suggestion retries
- recovery continuations - team mailbox live delivery
- CLI run resume paths - session recovery continuations
- Claude Code hook delivery - todo continuation resumes
- sync and background subagent prompts - CLI run resumes
- Claude Code hook injections
- sync subagent prompts
- background subagent prompts
These routes can observe the same idle, completion, or error edge. Route-local guards cannot close this race. Each route can be correct in
isolation and still collide with another route in the same process.
Without a shared gate, two routes can dispatch the same internal prompt into the The root `AGENTS.md` now records the governing invariant in the section
same parent session. "Internal message injection is dangerous": production code may call
`session.prompt` or `session.promptAsync` only inside
OpenCode also exposes a subtle durability gap. `src/shared/prompt-async-gate.ts`. Every other route must use the shared gate.
`session.promptAsync` can return before the prompt is durably accepted by the
target session.
A later `session.error` event can still arrive for the same attempt.
That means a route can think it finished while another hook still sees the
session as eligible for recovery.
The old pattern was unsafe:
```ts
await client.session.promptAsync({
path: { id: sessionID },
body: { text: message },
})
```
The unsafe properties were:
1. No per-session reservation before dispatch.
2. No shared active-session check.
3. No post-dispatch hold for late failures.
4. No timeout around a hung dispatch.
5. No central log trail for skipped or failed dispatches.
6. No static audit that could block new raw prompt routes.
Local guards inside each feature were not enough.
Different hooks can run in the same process and see different snapshots of
session state.
They need one shared reservation map keyed by session ID.
The root `AGENTS.md` now states the invariant:
```text
Treat every session.prompt / session.promptAsync call as a write to shared
session state. Production code may call them only inside
src/shared/prompt-async-gate.ts.
```
This ADR records the architecture behind that invariant.
## Decision ## Decision
All production internal message injection must go through Create `src/shared/prompt-async-gate.ts` as the single production owner of raw
`src/shared/prompt-async-gate.ts`. OpenCode prompt dispatch.
The module exports two gate functions: The gate coordinates callers with a module-global reservation map:
```ts ```ts
export async function promptAsyncAfterSessionIdle<TInput>(args: { const reservations = new Map<string, Reservation>()
client: PromptAsyncClient<TInput>
sessionID: string
input: TInput
source: string
settleMs?: number
postDispatchHoldMs?: number
dispatchTimeoutMs?: number
checkStatus?: boolean
}): Promise<PromptAsyncGateResult>
export async function promptAfterSessionIdle<TInput>(args: {
client: PromptClient<TInput>
sessionID: string
input: TInput
source: string
settleMs?: number
postDispatchHoldMs?: number
dispatchTimeoutMs?: number
checkStatus?: boolean
}): Promise<PromptAsyncGateResult>
``` ```
The gate returns a discriminated result instead of throwing for expected races: The map is keyed by `sessionID`. A reservation records the source that claimed
the session, an expiration time, and a `Symbol(source)` token. The token gives
each reservation identity beyond its text source.
```ts Every caller supplies a stable `source` string such as:
export type PromptAsyncGateResult =
| { status: "dispatched"; response: unknown }
| { status: "active" }
| { status: "reserved"; reservedBy: string }
| { status: "unavailable" }
| { status: "failed"; error: unknown }
```
Callers must treat `active` and `reserved` as successful suppression.
They mean another actor owns the session or the user is already active.
They are not retry signals by default.
Every call must provide a stable `source` string.
The source identifies the route that reserved the session.
Recommended source format:
```ts ```ts
const source = `background-agent:${taskID}` const source = `background-agent:${taskID}`
``` ```
The reservation flow is: The shared flow is:
1. Prune expired reservations. 1. Prune expired reservations.
2. Reject if the session already has an active reservation. 2. Reserve the session before waiting or dispatching.
3. Reserve the session before waiting or dispatching. 3. Wait for the idle settle period.
4. Wait for idle settle time. 4. Poll session activity unless the route has a proven opt-out.
5. Check current session status unless the caller opted out for a proven reason. 5. Dispatch through the selected OpenCode prompt API.
6. Dispatch through `session.promptAsync` or `session.prompt`. 6. Keep the reservation during the post-dispatch hold.
7. Keep a short post-dispatch hold after an attempted dispatch. 7. Release after the hold or through an explicit recovery path.
8. Release only after the hold expires or through an intentional recovery path.
The default timing constants are part of the decision: The reservation is taken before the activity poll so that two hooks cannot both
enter the poll-dispatch window.
The default post-dispatch hold is exported as:
```ts ```ts
export const DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS = 250 export const DEFAULT_PROMPT_ASYNC_POST_DISPATCH_HOLD_MS = 250
```
`postDispatchHoldMs` defaults to 250 ms. The gate holds the reservation briefly
after the dispatch attempt even when dispatch throws synchronously or returns a
failed result. This closes the AGENTS.md hazard where `promptAsync` returns
before durable acceptance and a late OpenCode error races with retry logic.
The default dispatch timeout is 30 seconds:
```ts
export const DEFAULT_PROMPT_DISPATCH_TIMEOUT_MS = 30_000 export const DEFAULT_PROMPT_DISPATCH_TIMEOUT_MS = 30_000
``` ```
The post-dispatch hold is required because `promptAsync` returning does not prove `dispatchTimeoutMs` wraps the underlying `session.promptAsync` or
that all related OpenCode events have drained. `session.prompt` call with `Promise.race`. A hung OpenCode API call must fail
closed instead of holding a reservation forever.
The dispatch timeout is required because a stuck OpenCode API call must not hold Both public gate helpers delegate to one internal runner:
the reservation forever.
The timeout is a circuit breaker, not a synchronization primitive.
Callers must not set `postDispatchHoldMs: 0`.
The static audit rejects that pattern.
If a caller needs custom behavior, it must add a route-specific regression test
that proves duplicate dispatch cannot occur.
The gate owns the raw prompt calls:
```ts ```ts
const promptAsync = client.session?.promptAsync dispatchAfterSessionIdle<TInput>(args)
if (typeof promptAsync !== "function") {
return { status: "unavailable" }
}
return dispatchAfterSessionIdle({
sessionName: "promptAsync",
client,
sessionID,
input,
source,
settleMs,
postDispatchHoldMs,
dispatchTimeoutMs,
checkStatus: args.checkStatus !== false,
dispatch: (dispatchInput) => promptAsync(dispatchInput),
})
``` ```
Production code outside this module must not access these APIs directly: `promptAsyncAfterSessionIdle` passes a `session.promptAsync` dispatcher.
`promptAfterSessionIdle` passes a `session.prompt` dispatcher. Sharing the
runner keeps reservation, hold, timeout, logging, and active-session behavior
identical for async and sync prompt routes.
The public gate result is a discriminated union. Callers must treat `active`
and `reserved` as successful suppression, not automatic retry signals. A route
that changed optimistic task or loop state before dispatch owns restoring that
state when the gate returns `failed`, `unavailable`, or a skipped status that
requires rollback.
The gate exposes `releasePromptAsyncReservation` for intentional recovery
paths. Prefix release is deliberately tight:
```ts ```ts
client.session.prompt(...)
client.session.promptAsync(...)
client["session"]["promptAsync"](...)
const { promptAsync } = client.session
Reflect.apply(client.session.promptAsync, client.session, [input])
```
Type guards may check that `promptAsync` exists when the eventual dispatch still
routes through the shared gate.
The allowlist in the audit must stay small and justified.
The gate also exposes reservation release helpers for intentional recovery:
```ts
releasePromptAsyncReservation(sessionID, {
reservedBy: "model-suggestion-retry",
})
releasePromptAsyncReservation(sessionID, { releasePromptAsyncReservation(sessionID, {
reservedByPrefix: "runtime-fallback:", reservedByPrefix: "runtime-fallback:",
}) })
``` ```
Prefix release is allowed only for prefixes that end with `:`. `reservedByPrefix` must end in `:`. This prevents broad releases such as
`runtime` matching unrelated sources. Exact source release remains available
for callers that know the full reservation source.
This prevents broad accidental releases such as `runtime` matching unrelated Raw prompt calls outside the gate are blocked by
sources. `src/shared/prompt-async-route-audit.test.ts`. The audit uses the TypeScript
Compiler API rather than regex so it catches destructuring, bracket access,
Release helpers exist for rollback and retry flows. optional chaining, and aliased or cast access patterns.
They must not be used as a normal cleanup path after dispatch.
## Consequences ## Consequences
Positive consequences: ### Positive
- Duplicate internal dispatches collapse to one reservation winner. - Duplicate internal prompt injection now has one reservation winner per
- Late `session.error` events no longer trigger immediate duplicate retries. session.
- Internal message routes share logging and result semantics. - The post-dispatch hold closes the AGENTS.md "returns before durably
- Tests can reason about a single gate instead of many ad hoc guards. accepted" hazard even when dispatch errors synchronously.
- New raw prompt routes are blocked by a static audit. - Dispatch timeout prevents a stuck OpenCode call from holding the gate forever.
- Retry flows can release only their own reservation source. - 13+ internal hook callers share one result model and one safety primitive.
- Hung dispatches fail closed through a timeout. - The AST-based audit from HIGH-5 catches more bypass shapes than the prior
regex audit.
- Route-specific tests can focus on route behavior while the shared gate tests
reservation semantics.
Negative consequences: ### Negative
- Internal prompt injection has a small default latency from idle settling. - Caller-side retry logic that releases and retries must call
- A post-dispatch hold can delay a legitimate retry by 250 ms. `releasePromptAsyncReservation` explicitly when the original prompt did not
- Callers must handle `PromptAsyncGateResult` instead of assuming dispatch. durably reach the server. `src/shared/model-suggestion-retry.ts` is the
- Tests that mock session APIs may need to model reservation state. reference case.
- Any new route must add route-specific duplicate-injection coverage. - 13+ wiring sites each need to be conscious of the gate result. Treating
`reserved` as a failure can create noisy retries.
- A valid retry can be delayed by the default 250 ms post-dispatch hold.
- The reservation map is process-local. It protects OMO hooks in the current
plugin process, not every possible OpenCode process.
Operational consequences: ### Migration
- CI green is not enough for race fixes tied to issue 4012. Existing `session.prompt` and `session.promptAsync` callers must route through
- Maintainers must re-run the documented reproducer against the fix commit. `promptAfterSessionIdle` or `promptAsyncAfterSessionIdle`.
- Logs containing `[prompt-async-gate]` are the first place to inspect when a
wake, retry, or recovery message does not appear.
Testing consequences: The AST-based audit fails CI if a raw prompt call is added without an allowlist
entry. Any allowlist entry must explain why the raw access is not a dispatch
route or why it is still gate-routed.
- `src/shared/prompt-async-gate.test.ts` covers gate behavior. New internal message routes must include duplicate-injection regression tests
- `src/shared/prompt-async-route-audit.test.ts` blocks raw production prompt for their trigger. Static policy alone is not enough.
routes.
- Route owners must add regression tests for the specific trigger they wire.
- Tests must not rely on sleeping to wait for the post-dispatch hold.
Design constraints that remain open: ### Future work
- The reservation map is process-local. - Replace prefix-tightened release with full Symbol-token-based release
- Cross-process OpenCode sessions still rely on the session API and event stream. ownership. This is the HIGH-7 deferred work.
- The gate does not deduplicate different semantic prompts for the same session. - Define same-source concurrent caller handling. Some routes may need collapse
- The gate prevents concurrent injection, not incorrect caller intent. semantics by source rather than by session only.
- Add dispatch metrics for observability, including reservation win, reserved
Rejected alternatives: skip, active skip, timeout, and failed dispatch counts.
- Consider cross-process coordination if OpenCode exposes a durable session
1. Keep route-local guards. lock or idempotency key.
This failed because hooks observe the same edge from different modules.
2. Disable recovery on any recent prompt event.
This would hide valid recovery paths and lose task state.
3. Use a global fixed delay after every dispatch.
A delay without a reservation does not prevent another route from entering.
4. Treat `promptAsync` success as durable acceptance.
Issue 4012 showed that later OpenCode errors can still arrive.
5. Allow raw prompt calls with code review discipline.
The risk is architectural, so the invariant needs an automated audit.
Migration rule:
```ts
const result = await promptAsyncAfterSessionIdle({
client,
sessionID,
input,
source: "runtime-fallback:retry",
})
if (result.status === "failed") {
restoreOptimisticState()
}
```
The caller owns any optimistic task or loop state it changed before dispatch.
If dispatch is skipped, unavailable, or failed, the caller must restore state
when needed.
## References ## References
- Issue 4012: https://github.com/code-yeongyu/oh-my-openagent/issues/4012 - Issue #4012: duplicate streaming output and two assistant bubbles.
- Introduction PR 4034: https://github.com/code-yeongyu/oh-my-openagent/pull/4034 - PR #4034: introduction of `prompt-async-gate`.
- Hardening commit: `b333a5280` `fix(prompt-async-gate): add dispatch timeout, shared runner, harden prefix release` - PR #3866 -> PR #4053: schema-compatible synthetic tool results for
- Test commit: `f93d7297c` `test(prompt-async-gate): cover dispatch timeout and post-dispatch error hold` post-compaction recovery, related to safe recovery dispatch.
- Retry release commit: `ff1b15d53` `fix(model-suggestion-retry): release reservation before retry attempt` - Root `AGENTS.md`: section "Internal message injection is dangerous".
- Root invariant: `AGENTS.md`, section `Internal message injection is dangerous` - `.sisyphus/rules/test-discipline.md`: forbids `setTimeout(resolve, N)` and
- Implementation: `src/shared/prompt-async-gate.ts` `await sleep(N)` in tests unless time itself is the system under test.
- Static audit: `src/shared/prompt-async-route-audit.test.ts` - Implementation: `src/shared/prompt-async-gate.ts`.
- Audit: `src/shared/prompt-async-route-audit.test.ts`.