fix(core): serialize prompt settlement by lilong555 · Pull Request #62167 · anomalyco/opencode

lilong555 · 2026-11-05T18:47:13Z

Issue for this PR

Type of change

Bug fix
New feature
Refactor / code improvement
Documentation

What does this PR do?

Claim each Form or Permission request before publishing its settlement event. Concurrent replies, cancels and permission cascades fail or skip immediately, including inline listener re-entry. always saves approvals before publishing; failed storage keeps requests retryable while the service is open.

The owner retains cleanup responsibility when close() skips an active reply. Shutdown-driven cancellation makes one publication attempt, including when a form is created after shutdown.

Follow-up to #52550. Thanks @535628443 (#52550) and @sahiljagtap08 (#44340) for the per-request settlement approach. No feature flag is needed for this bug fix.

How did you verify your code works?

Current head e3c7b3478ad1d3449fa9375cb77660ab8caca927, based on dev 36f559b0c6cb61ebd3431d69d287809bf66acbc5, using Bun 1.4.4 on WSL2/Linux:

Root bun run check passed: lint and 35/35 typecheck tasks, with no task-cache hits on the first run.
From packages/core, bun run test test/form.test.ts test/permission.test.ts: 156 passed, zero failures.
The normal pre-push checks passed.

The regressions cover concurrent settlements, inline listener re-entry, close/reply races, storage failures, and cleanup after a failed close. Both original failure paths were reproduced before the fix.

Earlier hosted coverage at 78857fca51b85544811616edd82c830e2de22118 remains available in the Ubuntu and Windows validation run: all 33 workspace unit tasks passed on each platform; Form/Permission repetition passed 1,560 cases per platform. The artifact manifest records those source revisions and integrity checks. Upstream workflow authorization and review remain pending.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72">
  <title>A prompt settles once</title>
  <style>.flow{animation:flow 2s linear infinite}@keyframes flow{to{stroke-dashoffset:-16}}@media(prefers-reduced-motion:reduce){.flow{animation:none}}</style>
  <g fill="none" stroke="currentColor" stroke-width="1.5">
    <path d="M16 22H38L56 36M16 50H38L56 36" opacity=".4"/>
    <rect x="56" y="18" width="68" height="36" rx="4"/>
    <path class="flow" d="M124 36H157" stroke-dasharray="4 4"/>
    <path d="m151 31 6 5-6 5"/>
    <rect x="164" y="18" width="68" height="36" rx="4"/>
  </g>
  <g fill="currentColor" font-family="sans-serif" font-size="12" text-anchor="middle"><text x="90" y="40">Pending</text><text x="198" y="40">Settled</text></g>
</svg>

Screenshots / recordings

Core lifecycle fix; UI components are unchanged.

Checklist

I have tested my changes locally
I have not included unrelated changes in this PR

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

lilong555 · 2026-11-05T18:50:26Z

The check, check-test, and check-nix workflows are waiting for fork-run approval. Could a maintainer authorize them when convenient? The PR includes the exact local regression commands and their results. Thank you.

opencodejr

Looks correct. I ran the reproduction script from #34853 on both checkouts. On the base, two form replies, a reply plus a cancel, and permission once plus reject all succeed and send conflicting events. With this change, the second caller gets Form.AlreadySettledError / Permission.NotFoundError and only one event goes out. form.test.ts and permission.test.ts pass (160 tests) and tsc is clean. The claim on each request is taken before anything yields, so replies from inline listeners, the reject/always cascades and close() all see it.

Public API: unchanged (same events, error types and service methods; a losing reply just gets the existing error).

Two small failure-path problems are noted inline: the publish/save order for "always", and a duplicate cancel when close()'s own publish fails.

The tests are much larger than the fix needs, about 600 lines for 168 lines of source. The release-signal harness, the polling checks for immediate failure and the scheduler-budget loop (3/8/32) make them hard to follow. Please cut them down to the core cases: two replies, reply vs cancel, a permission reply race, an inline listener replying again, a retry after a failed publish, and close skipping an in-flight reply. Please also drop the unrelated Prettier reflow of the shell-scanner grants data in permission.test.ts.

lilong555 · 2026-11-05T19:59:06Z

Thank you for reproducing the original races and for the detailed review. 78857fc addresses both failure paths.

The tests now use direct races and inline callbacks. I removed the scheduler-budget loop and shared release-signal harness, consolidated overlapping cases, and restored the original grants formatting. The changes remove 156 lines overall while retaining failure/retry and shutdown coverage.

Validation: 1,560 focused tests passed; the committed tree also passed 250 related tests and 6 MCP elicitation tests. Both core typecheck commands and focused lint passed. The PR body records the full-workspace check's local dependency limitation.

opencodejr

Both earlier points are resolved. permission.ts now saves an "always" approval before publishing permission.replied, so a failed save no longer leaves an extra event on retry. In form.ts, cancels started by close() (and by create after close) make one publish attempt and are not retried in cleanup, while a reply that close skipped is still cancelled by its owner if its publish fails. The tests are trimmed: the scheduler-budget loop is gone and the unrelated shell-scanner reflow is reverted.

I re-ran the #34853 reproduction at this commit: all three cases now settle once, and the loser gets AlreadySettledError / NotFoundError. form.test.ts and permission.test.ts pass (156) and tsc is clean.

Public API: unchanged.

No further concerns.

lilong555 · 2026-11-06T03:24:56Z

Supplemental hosted-runner validation for 78857fca51b85544811616edd82c830e2de22118 is complete:

Workspace checks: both the baseline and candidate pass bun run check and bun run typecheck:tsc on Ubuntu.
Prepared-environment unit run: the candidate passes all 33 workspace unit tasks on both Ubuntu and Windows, with zero task-cache hits. Core reports 7,003 pass / 337 skip / 0 fail on Linux and 6,231 pass / 1,110 skip / 0 fail on Windows.
Form/Permission tests pass 1,560 executions per platform with ten repetitions and seed 34853. The earlier Windows contender assertion passes 20 targeted repetitions on each revision; the two Linux TUI case families pass 70 executions on each revision.

The initial unit failures are retained in the first run. Missing ripgrep caused the same search-test failures on the baseline; the follow-up prepares the repository-pinned ripgrep 15.1.0 before testing and uses --continue=always to collect every package's outcome. The follow-up Windows baseline retains two timeouts in the existing artifact and Git-object tests; all candidate tasks complete successfully.

The PR source is unchanged, and all source-integrity checks pass. Full results and the verified artifact manifest include all attempts and runner versions. This is supplemental validation of fixed commits while upstream workflow authorization and review remain pending.

Signed-off-by: liyinlong <[email protected]>

opencodejr

The new commit only merges current dev into the branch. The Form/Permission changes are identical to the commit I last reviewed, and the earlier points remain resolved. On the merged head the #34853 reproduction still settles once in all three cases, form.test.ts and permission.test.ts pass (156), and tsc is clean.

Public API: unchanged.

No new concerns.

fix(core): serialize prompt settlement

9a5222f

Copilot AI balanced review requested due to automatic review settings November 5, 2026 18:47

Copilot AI reviewed Nov 5, 2026

lilong555 mentioned this pull request Nov 5, 2026

fix(core): serialize V2 prompt settlement around event publication #34853

Open

opencodejr Bot reviewed Nov 5, 2026

View reviewed changes

Comment thread packages/core/src/permission.ts Resolved

Comment thread packages/core/src/form.ts Resolved

fix(core): preserve settlement failure ordering

78857fc

opencodejr Bot reviewed Nov 5, 2026

View reviewed changes

chore(core): merge current dev into settlement release

e3c7b34

Signed-off-by: liyinlong <[email protected]>

opencodejr Bot reviewed Nov 11, 2026

View reviewed changes

github-actions Bot mentioned this pull request Nov 11, 2026

📊 AI CLI 工具社区动态日报 2026-11-11 96loveslife/big_model_radar#776

Open

Navigation Menu

Repository navigation

fix(core): serialize prompt settlement - #62167

fix(core): serialize prompt settlement#62167
lilong555 wants to merge 3 commits into
anomalyco:devfrom
lilong555:settlement-release

lilong555 commented Nov 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

lilong555 commented Nov 5, 2026

Uh oh!

opencodejr Bot left a comment

Uh oh!

Uh oh!

Uh oh!

lilong555 commented Nov 5, 2026

Uh oh!

opencodejr Bot left a comment

Uh oh!

lilong555 commented Nov 6, 2026

Uh oh!

opencodejr Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lilong555 commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue for this PR

Type of change

What does this PR do?

How did you verify your code works?

Visual

Screenshots / recordings

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

lilong555 commented Nov 5, 2026

Uh oh!

opencodejr Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lilong555 commented Nov 5, 2026

Uh oh!

opencodejr Bot left a comment

Choose a reason for hiding this comment

Uh oh!

lilong555 commented Nov 6, 2026

Uh oh!

opencodejr Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lilong555 commented Nov 5, 2026 •

edited

Loading