Skip to content

feat: add Pi runtime provider via external CLI RPC - #2131

Merged
yuezengwu merged 39 commits into
mainfrom
feat/pi-runtime-provider
Aug 4, 2026
Merged

feat: add Pi runtime provider via external CLI RPC#2131
yuezengwu merged 39 commits into
mainfrom
feat/pi-runtime-provider

Conversation

@yuezengwu

@yuezengwu yuezengwu commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add pi as a first-class runtime provider across Shared, Client, Server, Web, CLI, and the committed QA case library.
  • Drive the official external Pi CLI through LF-JSONL RPC with one supervised, long-lived process per chat while keeping authentication and configuration provider-owned.
  • Support native read, bash, edit, write, grep, find, and ls tools, Context Tree attribution, managed Skills/config refresh, model assertion, live same-turn steering, finite shared retry policy, and fail-closed protocol/custody boundaries.
  • Preserve accepted-message custody through the provider settlement boundary: active-session follow-ups enter as steer, ACK only after agent_settled, and do not replay unsafe work after suspend, shutdown, reconnect, Reset, or notice-delivery recovery.

V1 boundaries

  • macOS and Linux are supported. Windows fails closed before provider spawn until the accepted Job Object supervisor exists.
  • Non-empty First Tree-managed MCP configuration is rejected before prompt entry.
  • Authentication remains provider-owned; First Tree gives actionable recovery guidance (pi, then /login) but does not take credential custody or add in-product Pi login.
  • Empty model and reasoning-effort selections inherit Pi's persisted/local defaults.
  • Pi owns helper/catalog acquisition by default; First Tree disables provider update checks and telemetry while preserving an explicit operator PI_OFFLINE override.

Main integration and review repairs

  • Ordinary-merged main through exact 11a29015af589468195323a49b129413b4a65c71, preserving current chat-session Reset, SCM audience/echo routing, replay fencing, pending teardown, and terminate-join behavior alongside Pi custody.
  • Reset rotates a durable opaque per-chat fresh-start nonce outside the active resume mapping. Mapping removal and the nonce are flushed in one authoritative registry snapshot before apply-ACK; a failed flush retains the pending nonce and keeps Reset retryable. Pi includes the nonce in new-session identity, so even the same durable inbox row cannot reconstruct a retired provider session after Client restart.
  • In-flight termination and failed Reset persistence form one provider-route admission fence. Intervening deliveries cannot start, resume, inject, retry, ACK, or consume the pending nonce; they are parked without requesting recovery, so repeated delivery cannot open the Server no-progress circuit.
  • A truthful applied:true arms a generation-scoped post-apply fence. Every Server terminal branch then produces one exact, receipted disposition: durable eviction sends session:command:finalized; reactivation, route refusal, or rolled-back cleanup sends session:command:aborted before its HTTP error. Either accepted disposition releases only that generation and may request one coalesced same-socket recovery; abort never restores the retired provider mapping or history.
  • Reset uses the composite two-sided wsSessionResetV1 protocol. Destructive terminate delivery and apply settlement revalidate the current route, instance, and V1 capability. Post-apply finalized and aborted delivery/receipt are identity-scoped to the original applying Client so an already-applied generation can converge after route or capability metadata changes. Stale, foreign, superseded, duplicate, and joined alias refs cannot release another generation.
  • SessionRegistry.load() retains its existing Map-returning API; SessionManager uses the additive snapshot API when it needs mappings plus Reset nonces. Legacy v1 registry files load with an empty nonce map, and stale debounced writes cannot erase a durable tombstone or restore a retired mapping.
  • Client-switch detection treats pi as a provider only when it is the executed basename; the known pi-coding-agent entrypoint remains recognized, while unrelated later argv words no longer block switching.
  • Pi and the shared retry policy consume one provider-scoped exhausted-capacity predicate, so too many requests and resource has been exhausted consistently become provider_capacity / provider_rate_limited only for the intended providers.
  • Live inject opens only after the head delivery reports provider entry, closes at settlement, remains available in provider-entered retries, and invalidates on suspend/terminate/failure boundaries.
  • The CodeQL missing-binary classifier is linear; lifecycle-abort waiters are disposed; manager shutdown best-effort stops handlers whose failed suspend left stop unconfirmed.

Validation

Exact root-reviewed and independently qualified source HEAD: 5d873fa7577779e87b6a4c5813dfa550fed212e3 (parent 18d4881f1f9b20e479e465e2852cbeb1c186d672).

  • Independent full-isolated formal QA: PASS; strengthened packages/qa/cases/runtime/pi-provider.md used unchanged; disposition no-change.
  • Full Shared: 818/818; Client: 2,280 passed / 7 skipped; Server clean rerun: 3,015/3,015; Web: 2,009/2,009; CLI: 1,593 passed / 1 skipped across 24/24 batches.
  • Focused Shared/Client/Server/CLI: 772/772; terminal-disposition Client: 48/48; terminal-disposition Server: 85/85; real Pi host smoke: 4/4.
  • @first-tree/qa 4/4; skill-evals 505/505; no-model floor 8/8.
  • Typecheck 11/11; build 5/5; pnpm check PASS (18 existing warnings / 7 infos / 0 errors); release-pack smoke, four portable targets, exact topology, integrity, and git diff --check PASS.

The exact-head product path used the final packaged Darwin arm64 Client, an isolated production Server/PostgreSQL/MinIO/Web stack, and official authenticated Pi 0.83.0. It proved:

  • A real addressed row reactivated the chat after truthful applied:true. Before disposition it remained durable and unACKed with zero provider entry, recovery, or ACK. The Server sent exact aborted(reason=reactivated), received its release receipt, and only then returned HTTP 409. That opened exactly one same-socket recovery; the exact row entered one fresh nonce-derived Pi session and ACKed once without another Reset, reconnect, restored history, duplicate recovery, or no-progress rejection.
  • A real registry write fault made Reset return HTTP 503 with applied:false; the old mapping and one pending nonce stayed durable. Intervening work remained fenced without recovery storm. A genuine retry ordered applied → durable eviction → exact finalized receipt → one same-socket recovery under the expected fresh nonce-derived identity.
  • A clean Reset with no debt at apply time still fenced a row delivered between apply and terminal disposition. Deterministic exact-head seams also proved route-refusal and cleanup-failure abort, original-client route movement, stale/foreign/duplicate/superseded refs, receipt-loss/new-ref retry, and both sides of V1 capability/route revalidation.
  • Live same-turn custody remained prompt=1, steer=1, agent_settled=1, one unsafe effect, and prefix ACK only after settlement. Seven native tools, managed Skills, auth notice-before-ACK, helper acquisition, platform drain, runtime proof, SCM paths, privacy, and cleanup all passed.

The branch is a clean two-commit fast-forward from the previously pushed head and GitHub reports it mergeable. Hosted GitHub CI is fully green on the exact head/current base: 15 successful, 0 failing, 0 pending; the remaining checks are expected skips. Client/Web, Server, CLI, lint/typecheck, migrations, portable smoke, all CodeQL analyses, CLA, and branch checks passed.

Context Tree

Durable provider/runtime decisions are tracked in draft Context Tree PR first-tree-context#875. It remains Draft until this source PR merges, then will be reconciled against merged source truth before becoming ready.

yuezengwu and others added 17 commits August 1, 2026 22:05
Introduce a macOS/Linux Pi provider driven by official `pi --mode rpc`
JSONL, with resolve-only capability probing, provider-owned auth, and
deterministic V1 MCP rejection before prompt launch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the rejected request envelope with official command objects, parse
assistantMessageEvent/usage shapes, and keep abort/steer/settlement waiters
from hanging. Also refresh active config/Skills at idle turns, restart the
RPC process for spawn-scoped changes, treat prompt-response timeouts as
unknown custody, fix lowercase CT tool refs + git deltas, raise the minimum
published version to 0.80.5, and expand the QA case.

Co-authored-by: Cursor <cursoragent@cursor.com>
… auto-retry

Keep Pi-owned auto-retry as the only in-turn retry path, advance one-shot
context at accept/unknown boundaries, refresh briefing/Skills digests with
idle RPC restarts, and harden auth/capability classification plus usage and
model assertion contracts for the next root-reviewable head.

Co-authored-by: Cursor <cursoragent@cursor.com>
Emit durable exhausted-retry and config notices before ACK, follow shared
preflight retry, advance briefing fingerprints on the hot path, parse
colon-bearing model IDs like Pi 0.83, attach Context Tree HEAD/deltas, and
replace raw provider diagnostics with allow-listed detail.

Co-authored-by: Cursor <cursoragent@cursor.com>
…rivacy

Route active model/MCP config failures through the shared classifier so
hot-path injects emit a durable terminal notice before consumed ACK;
validate get_state with Pi 0.83 exact-first model matching; and keep RPC
diagnostics on allow-listed tokens instead of raw response ids/commands.

Co-authored-by: Cursor <cursoragent@cursor.com>
Route hot-path drain failures (refresh, format, RPC restart, get_state,
version gate) through ProviderAttempt with a finite batch-local retry loop
instead of unbounded inbox token.retry; classify get_state/session protocol
errors as terminal Pi capability.

Co-authored-by: Cursor <cursoragent@cursor.com>
Track the active drain batch/promise and use a shared abortable retry sleep
so suspend/shutdown can cancel backoff, await in-flight drain/turn work, and
recover each delivery exactly once with no late post-lifecycle token mutation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Formal QA rejected 521f17a for missing grep/find/ls on Pi 0.83 spawns and
for replaying an accepted bash turn after graceful client shutdown. Pass the
centralized V1 --tools allowlist on every RPC spawn, keep delivery leases valid
while handlers drain on manager shutdown, and terminally ACK provider-entered
custody exactly once before reconnect.

Co-authored-by: Cursor <cursoragent@cursor.com>
Root rejected reason-string inference for provider-entered shutdown custody
and the conflation of route adoption with delivery settlement. SessionManager
now passes settleProviderEntered only on full graceful drain, restores the
shuttingDown adoption fence, and keeps a separate settlement lease for
already-issued tokens so durable notice + ACK can complete before invalidate.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep SessionContext mutations behind the shuttingDown fence while delivery
tokens still settle terminal notice+ACK. Abort and consume on prompt write
or accept gaps without requiring a first stream event, and regress deferred
ctx probes plus before-write/write-pending/accepted-no-event shutdowns.

Co-authored-by: Cursor <cursoragent@cursor.com>
Manual session:suspend now settles write/tool-entered Pi turns before
prepareOperatorSuspend ACK, keeping route adoption fenced without
invalidating the settlement lease mid-drain.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep suspended+suspending only on the settlement lease so SessionContext
mutations stay fenced during operator suspend, and classify post-writeLine
onCommandWritten failures as after_write so FT cannot re-prompt.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clearing the dynamic token provider before SessionManager.shutdown made
notice HTTP fall back to a stale bind-time proof, so graceful drain could
settle Pi without posting notice/ACK and reconnect replayed the entered turn.
Keep the provider resolvable until drain joins; cleanup in finally after.

Co-authored-by: Cursor <cursoragent@cursor.com>
Active and deferred inject DeliveryTokens reused the adoption-only route
lease for notice+ACK, so manual session:suspend flipped the entry to
suspended and blocked token.complete; prepareOperatorSuspend then ACKed
the processing prefix without a durable runtime notice. Mirror
start/resume: mutation stays isRouteAdoptionValid, settlement uses
isDeliverySettlementLeaseValid, and attempt cancel revokes both.

Co-authored-by: Cursor <cursoragent@cursor.com>
Notice persistence failure left owned/processing rows that prepareOperatorSuspend
promoted to ordinary terminal. After recoverChat cleared debt, redelivery hit
redelivery_terminal_retry and ACKed without ever posting the human-visible
notice. Mark notice-required custody on the inbox ledger, retain it across
recovery, and settle notice-then-ACK on redelivery without re-entering the
provider.

Co-authored-by: Cursor <cursoragent@cursor.com>
A recovered notice-only redelivery that fails again must close the
activation window so the next dispatch can request recovery #2 instead
of looping on recoveryWindowOpen with no recoverChat call.

Co-authored-by: Cursor <cursoragent@cursor.com>
Unconditional --offline set PI_OFFLINE and blocked Pi's ensureTool("fd")
on clean supported hosts. Suppress version checks and install telemetry
via forced child env instead; preserve explicit operator PI_OFFLINE.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread packages/client/src/runtime/pi-binary.ts Fixed
@yuezengwu

Copy link
Copy Markdown
Contributor Author

Context Tree draft: https://github.com/agent-team-foundation/first-tree-context/pull/875

It records the durable Pi provider, authentication, platform-support, configuration, and onboarding boundaries from this source PR. It will remain draft until this source change is merged and the Tree diff is reconciled against merged source truth.

yuezengwu and others added 8 commits August 2, 2026 15:11
Integrate exact origin/main 7810022 (Agent Template lifecycle, chat
session reset / replay-fence, Kimi background lifecycle) while preserving
Pi delivery-custody settlement leases, operator-suspend notice-before-ACK,
and graceful-shutdown settleProviderEntered behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep settlement-lease notice-before-ACK, notice-required redelivery, and
operator/manager shutdown ordering while adopting main replay-fence and
pending-teardown semantics. Abort gated Pi config refresh on lifecycle end
so manager shutdown cannot hang before-write.
Classify missing Pi CLI errors with linear string checks and add an
adversarial repeated-pi regression that does not rely on wall-clock timing.
Treat completed failed operator suspend/teardown markers as unconfirmed-stop
work during manager shutdown, and dispose Pi lifecycle-abort waiters when a
config refresh wins so long-lived sessions do not accumulate closures.
Formal QA showed same-chat follow-ups waiting behind routeTransition until
agent_settled, then entering as a second prompt. Open inject after the head
proves membership via processingStarted, allow Pi steer once the prompt is
written/accepted, and make Pi credential notices point at host-local pi /login.

Co-authored-by: Cursor <cursoragent@cursor.com>
Root review found late steers after agent_settled could strand custody during
turn finalization, and provider-entered session retries still deferred follow-ups
until settlement. Close steer once the observation is settled, and open the same
identity-fenced live-inject path for retry transitions after processingStarted.

Co-authored-by: Cursor <cursoragent@cursor.com>
Operator suspend cleared routeTransition without resetting the inject-readiness
latch. Keep the pointer and latch coherent so readiness cannot outlive the
transition or be reused by a later branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Ordinary merge of exact origin/main HEAD 2cf0c27
(docs: introduce risk-tiered QA execution #2132). No Client runtime path changes.
@yuezengwu
yuezengwu marked this pull request as ready for review August 2, 2026 23:47
@yuezengwu
yuezengwu requested a review from baixiaohang as a code owner August 2, 2026 23:47

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: request changes

  • Rationale: Reset reopens the discarded Pi provider thread, client switching can be blocked by unrelated processes that merely mention pi, and two Pi capacity phrasings bypass the provider-capacity policy.

Risk level: A

  • Path baseline: includes apps/cli/** and packages/client/** -> A
  • Semantic lift: touches provider lifecycle, inbox custody, and runtime session state (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators run First Tree agents through the host-authenticated Pi CLI while preserving per-chat continuity, native tools, same-turn follow-up steering, and accepted-message custody.
  • Approach: Supervise one long-lived LF-JSONL Pi RPC process per chat, keep authentication/configuration provider-owned, and integrate Pi across shared schemas, client lifecycle, server attribution, CLI switching, web configuration, and QA cases.
  • Impacted modules: apps/cli, packages/client, packages/server, packages/shared, packages/web, packages/qa

Review findings

❌ 1. Reset deterministically reopens the Pi session it is supposed to discard. stablePiSessionId() derives the same ID from (agentId, chatId) on every start, prepareSession() always points that ID at the same persisted directory, and shutdown() only closes the RPC process. Pi 0.83.0 explicitly opens an existing project session when --session-id matches (upstream implementation). Therefore session:terminate can durably remove First Tree's mapping while the next addressed message silently restores the old Pi transcript/model state, violating the Reset contract documented in the SessionManager itself. Please give terminate/reset an explicit provider-session retirement boundary (with failure propagated before apply-ACK) and add a regression that proves the next post-Reset Pi turn has fresh provider history/state. [R2/R4/R5 / packages/client/src/handlers/pi/index.ts:105, packages/client/src/handlers/pi/index.ts:1793, packages/client/src/handlers/pi/index.ts:2127, packages/client/src/runtime/session-manager.ts:991]

❌ 2. The provider-process detector treats pi as any command-line word, not the executed binary. A harmless process such as python worker.py --constant pi now satisfies isKnownProviderCommand(); because it has no trusted First Tree runtime markers, client/account switching fails closed with a drain issue. The adjacent Cursor agent handling already explains why generic names must be matched only as the first token's basename. Please apply that executable-basename rule to Pi (while retaining the known package-entrypoint match) and cover both a real /path/pi --mode rpc process and an unrelated argv containing pi. [R5 / apps/cli/src/core/client-switch.ts:862, apps/cli/src/__tests__/client-switch.test.ts:260]

❌ 3. Pi's sanitizer classifies too many requests and resource has been exhausted as capacity, but the shared classifier recognizes those exact strings only when provider === "grok". The same raw Pi error is consequently emitted as unknown/unknown_exhausted, selecting different retry delays and a different durable notice from provider_capacity/provider_rate_limited. Please make the Pi recognition and shared policy use one provider-scoped classification rule and add Pi regressions for both phrasings. [R1/R4 / packages/client/src/handlers/pi/index.ts:87, packages/client/src/runtime/provider-retry-policy.ts:641]

✅ 4. The live-inject gate, agent_settled prefix ACK boundary, and after-write no-replay fencing are internally consistent with the existing custody contract.

Action taken

  • Submitted request changes.

@yuezengwu
yuezengwu requested a review from baixiaohang August 3, 2026 04:36

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: request changes

  • Rationale: Two findings are fully repaired, but the new Reset identity can still be reconstructed from a durable redelivery and reopen the discarded Pi session after a client restart.

Risk level: A

  • Path baseline: includes apps/cli/** and packages/client/** -> A
  • Semantic lift: the repair changes the provider-session identity and Reset boundary (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators use the host-authenticated Pi CLI with per-chat continuity, same-turn steering, and settlement-based inbox custody.
  • Follow-up scope: reviewed the three repair commits after 9a5b5d80e6a1d8aedb8fe43abe797483f1c36bd7 and verified the subsequent main merge did not alter their blobs.
  • Impacted modules: apps/cli, packages/client, packages/qa

Review findings

❌ 1. freshStartPiSessionId(..., message.id) is reproducible from the durable inbox row, so mapping deletion is not an irreversible Reset boundary. A concrete failure path is: Pi settles first message M and persists its transcript, the inbox ACK keeps failing so M remains durable recovery debt, the operator pauses and successfully Resets, then the Client restarts and bind recovery redelivers M after the in-memory terminal ledger is gone. Reset deleted the registry mapping, so this routes through start; freshStartPiSessionId hashes the same M id and Pi's --session-id opens the discarded on-disk session again. drainForTerminate() deliberately preserves an unacked remainder, and server Reset evicts the session/clears traces without deleting inbox rows, so “the next start has a new message id” is not a valid invariant. Please make retirement non-reconstructible from redelivery (for example, durably retire the Pi artifact before apply-ACK or retain a durable reset epoch/tombstone outside the deleted resume mapping) and add a restart regression: settled first turn + persistent ACK failure + Pause/Reset + new SessionManager + redelivery of the same inbox message must not reopen the old Pi history/model state. [R2/R4/R5 / packages/client/src/handlers/pi/index.ts:120, packages/client/src/handlers/pi/index.ts:1931, packages/client/src/runtime/inbox-delivery-coordinator.ts:582, packages/client/src/runtime/session-manager.ts:1115]

✅ 2. The client-switch repair is complete: generic pi is now matched only as the first executable basename, while the package entrypoint stays recognized and both true/false-positive regressions are present.

✅ 3. The capacity repair is complete: Pi and Grok now share one provider-scoped exhaustion predicate, while unrelated providers remain excluded.

Action taken

  • Submitted request changes on cbcb88c087f1d17a2349de0f41d9e28d697c6ef8.

@yuezengwu

Copy link
Copy Markdown
Contributor Author

CI is now fully green on pushed HEAD cbcb88c087f1d17a2349de0f41d9e28d697c6ef8: Client/Web, Server, CLI, lint/typecheck, migrations, portable smoke, CodeQL, CLA, and branch checks all passed. The PR remains mergeable; the only remaining gate is the requested human re-review.

@yuezengwu
yuezengwu requested a review from baixiaohang August 3, 2026 04:42
yuezengwu and others added 2 commits August 3, 2026 12:59
Make provider-session retirement non-reconstructible from same-row inbox
redelivery after Pause/Reset and Client restart by persisting a per-chat
fresh-start nonce outside the deleted resume mapping and hashing it into
Pi's start identity.

Co-authored-by: Cursor <cursoragent@cursor.com>
Restore the exported load() Map return shape via loadSnapshot() for nonce
consumers, tighten the Reset restart regression premise, and correct the
freshStartNonce comment for pending failed-flush rotations.

Co-authored-by: Cursor <cursoragent@cursor.com>
@yuezengwu

Copy link
Copy Markdown
Contributor Author

@baixiaohang The same-row restart finding is now closed additively at exact HEAD fc83865b44be3fdeff5901ff5b1f48e20d16a68d.

  • Reset rotates an opaque per-chat nonce stored durably outside the active resume mapping. Mapping removal and the nonce flush together before apply-ACK; failed flushes retain the pending nonce and leave Reset retryable.
  • Pi fresh-start identity includes that nonce. Resume still uses the persisted provider id, and same-row retries before Reset remain stable.
  • The restart regression now awaits the settled first dispatch, proves the old mapping is durable while ACK remains failed, applies Pause/Reset, creates a new SessionManager, and redelivers the exact same entry/message. The new Pi --session-id is nonce-derived and cannot reopen the retired artifact.
  • SessionRegistry.load() keeps its prior Map contract; the new snapshot API is additive, and the cumulative CLI diff for this repair is empty.

Independent full-isolated QA repeated the exact product path with the final packaged Client and authenticated Pi 0.83: the first row settled while three ACK attempts failed, Reset durably removed the mapping and stored the nonce, then a fresh Client recovered that same durable row under a different session id. The retired artifact hash was unchanged, the new history had parentSession: null, and the row ACKed once. Full gates include Client 2259, Server 2997, CLI 1593, real Pi smoke 4/4, typecheck 11/11, and check PASS.

Please re-review exact HEAD fc83865b44be3fdeff5901ff5b1f48e20d16a68d after its GitHub CI completes.

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: request changes

  • Rationale: The durable nonce closes the original restart/redelivery gap, but retaining that nonce across a failed Reset allows an intervening Pi start to consume the identity that a later successful Reset treats as retired.

Risk level: A

  • Path baseline: includes apps/cli/** and packages/client/** -> A
  • Semantic lift: the follow-up changes provider-session identity and the Reset apply boundary (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators use the host-authenticated Pi CLI with per-chat continuity, same-turn steering, and settlement-based inbox custody while making Reset discard provider history even across restart/redelivery.
  • Follow-up scope: reviewed 13011435f and fc83865b4 after cbcb88c0, including the new registry tombstone, fresh-start identity, failure retry, and same-row restart regression.
  • Impacted modules: packages/client, packages/qa

Review findings

❌ 1. A failed Reset releases admission while preserving the pending nonce, so an intervening fresh Pi route can use the exact identity that the successful retry later records as the post-Reset tombstone. Concrete path: the first terminate rotates N, deletes local session/mapping, then flushOrThrow fails; finally removes terminatingChats, while terminatePersistFailures is not checked by dispatch, routeMessage, startNewSession, or held-chat sync. A new/redelivered row M can therefore start Pi as hash(agent, chat, M, N) (and Server explicitly leaves the stopped row retryable; an addressed message can reactivate it). After Pause and Reset retry, rotateFreshStartNonce returns the same pending N and the flush succeeds. If M remains durable and the Client restarts, it reconstructs the intervening Pi id and reopens history even though that second Reset returned applied. Please either fence route admission until the failed Reset is successfully retried, or ensure the successful retry advances past any nonce an intervening start could have observed; add the corresponding regression: failed flush -> intervening start with durable/unACKed row -> Pause/successful Reset retry -> new SessionManager/same-row redelivery must not reuse the intervening Pi id. [R4/R5 / packages/client/src/runtime/session-manager.ts:1103, packages/client/src/runtime/session-manager.ts:1128, packages/client/src/runtime/session-manager.ts:2495, packages/client/src/runtime/session-registry.ts:125]

✅ 2. The persisted per-chat nonce correctly repairs the originally reported settled+ACK-failed -> Reset -> restart -> same-row-redelivery path when the Reset flush succeeds without an intervening route.

✅ 3. The prior client-switch attribution and shared provider-capacity fixes remain complete in this head.

Action taken

  • Submitted request changes on exact HEAD fc83865b44be3fdeff5901ff5b1f48e20d16a68d.

@yuezengwu

Copy link
Copy Markdown
Contributor Author

GitHub CI is fully green on exact HEAD fc83865b44be3fdeff5901ff5b1f48e20d16a68d: 14 successful, 0 failing, 0 pending; the remaining checks are expected skips. Client/Web, Server, CLI, lint/typecheck, migrations, portable smoke, all CodeQL analyses, CLA, and branch checks passed. The PR remains mergeable and is ready for the requested re-review.

@yuezengwu
yuezengwu requested a review from baixiaohang August 3, 2026 06:47
Keep route admission closed while terminatePersistFailures holds the chat
so an intervening start cannot consume the pending Reset nonce before the
successful terminate retry durably retires the mapping.

Co-authored-by: Cursor <cursoragent@cursor.com>

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: request changes

  • Rationale: Provider admission is now fenced, but a delivery that hits the fence enters an automatic same-socket recovery loop that opens the server's no-progress circuit and leaves the chat unable to recover without reconnecting.

Risk level: A

  • Path baseline: includes apps/cli/** and packages/client/** -> A
  • Semantic lift: the follow-up changes provider-route admission and Reset/recovery custody (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators use Pi with durable message custody while Reset reliably discards provider history, including flush-failure retries and same-row restart recovery.
  • Follow-up scope: reviewed the single repair commit after fc83865b44be3fdeff5901ff5b1f48e20d16a68d, including all admission gates, held-chat reporting, retry paths, production inbox recovery, and the two new regressions.
  • Impacted modules: packages/client, packages/qa

Review findings

❌ 1. A row that reaches dispatch() while the new Reset fence is set is passed to the generic retryDeliveryTurn(), which immediately requests chat-scoped recovery. In production, every accepted recovery resets and redelivers that row on the same socket; recoveryActivationReady re-admits it, it hits the unchanged fence, and requests recovery again. After two identical accepted resets, the third opens the server's no-progress circuit and leaves the row pending until an ACK or fresh bind. The fence forbids provider entry/ACK, and a successful terminate retry does not clear that circuit, so subsequent same-socket messages only encounter rejected recovery and the chat cannot resume without reconnecting. Both new tests omit recoverChat, so they prove “no provider/no ACK” but never exercise this production loop. Please park fenced delivery without initiating repeated same-socket recovery, then release/recover it once the successful Reset retry has cleared the fence; add a production-shaped same-socket regression proving the intervening row enters the fresh post-Reset session without a reconnect and without opening the no-progress circuit. [R4/R5 / packages/client/src/runtime/session-manager.ts:816, packages/client/src/runtime/inbox-delivery-coordinator.ts:418, packages/server/src/api/agent/ws-client.ts:1910]

✅ 2. The combined fence now covers start, resume, inject, deferred retry, and transition revalidation, so an intervening provider route cannot consume the pending Reset nonce.

✅ 3. Failed persistence is force-kept in held-chat reconciliation and the genuine terminate retry can still execute and clear the marker.

Action taken

  • Submitted request changes on exact HEAD bd1ec4366fe1b57d3d9a13fc50cfebc6dca43aeb.

@yuezengwu

Copy link
Copy Markdown
Contributor Author

@baixiaohang The failed-Reset admission finding is now closed additively at exact HEAD bd1ec4366fe1b57d3d9a13fc50cfebc6dca43aeb (parent fc83865b44be3fdeff5901ff5b1f48e20d16a68d).

  • SessionManager now treats both an in-flight terminate and terminatePersistFailures as one provider-route admission fence. Every prior admission/post-await CAS site uses it; retry entry is fenced too, while duplicate terminate still joins only the genuine in-flight promise so a real retry can clear the persistence failure.
  • In-flight and persist-failed Reset chats are force-kept in runtime sync. An intervening delivery cannot start, resume, inject, retry, ACK, or consume the pending Reset nonce.
  • Independent full-isolated QA on the immutable exact head returned PASS (no-change). A real registry fault produced HTTP 503 / applied:false; the exact intervening row stayed durable and unACKed with no new Pi process/prompt/steer. The genuine retry returned HTTP 200 only after deletion plus the same nonce were durable. A fresh packaged Client then recovered that exact row under the expected nonce-derived identity; the new Pi history had no parent, the retired artifact was unchanged, and the row ACKed exactly once.
  • Exact-head gates: full Client 2261 passed / 7 skipped; focused Client 409/409; focused Server 119/119; focused CLI 126/126; real Pi 4/4; typecheck 11/11; QA 4/4; skill-evals 505/505 plus floor 8/8; pnpm check passed with 0 errors.
  • GitHub CI is fully green on the pushed head/current base: 15 successful, 0 failing, 0 pending; remaining checks are expected skips. The PR is mergeable.

Please re-review exact HEAD bd1ec4366fe1b57d3d9a13fc50cfebc6dca43aeb.

@yuezengwu
yuezengwu requested a review from baixiaohang August 3, 2026 08:17
@yuezengwu

Copy link
Copy Markdown
Contributor Author

Acknowledged. The same-socket no-progress circuit is a valid gap in bd1ec436…: the provider fence is correct, but its generic retryDeliveryTurn() path requests recovery too early and can repeat until the server parks the row behind the circuit. The exact-head formal scenario reconnected before recovery, so it did not prove same-socket progress.

I am repairing this additively by parking Reset-fenced custody without repeated recovery, then issuing one coalesced release after the successful persistence retry. The regression will use a production-shaped recovery/redelivery loop and prove the exact row reaches the fresh nonce-derived session on the same socket without opening the circuit. I will re-request review only after root review, independent QA, and hosted CI on the new exact head.

yuezengwu and others added 5 commits August 3, 2026 16:31
Avoid same-socket recoverChat loops when Reset admission fencing rejects
a delivery, then release parked work once after a successful terminate
flush clears the fence so the intervening row can settle without
opening the server's no-progress circuit.

Co-authored-by: Cursor <cursoragent@cursor.com>
Scope parked-debt recovery to successful Reset finalization only: do not
recover from terminate's finally or local flush success, wait for
session:command:finalized (post-finalizeTerminatedSession), and rename the
deferred park helper to neutral semantics.

Co-authored-by: Cursor <cursoragent@cursor.com>
The post-finalize half of chat-session Reset was fire-and-forget and
chat-scoped, so a lost frame, a route move, or a stale generation could
leave a client parked behind a fence nobody lifts while the operator saw
HTTP 200.

- Release the parked Reset fence only for the generation the slot armed:
  the terminate ref now flows into SessionManager arm/release, and stale,
  duplicate, superseded, or foreign refs are answered honestly instead of
  lifting a newer Reset's fence.
- Route the finalized signal exactly like the ref'd terminate — same
  replica requires the live agent/client pair, everything else fans out
  through PG with the resolved clientId and instance, and the fan-out
  revalidates the DB route before delivering.
- Hold the Reset request open until the client's receipt lands, using a
  separate ackRef rendezvous plus the same durable fallback as the
  apply-ack so a lost wake still converges.
- Negotiate the handshake two-sidedly: server:welcome now precedes
  auth:ok and advertises server support, the client declares its half
  only in response, and either skew direction fails Reset closed before
  anything destructive is applied.

Co-authored-by: Cursor <cursoragent@cursor.com>
Close the post-apply window, make SessionManager the only Reset-generation
authority, and version the Reset handshake so mixed fleets fail closed.

- Arming a ref'd Reset now fences the chat unconditionally, so a row that
  arrives between applied and finalized parks instead of starting a turn on
  a session the server has not finished evicting.
- SessionManager owns generations outright: refs that join one termination
  are aliases of a single generation, release returns an accepted /
  idempotent / stale verdict the AgentSlot reports verbatim, and unref'd
  authority goes through an explicit supersede instead of incidental
  cleanup, so a delayed stale reconcile cannot lift a newer Reset's fence.
- Reset is negotiated through the composite wsSessionResetV1 capability
  only; a client never advertises the legacy apply-only flag as Reset-ready,
  so neither an old server nor an old client can start a Reset whose fence
  nothing would ever release.

Co-authored-by: Cursor <cursoragent@cursor.com>
The composite `wsSessionResetV1` capability was only checked at the HTTP
preflight. Because `client:register` replaces `wireCapabilities` wholesale,
the same clientId on the same instance could reconnect as a legacy build and
still receive a destructive `session:terminate`, since send, cross-replica
fan-out and the durable apply-ACK all revalidated identity only.

The capability is now rechecked at every step that can lead to a destructive
apply: the same-replica send, the fan-out (DB route plus owning-process live
socket), the apply-ACK frame handler, and the `applied` phase of the durable
rendezvous store. The post-apply finalize delivery and receipt stay
identity-only so a client that already applied still converges even if its
advertised capability changes afterwards.

Co-authored-by: Cursor <cursoragent@cursor.com>

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: request changes

  • Rationale: The finalize handshake closes the recovery-storm gap, but a Reset that fails after the client has applied the terminate has no terminal wire outcome and can leave the chat permanently fenced.

Risk level: A

  • Path baseline: includes apps/cli/** and packages/client/** -> A
  • Semantic lift: this follow-up changes the Client/Server Reset protocol, session identity, and inbox-custody boundary (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators reset a paused/failed Pi-backed chat without reviving discarded provider history, while preserving durable inbox work and same-socket progress across Reset failures.
  • Follow-up scope: reviewed the five commits after bd1ec4366fe1b57d3d9a13fc50cfebc6dca43aeb, including parked custody, Reset generations, the apply/finalize/receipt handshake, V1 negotiation, cross-replica delivery, and the production-shaped wire regression.
  • Impacted modules: packages/client, packages/server, packages/shared, packages/web, packages/qa

Review findings

❌ 1. Once the client applies a ref'd terminate, it unconditionally arms that Reset generation and only a matching session:command:finalized can release provider admission. However, the server has terminal paths after the apply where it never sends any outcome: most concretely, an addressed message can reactivate the session while the apply ack is in flight, finalizeTerminatedSession returns active, and terminateWithApplyAck throws 409 before confirmResetFinalization. The existing server regression at admin-sessions-suspend-terminate.test.ts:1201 proves this race, but it only checks the DB. On the real client, the provider mapping is already deleted, the row that caused reactivation is parked behind the armed generation, and no frame can now release it; the chat cannot process that message on the same socket unless the operator performs another Pause + Reset (or reconnects). Route/cleanup failures after a truthful apply have the same missing-terminal-outcome shape. Please make every post-apply terminal branch either finalize or explicitly abort/supersede the exact generation through a receipted wire outcome, and add a production-shaped regression where a new addressed row reactivates during the ack wait yet reaches the fresh provider session without an extra Reset or reconnect. [R4/R5 / packages/server/src/api/sessions.ts:267, packages/server/src/api/sessions.ts:271, packages/client/src/runtime/session-manager.ts:1177, packages/client/src/runtime/session-manager.ts:1629]

✅ 2. Fenced delivery now parks without entering the generic same-socket recovery loop, and the matching finalized generation opens one coalesced recovery after server eviction.

✅ 3. The composite V1 capability is negotiated in both directions and revalidated at destructive terminate delivery and apply persistence; the legacy apply-only flag no longer authorizes Reset.

Action taken

  • Submitted request changes on exact HEAD fc639aca439cb425f542c697649fb784228bc761.

@yuezengwu

Copy link
Copy Markdown
Contributor Author

@baixiaohang The same-socket no-progress finding and its follow-on Reset-finalization/capability races are now closed additively at exact HEAD fc639aca439cb425f542c697649fb784228bc761 (previous pushed head bd1ec4366fe1b57d3d9a13fc50cfebc6dca43aeb).

  • Reset-fenced delivery is parked without recoverChat, so repeated delivery cannot trip the no-progress circuit. A successful apply arms a generation-scoped fence; only the exact Server-confirmed finalization releases it and requests one coalesced same-socket recovery.
  • The complete two-sided wsSessionResetV1 handshake now covers client apply, durable Server finalization, an exact generation release, and a distinct receipt before HTTP 200. Stale, foreign, superseded, duplicate, and joined alias refs cannot release another generation.
  • The post-apply/pre-finalized window stays closed even when no row was parked at apply time. A genuine new-ref Reset retry converges after a lost finalization.
  • Capability is revalidated at destructive delivery and apply settlement: exact live V1 on same-replica send, DB plus live V1 on cross-replica fan-out, and atomic route/instance/V1 on durable apply storage. Post-apply finalization remains identity-scoped so an already-applied capable client can converge.

Independent full-isolated QA on the immutable exact head returned PASS (no-change). The final packaged Darwin Client, isolated production Server/PostgreSQL/MinIO/Web stack, and authenticated official Pi 0.83.0 proved the real failed-flush row stayed unACKed with no provider entry or recovery storm; the successful retry finalized first, then recovered once on the same socket into the exact fresh nonce-derived identity. A clean post-apply window was independently held and remained fenced until exact finalization.

Exact-head gates: Client 2,278 passed / 7 skipped; Server 3,011/3,011; Shared 817/817; Web 2,009/2,009; CLI 1,593 passed / 1 skipped; focused Client 548/548, Server 315/315, Shared 45/45, CLI 79/79; real Pi 4/4; typecheck 11/11; build 5/5; QA 4/4; skill-evals 505/505 plus floor 8/8; pnpm check passed with 0 errors.

GitHub CI is fully green on the pushed exact head/current base: 15 successful, 0 failing, 0 pending; remaining checks are expected skips. The PR is mergeable. Draft Context Tree PR #875 has also been reconciled with the durable Reset lifecycle and parked-custody constraint and remains draft until this source PR merges.

Please re-review exact HEAD fc639aca439cb425f542c697649fb784228bc761.

@yuezengwu
yuezengwu requested a review from baixiaohang August 3, 2026 13:09
@yuezengwu

Copy link
Copy Markdown
Contributor Author

Acknowledged. The post-apply reactivation finding is valid on fc639aca…: applied:true proves the Client has irreversibly retired the provider mapping, but the Server's active/route/cleanup exits do not currently send a terminal disposition, so the exact Reset generation can remain fenced indefinitely.

I am repairing this additively so every branch after truthful apply ends in one receipted exact-generation outcome: durable eviction finalizes it; a downstream refusal or failure explicitly aborts/supersedes it. The terminal phase will target the original applying Client identity, will not restore discarded provider context, and will release at most one same-socket recovery for the durable row. Pre-apply V1/route/capability gates and no-false-200 behavior remain unchanged.

The production-shaped regression will reactivate with a real addressed row during the ACK/finalize window and prove that the same row reaches a fresh nonce-derived provider session on the same socket, with no extra Reset or reconnect and no recovery circuit. I will re-request review only after root review, independent QA, and hosted CI on a new exact head.

yuezengwu and others added 2 commits August 3, 2026 21:54
Accepting `session:command:applied` with `applied: true` arms a Reset
generation on the client that only an exact receipted disposition can
lift. Every server branch that then failed to commit the eviction — a
session re-activated inside the ack window, a route refused by the
finalize transaction, a rolled-back cleanup — returned 409/500 with no
terminal wire outcome at all, leaving that chat fenced forever: no
provider admission, no recovery, no ACK, and no operator action that
converges.

Add `session:command:aborted` + `session:command:aborted:ack` as the
second post-apply disposition, mirroring finalized, and make every
post-apply exit complete exactly one receipted disposition before it
reports an HTTP outcome. Abort restores nothing — the provider mapping
is durably gone the moment the client acked — it lifts only that one
generation, so the parked durable row recovers once into a fresh
nonce-derived provider session. Stale, foreign, superseded, and
duplicate refs are still answered by the client's own generation
authority, so a late abort cannot lift a newer fence. If the abort
signal or its receipt cannot converge, the Reset fails closed and says
the client may still be fenced.

Both dispositions are now scoped to the ORIGINAL applying client
identity rather than the agent's current route — for delivery, for the
cross-replica fan-out, and for the durable receipt row. The route moving
after a truthful apply is precisely when abort fires, so guarding these
phases on it refused the receipt in exactly the cases they exist for.
The destructive `applied` phase keeps every route and `wsSessionResetV1`
gate unchanged, and an HTTP 200 still requires both a durable eviction
and a matching finalize receipt.

Co-authored-by: Cursor <cursoragent@cursor.com>
After applied:true, either receipted terminal disposition (finalized or
aborted) releases the generation; both post-apply phases are identity-scoped.

Co-authored-by: Cursor <cursoragent@cursor.com>
@yuezengwu

Copy link
Copy Markdown
Contributor Author

@baixiaohang The post-apply missing-terminal-outcome finding is closed additively at exact HEAD 5d873fa7577779e87b6a4c5813dfa550fed212e3 (previous pushed head fc639aca439cb425f542c697649fb784228bc761).

  • After truthful applied:true, every non-finalizing Server exit now sends and receipts an exact-generation session:command:aborted before returning its HTTP error: reactivation, route refusal, and rolled-back cleanup are covered. Durable eviction remains the only session:command:finalized / HTTP 200 path. Abort never restores the retired provider mapping or history; it releases only the applying generation and opens at most one same-socket recovery.
  • Both terminal dispositions target the original applying Client identity after the irreversible apply. Destructive terminate delivery and apply persistence still require the current route, instance, and composite wsSessionResetV1; stale, foreign, duplicate, superseded, and joined alias refs cannot release another generation.
  • Independent full-isolated QA on the immutable exact head returned PASS (no-change). A real addressed row reactivated during the apply/finalize window: before abort it stayed durable/unACKed with zero provider entry, recovery, or ACK; the exact aborted(reason=reactivated) receipt preceded HTTP 409, then one same-socket recovery admitted that row under one fresh nonce-derived Pi identity without another Reset/reconnect, restored history, duplicate recovery, or no-progress rejection.
  • The failed-flush/finalized-retry and clean post-apply window also passed live. Deterministic seams covered route refusal, cleanup rollback, route movement, receipt loss/new-ref retry, ref authority, and both V1 downgrade directions.
  • Exact-head gates: Shared 818/818; Client 2,280 passed / 7 skipped; Server clean rerun 3,015/3,015; Web 2,009/2,009; CLI 1,593 passed / 1 skipped; focused 772/772; typecheck 11/11; build 5/5; QA 4/4; skill-evals 505/505 plus floor 8/8; real Pi 4/4; pnpm check passed with 0 errors.
  • Hosted GitHub CI is fully green on the exact head/current base: 15 successful, 0 failing, 0 pending; remaining checks are expected skips. The PR is mergeable.

Draft Context Tree PR #875 has been reconciled to the phase boundary: route/V1 revalidation through destructive apply, then identity-scoped receipted finalized | aborted convergence after apply. It remains draft until this source PR merges.

Please re-review exact HEAD 5d873fa7577779e87b6a4c5813dfa550fed212e3.

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: approve

  • Rationale: The exact-head repair closes the last post-apply Reset liveness gap without weakening the durable eviction, provider-history retirement, or inbox-custody boundaries.

Risk level: A

  • Path baseline: the cumulative PR includes apps/cli/** and packages/client/** → A
  • Semantic lift: the follow-up changes the Client/Server Reset protocol and inbox-custody boundary (remains A)

PR Summary

  • Author / repo: yuezengwu / agent-team-foundation/first-tree
  • Problem: Let operators use the host-authenticated Pi CLI with durable message custody while Reset reliably discards provider history and preserves same-socket progress through restart, redelivery, and post-apply Server failures.
  • Follow-up scope: reviewed the two commits after fc639aca439cb425f542c697649fb784228bc761, including the aborted disposition, original-client delivery/receipt authority, every post-apply terminal branch, and the production-shaped reactivation regression.
  • Impacted modules: packages/client, packages/server, packages/shared, packages/qa

Review findings

✅ 1. Every branch after truthful applied:true now reaches an exact, receipted terminal disposition: durable eviction finalizes, while reactivation, route refusal, and rolled-back cleanup abort before returning their HTTP error.

✅ 2. finalized and aborted share the Client SessionManager generation authority, so stale, foreign, superseded, and duplicate refs cannot release a newer fence; abort releases custody without restoring the retired provider mapping or history.

✅ 3. Post-apply delivery and receipt target the original applying Client identity, while destructive terminate delivery and apply persistence retain current route, instance, and composite V1 capability validation.

✅ 4. The same-socket reactivation regression now proves the durable row reaches one fresh nonce-derived provider session without another Reset, reconnect, duplicate recovery, or no-progress circuit.

Action taken

  • Approved exact HEAD 5d873fa7577779e87b6a4c5813dfa550fed212e3 with explicit reviewer authorization.

@yuezengwu
yuezengwu merged commit 8a03c0b into main Aug 4, 2026
19 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 4, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants