Skip to content

fix(cli): stop replaying completed operation deliveries - #322

Open
wibus-wee wants to merge 56 commits into
mainfrom
fix/operation-completion-delivery-replay
Open

fix(cli): stop replaying completed operation deliveries#322
wibus-wee wants to merge 56 commits into
mainfrom
fix/operation-completion-delivery-replay

Conversation

@wibus-wee

@wibus-wee wibus-wee commented Sep 3, 2026

Copy link
Copy Markdown
Member

Related issue

Same-repository branch; no intake Issue was created per repository policy.

Incident / impact

A finished Operation could leave its completion Delivery pending. Each later reconciliation wake then started the requester session ACP again, including Custom ACP providers, even though the Operation itself had already finished.

Early repairs inferred completion from the stable Assistant entry. That was unsafe in both directions: the entry is created before prompt execution, while teardown can mark an interrupted entry as finished with endedAt.

Root cause

Delivery had no durable success acknowledgement owned by the execution path. Session history represented turn lifecycle, not successful continuation settlement.

The first attempt-token implementation also left two check-then-act races:

  • A coordinator could overwrite an active attempt owned by another coordinator.
  • Configuration resolution is asynchronous. A coordinator could observe no active attempt, later resolve the configuration as unavailable, and write CONFIGURATION_UNAVAILABLE to history after another coordinator had claimed and started execution. The guarded SQLite consume rejected the stale mutation, but the history write had already happened.

A Host lease alone cannot distinguish Worker generations in daemon mode because the supervisor keeps that lease while replacing its child Worker.

Resolution

Delivery mutation now uses three fencing layers:

  • Host: the existing Host lease excludes other daemon runners, Electron Hosts, and foreground Hosts for one installation.
  • Worker: every CLI Worker process has a fresh workerBootId. The daemon supervisor waits for the previous child to exit before spawning its replacement; foreground startup crosses the Host-lease barrier. A new Worker performs orphan recovery once at startup.
  • Claim: execution attempts and no-execution terminal settlement share one exclusive token slot. Every claim has a fresh id and is conditional on both active-token columns being empty. A foreign or lost claim is normal contention.

Execution claims increment attemptCount; terminal claims do not. Configuration-unavailable, attempts-exhausted, and expired-stale paths must claim before any history or consume side effect. A static failure is written while that claim is held, then consumed only when workerBootId + claimId still match.

Acknowledgement and interruption use the same pair. Claim contention exits before Assistant history, failure bookkeeping, or ACP startup. A late acknowledgement, release, or terminal consume from an old Worker cannot alter a current claim.

If a Worker exits while holding either claim kind, replacement-Worker startup clears the old token without resetting attemptCount. If a crash left a static not_started marker before consume, a later valid execution removes that stale marker when it begins.

Unknown execution interruption gets one replacement-Worker retry. If two attempts end without durable settlement, lifecycle-safe reconciliation claims terminal ownership, writes DELIVERY_ATTEMPTS_EXHAUSTED, consumes the Delivery, and never starts a third ACP.

Execution result Delivery behavior
Normal output Durable completed acknowledgement and consume
Durable execution failure or no output Durable failed acknowledgement and consume
Shutdown, cancellation, crash, or missing settlement No acknowledgement; release or recover the orphan
First unknown interruption One retry after the Worker lifecycle barrier
Second unknown interruption Fenced static exhaustion result; no third ACP
Configuration disappears during a competing execution Terminal claim loses; no history mutation
Claim lost to another Worker Silent contention; no Assistant, failure, or ACP
Requester machine/session unavailable Remains pending until runnable

Verification

  • pnpm check on Node 22: PASS
  • pnpm format:check: PASS
  • Focused changed-area suites: 156 tests passed (Coordinator 41, store 25, execution 78, model 12).
  • Existing supervisor suite: 49 tests passed, including the replacement-child exit barrier.
  • Deterministic regressions cover config-sync contention, terminal-claim versus execution exclusion, orphaned terminal claims, stale history repair, two-Worker execution exclusion, stale acknowledgement/release, graceful teardown, hard crashes, steering identity, and permanent stop after two unsettled attempts.

Context handoff

Instructions for reviewing agents

  • Review focus: Verify that every Delivery-side history or consume mutation first owns the exclusive token, orphan recovery occurs only at Worker startup, and all settlement operations match Worker plus claim identity.
  • Decisions to challenge: Confirm config resolution cannot write history after losing a terminal claim, terminal claims do not spend the two-attempt budget, and recovered execution removes stale not_started metadata.
  • Plausible failures / evidence gaps: A crash after external ACP side effects but before local acknowledgement can cause one retry; the two-attempt bound prevents infinite replay.

Authoring context

  • User goal / directives: Stop completed Operation notifications from repeatedly waking a Custom ACP provider without silently dropping interrupted Delivery work or corrupting continuation history.
  • Constraints / non-goals: Preserve offline pending delivery and ordinary user-dispatch ownership. Generic Assistant terminal fields are not success evidence.
  • Risk-bearing decisions: Host, Worker, and claim identity are separate fences. Terminal history and consumption occur under the same claim as execution exclusion.
  • Destructive or irreversible behavior: Exhausted Delivery is consumed only after writing a durable static failure while holding terminal ownership.
  • Deliberately not changed: Remote machine availability and routing policy.
  • Confidence: Crash and contention windows are covered deterministically; no test relies on sleeps or scheduler timing.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T12:33:43.650923Z d8bfd33 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@wibus-wee
wibus-wee requested a review from zxch3n September 3, 2026 02:00
Bind delivery assistant history to the stable completion system turn so reconciliation can consume it across intervening user turns without claiming user dispatch state.

Model: gpt-5
@wibus-wee
wibus-wee force-pushed the fix/operation-completion-delivery-replay branch from 4f6f45d to 26e4737 Compare September 3, 2026 10:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26e4737d3d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/orchestration/operation-coordinator.ts Outdated
wibus-wee and others added 3 commits September 3, 2026 19:46
Persist attempt ownership and settle Deliveries only from execution outcomes so teardown-finalized Assistant entries cannot masquerade as completion. Bound interrupted recovery to one retry.

Model: gpt-5

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66718a55ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/orchestration/operation-coordinator.ts Outdated
wibus-wee and others added 7 commits September 4, 2026 00:09
Keep the owning Assistant turn as an immutable settlement identity while steer advances the visible prompt tail. Add lifecycle coverage for A-to-B-to-C steering.

Model: gpt-5
A GFM literal autolink only ends at ASCII whitespace, so CJK prose written
without one is swallowed into the destination: `**https://.../pull/317**,分支`
rendered as a single link over `...317**,分支`, with the `**` never closing
the strong it opened.

Treat a non-ASCII punctuation mark or separator as the end of a bare URL in
both autolinkers — those never appear unencoded in a URL, while non-ASCII
letters still can (`/wiki/中文`). For GFM autolinks this runs as a pass before
the existing bold repair; every boundary character is Markdown punctuation or
whitespace, so a `**` left text-final by the cut was already a valid strong
closer in the source. That also makes the repair's "suffix must contain inline
markup" guard wrong — the swallowed suffix here is plain text — and the guard's
original case (`**https://example.com/path**segment`) is already covered by the
strong-closer validity check.

Model: claude-opus-5[1m]

Co-authored-by: Zixuan Chen <me@zxch3n.com>
…rvable [risk:medium] (#310)

The iOS review prompt has effectively never fired, and there was no way to tell
why: StoreKit reports neither whether the sheet rendered nor what rating was
chosen, and every gate in front of it is device-local. Reviewing the path found
no wiring bug — the plugin is registered, the bridge is installed, the hook is
mounted, the app version is populated. The gates themselves were the problem.

Simplify the gates. StoreKit already caps the sheet at three presentations per
device per 365 days and silently drops the rest, so a second, stricter rate
limiter in the product layer bought nothing:

- Engagement: 50 completed turns in a rolling 60-day window, replacing 51
  lifetime turns that never decayed — someone who used Lody heavily months ago
  stayed "engaged" forever.
- Cooldown: 90 days, unchanged.
- Negative context: narrowed from "any failure in any session opened in the last
  72h" — very nearly always true for anyone active enough to clear the turn
  threshold — to "this session's last five finalized turns are clean". StoreKit's
  limiter protects the user from spam; it does not protect us from a one-star
  review earned by asking right after a turn failed. Reading the live session
  means the check needs no durable state at all.
- Removed: the 2-of-3 recent-active-days requirement, and the per-version block
  that the cooldown already subsumes.

Shrink the persisted state. v1 stored a lifetime counter, three day keys, and up
to 512 `sessionId:turnId` dedup strings — roughly 25KB JSON-serialized into
localStorage on every completed turn, and unbounded until the threshold was
reached. v2 stores the newest 50 completed-turn timestamps and the last attempt
time. That list answers the threshold exactly (if the oldest of the newest N is
already outside the window, then fewer than N are inside it), and its last
element doubles as the watermark that makes a repeated history scan idempotent.
The watermark also let the per-mount observed-id set go: the recording effect
passes the whole outcomes array and still returns the same state object — so
skips the write — when nothing is new.

Because the newest stored timestamp is the watermark, no stored time may ever be
in the future. Turn times come from the agent machine's clock while `nowMs` is
the phone's, so the two disagree routinely, and a stored future value swallows
every genuinely newer turn until real time catches up to it — one turn dated
2099 would have stopped all counting forever. Future input is rejected, which
defers rather than drops it because the recording effect re-scans the whole
session on the next history update, and stored future times are dropped too, so
a phone clock corrected backwards heals on the next turn instead of going quiet
for the length of the correction.

Instrument both ends of the funnel, since none of the above is observable from
outside the device:

- `mobile/app_store_review_prompt_requested` fires immediately before the bridge
  call, the furthest point we can observe.
- `mobile/app_store_review_prompt_blocked` names the first gate a candidate turn
  died on. `hasAppStoreReviewEligibility` delegates to
  `resolveAppStoreReviewBlockReason` so the reported reason and the decision come
  from one implementation, and the runtime gates outside the policy (missing
  bridge, text entry, interaction cancel, hidden app) report through the same
  event. Deduplicated per user AND per reason for the app process's lifetime: a
  candidate turn arrives on every completed turn, so an undeduplicated event
  would be among the noisiest in the product, while deduplicating on the user
  alone would let whichever gate trips first mask the rest.

Migration: v1's total cannot be reprojected onto per-turn times, so v2 uses a new
key prefix, starts fresh, and deletes the v1 blob on first read.

Model: claude-opus-5[1m]

Co-authored-by: Zixuan Chen <me@zxch3n.com>
A child session tab had no unread indicator. Sub-sessions get no sidebar
row of their own, so the tab is the only place their new output can
surface — a subagent that finished while the user read the parent thread
stayed silent until they happened to click it.

The tab's leading icon slot becomes ONE priority-ordered status slot,
`waiting > working > unread > agent icon`, the same ladder the desktop
sidebar row and the mobile tab sheet already use. Unread reads through
the new `sessionHasUnreadMessages`, which shares its comparison with
`shouldMarkSessionRead` so announcing unread cannot drift from deciding
the read receipt, and is suppressed on the ACTIVE tab — that surface is
the one clearing unread, so a dot there would only flash between the
click and the receipt landing.

That reordering also revives a dead branch: `isWaiting` was tested after
`isWorking`, but a permission request is itself live presence, so a tab
blocked on approval always rendered the busy spinner and its marker
never showed. Waiting now renders the sidebar's `Hand` rather than the
old amber dot, because `--primary` and `--status-warning` are both amber
in the shipped themes and two dots side by side read as one marker.

Adds `tests/session-tab-bar-status-slot.test.tsx` over the whole ladder
and a `Sessions/SessionTabBar` story rendering it in one row.

Model: claude-opus-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Model: gpt-5.4

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
Comment thread apps/cli/src/orchestration/operation-store.ts Outdated
lodystage Bot and others added 11 commits September 4, 2026 03:51
Model: gpt-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
Model: gpt-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
* fix(components): preload billing overview

Warm the existing session-scoped billing cache when authenticated user and workspace resolution completes, without changing live permission or quota data paths.

Model: gpt-5.6-sol

* test(components): remove billing preload test

Keep the latency-only preload change small and rely on the existing billing cache coverage.

Model: gpt-5.6-sol
Make Worker boot identity and attempt identity jointly fence Delivery settlement and interruption. Recover old-boot orphans only after Worker startup, and stop permanently after two unsettled attempts.

Model: gpt-5
Acquire an exclusive Delivery finalization claim before writing configuration or exhaustion results. Token-match terminal consumption and repair stale non-started history when recovered execution begins.

Model: gpt-5
…| Lody (#379)

* fix(site-docs): brand docs and marketing head titles with | Lody

Append the Lody brand to docs, blog, changelog, and pricing document
titles without double-branding landing pages that already include it.

Model: cursor-grok-4.6

Co-authored-by: Zixuan Chen <zx@loro.dev>

* test(site-docs): await node:test registrations for oxlint

Model: cursor-grok-4.6

Co-authored-by: Zixuan Chen <zx@loro.dev>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* fix: derive DeepSeek ACP options from runtime

Model: gpt-5

* refactor: remove redundant DeepSeek ACP work

Model: gpt-5

---------

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
Model: gpt-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
Add a dedicated docs pillar for inviting a teammate into an existing
coding agent session to continue, with a comparison against transcript
share/export tools and a short cross-link from Team Features.

Model: Cursor Grok 4.6

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
#388)

Streamdown's bundled Mermaid overlay put its only exit — a 32px button — at a
raw `top-4 right-4`, which on a phone lands entirely inside the status-bar
inset, where the system takes the touch. Its content layer then covered the
whole backdrop and stopped propagation, so a backdrop tap never reached the
close handler either, and touch surfaces have no Escape. The overlay could be
opened and not left. It also rendered the diagram scaled to fit, which turns an
agent's sequence diagram into an unreadable grey texture.

Turn that overlay off and mount `MermaidDiagramViewer` instead: controls padded
by the `--safe-area-*` variables at 44px, three exits (close button, a click off
the diagram, Escape), and `--z-image-viewer` so a diagram opened from a message
inside a dialog lands above it. A diagram that does not fit opens at natural
size and pans, with explicit zoom controls.

Streamdown owns the diagram markup, so the click target and its role/tabindex
are applied by observer, and the block's own copy/download controls now reveal
themselves on `(hover: none)` — the same rule the code block already carried,
without which they were invisible on touch for good.

Model: claude-opus-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Model: gpt-5

Co-authored-by: Zixuan Chen <me@zxch3n.com>
wibus-wee and others added 17 commits September 4, 2026 23:50
Keep worker fencing, exclusive claims, bounded attempts, and handled versus interrupted settlement while removing redundant acknowledgement metadata. Backfill legacy pending deliveries as one unknown prior attempt.

Model: gpt-5
* fix(components): render TeX bracket delimiters

Model: GPT-5

* fix(components): preserve nested fenced math

Model: GPT-5

* fix(components): preserve indented TeX code

Model: GPT-5

---------

Co-authored-by: Zixuan Chen <me@zxch3n.com>
Advance acp-extension-codex to the latest commit on its main branch.

Model: GPT-5

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
Model: GPT-5

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
* fix: enable Codex Astra extended reasoning options

Align extended reasoning with the ACP model catalog, including Max-only support for Luna.

Model: gpt-6-astra

* fix: align Codex lockfile specifier with submodule

Match the submodule's ^0.153.4 requirement so frozen installs can run in both CI jobs. The resolved package remains 0.153.4.

Model: gpt-6-astra

---------

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
* feat: display Claude Fable weekly quota

Model: gpt-6

* chore: pin merged Fable quota submodules

Model: gpt-6

---------

Co-authored-by: Leeeon233 <leeeon233@gmail.com>
Bind delivery assistant history to the stable completion system turn so reconciliation can consume it across intervening user turns without claiming user dispatch state.

Model: gpt-5
Persist attempt ownership and settle Deliveries only from execution outcomes so teardown-finalized Assistant entries cannot masquerade as completion. Bound interrupted recovery to one retry.

Model: gpt-5
Keep the owning Assistant turn as an immutable settlement identity while steer advances the visible prompt tail. Add lifecycle coverage for A-to-B-to-C steering.

Model: gpt-5
Make Worker boot identity and attempt identity jointly fence Delivery settlement and interruption. Recover old-boot orphans only after Worker startup, and stop permanently after two unsettled attempts.

Model: gpt-5
Acquire an exclusive Delivery finalization claim before writing configuration or exhaustion results. Token-match terminal consumption and repair stale non-started history when recovered execution begins.

Model: gpt-5
Keep worker fencing, exclusive claims, bounded attempts, and handled versus interrupted settlement while removing redundant acknowledgement metadata. Backfill legacy pending deliveries as one unknown prior attempt.

Model: gpt-5
Keep execution claims in a separate table so stable binaries can read Delivery rows. Count attempts only after the completion history write succeeds, and cover pre-start failures in coordinator and model tests.

Model: gpt-6
The remote branch contains patch-equivalent copies of the six delivery commits already present locally. Preserve the rebased source tree and the new compatibility fix while retaining remote ancestry for a fast-forward push.

Model: gpt-6

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6754d562cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/orchestration/operation-coordinator.ts
Track claimed, prepared, started, and uncertain Delivery phases so only confirmed pre-provider work is retryable. Settle user cancellation and accepted steer explicitly, retry known settlement writes without invoking ACP, and migrate legacy pending rows conservatively.

Add deterministic store, coordinator, model, and execution-service coverage for crash, restart, cancellation, settlement failure, and compatibility boundaries.

Model: gpt-5
Abandon only owner-matched claims before the coordinator store closes. Prepared work becomes runnable, while provider-started work becomes uncertain and is never replayed automatically. Fence async claim and terminal-finalization paths after stop.

Model: gpt-5
Keep terminal history and consume writes under the same cleanup boundary. A failed settlement releases its fenced claim so a later wake can retry idempotent finalization without invoking ACP or spinning immediately.

Model: gpt-5

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fa076d770

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/orchestration/operation-coordinator.ts Outdated
Keep claim-bound turn outcomes in the live coordinator when settlement persistence fails so later wakes retry only the store transition without replaying ACP.

Model: gpt-5
Retain claim-bound terminal history and consume steps across wakes without replaying ACP or relying on cleanup writes. Recheck ownership after asynchronous history persistence and cover repeated write outages and claim replacement.

Model: gpt-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants