fix(sdk): reconcile ordered lifecycle outcomes - #4186
Conversation
A transport timeout after WebSocket.send cannot prove rejection.\n\nRetain process-local reconciliation material and require explicit lookup rather than resend.\n\nLore-id: 3986-client-uncertainty\nConstraint: ordered lifecycle work must never auto-resend\nConfidence: medium\nScope-risk: bounded\nReversibility: straightforward\nTested: bun --cwd=packages/bridge-client run check:types
Wire clients need explicit accepted and lookup state without moving broker authority into Rust.\n\nLore-id: 3986-rust-protocol\nConstraint: Rust remains authenticated wire ingress only\nConfidence: medium\nScope-risk: bounded\nReversibility: straightforward\nTested: cargo test -p gjc-sdk broker_protocol
Accepted lifecycle admission must have durable identity evidence before callers reconcile terminal state separately.\n\nLore-id: 3986-broker-reconciliation\nConstraint: target chains serialize effects but do not deduplicate\nConstraint: broker requests settle once\nConfidence: medium\nScope-risk: bounded\nReversibility: straightforward\nTested: bun test test/sdk-lifecycle-idempotency-restart.test.ts\nNot-tested: package typecheck blocked by missing generated docs-index.generated
Sent lifecycle frames cannot be classified as rejected when their response is lost.\n\nLore-id: 3986-client-tests\nConfidence: high\nScope-risk: narrow\nReversibility: straightforward\nTested: bun --cwd=packages/bridge-client run check:types; bun test packages/bridge-client/test/client.test.ts
Persist canonical request fingerprints separately from normalized request hashes so lookup can reconcile uncertain sends. Retain accepted lifecycle admission as durable in-progress evidence, and classify uncertain close and chat delivery paths as ambiguous rather than definitive failure. Lore-id: 3986-round1-repair Constraint: Option 1 safety-first lockout; no new migration authority or indexed mapping Constraint: ordered lifecycle requests never auto-resend Constraint: repair preserves origin/dev..HEAD history Reviewed: Opus round-1 findings 1-10 Tested: bun test packages/coding-agent/test/sdk-lifecycle-idempotency-restart.test.ts packages/bridge-client/test/client.test.ts packages/coding-agent/test/sdk-chat-daemon-worker.test.ts Tested: cargo fmt --check Tested: cargo check -p gjc-sdk Tested: cargo test -p gjc-sdk broker_protocol Not-tested: bun run check:ts is blocked by pre-existing missing packages/coding-agent/src/internal-urls/docs-index.generated.ts Confidence: high Scope-risk: bounded Reversibility: straightforward
Accepted lifecycle admissions can safely retry until an effect fence is durable. Legacy target identities migrate only on an exact target match, while ambiguous changes reject instead of gaining authority. Lore-id: 3986-final-repair Constraint: preserve Option-1 typed ambiguous lockout Constraint: accepted is retry-safe; effect_started is the lockout boundary Tested: bridge client lifecycle tests; broker lifecycle tests; cargo fmt --check; cargo check -p gjc-sdk Not-tested: full check:ts blocked by pre-existing generated docs index absence Confidence: medium Scope-risk: scoped Reversibility: code-only
Legacy target-inclusive ledger rows lacked an operation/key index, allowing changed-target requests to escape the ambiguity fence. Reject unmatched legacy rows while preserving exact-identity migration, align lookup fingerprints with transmitted JSON, and reconcile terminal-facing callers after post-send uncertainty.\n\nLore-id: 3986\nConstraint: preserve exact-target legacy migration only\nConstraint: do not add migration authority or automatic resend\nConfidence: medium\nScope-risk: bounded\nReversibility: straightforward\nTested: bridge client, ledger regression, Rust fmt/check, TypeScript aggregate gate\nNot-tested: full lifecycle e2e suite timed out on existing subprocess close scenario
A durable accepted row can survive a crash before an effect marker. Re-admitting it would re-execute an operation without proof, while exact legacy migration must retain its reconciliation metadata. Lore-id: 3986 Constraint: accepted rows remain retry-safe; effect_started retains the lockout boundary; legacy migration is exact-target only Tested: focused bridge/client lifecycle lane; selected broker cleanup replay regressions; cargo fmt --check; cargo check -p gjc-sdk Not-tested: full broker lifecycle suite has pre-existing subprocess state-root failures Confidence: high Scope-risk: narrow Reversibility: source-only
A crash after durable admission but before effect start could permanently wedge the lifecycle key. Re-admit accepted rows while preserving effect_started as the uncertainty lockout. Lore-id: issue-3986-accepted-repair Constraint: accepted remains retry-safe until effect_started Confidence: high Scope-risk: narrow Reversibility: clean Tested: focused lifecycle test, cargo fmt, cargo check, cargo test
The mechanical rebase retained two imports that current dev no longer uses. Removing them restores the package lint gate. Lore-id: issue-3986-rebase-imports Constraint: preserve reviewed lifecycle behavior Confidence: high Scope-risk: narrow Reversibility: clean Tested: focused SDK tests and package Biome check
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — hard integration conflict with active lifecycle-ownership PR #4098; exact-head CI not green
Reviewed exact head: 4f274651b46b89aea6a21271e5b8d1b3089e327c (PR #4186, 18 files, +325/−58), base 7858b0ff63db6a3fc0cce93639a8d329ca474b2c (current dev tip — same base as PR #4098). Author: grantjayy. Review-only; no source mutation, push, merge, or release performed.
1. Hard #4098 modify/delete conflict — this PR cannot land as written
PR #4098 (refactor(sdk): make core own session lifecycle and attachments, owner-authored, OPEN, 204 files, +11662/−48333) is the active mutation owner of the session-lifecycle domain and deletes the two primary surfaces this PR modifies:
packages/bridge-client/src/client.ts— #4098 deletes the wholepackages/bridge-clientpackage (7 files, 1359 deletions, incl.src/client.ts−722 andtest/client.test.ts−524) and moves the SDK client intopackages/coding-agent/src/sdk/client/client.ts. #4186's core contract —uncertain_after_sendcode,SdkSentRecord,lookupLifecycle, fingerprinting,#settlePendingrewrite (+91 inclient.ts, +9/−11 inclient.test.ts, CHANGELOG entry) — all lands inside that deleted package.crates/gjc-sdk/src/lifecycle.rs— #4098 deletes it outright (548 lines, "remove … the obsolete Rust lifecycle protocol") and rewriteslib.rs. #4186 addsLifecycleStatetolifecycle.rs(+12) andBrokerLookupLifecycletobroker_protocol.rs/ re-exports it fromlib.rs— new protocol surface on a layer #4098 removes.
Read-only conflict proof, git merge-tree 7858b0ff <4186-head> <4098-head>: modify/delete (whole-file deletion of client.ts, lifecycle.rs vs. #4186's modifications) plus 9 "changed in both" textual conflicts: crates/gjc-sdk/src/lib.rs, packages/coding-agent/src/coordinator-mcp/server.ts, src/sdk/acp/adapter.ts, src/sdk/broker/broker.ts, src/sdk/broker/lifecycle.ts, src/sdk/bus/chat-daemon-runtime.ts, src/sdk/cli/session-cli.ts, src/sdk/mcp/server.ts, test/sdk-chat-daemon-worker.test.ts.
This is a conflicting duplicate in #4098-owned territory, not complementary non-overlap: 11 of #4186's 18 files overlap #4098, two of them by outright deletion. Required: rebase/reconstruct onto the post-#4098 surface after #4098's disposition (SessionRouter/SessionLifecycleService, relocated SDK client, removed Rust lifecycle protocol), and re-run exact-head verification against the new base. Neither merge order works as-is: before #4098, this diff is obsolete work on a package about to be deleted; after #4098, it does not apply.
2. Exact-head CI is not green
Affected path validation / test:@gajae-code/coding-agent:shard-1-of-8(run31406877251, job93517514300) — FAILURE:(fail) resident cache prune retention, lifecycle cleanup, and JSONL parity > moveTo materializes before cache reset and rewrites JSONL from the new resident store [95.51ms]; summary 2492 pass / 1 fail / 39 skip across 165 files.- The same test file passed its dedicated affected-path job (
test:packages/coding-agent/test/session-manager-resident-cache.test.ts→ SUCCESS), so the shard failure is interference/flake-shaped rather than diff-local — but it is still an exact-head red run and must be reconciled before any merge. - One job still
IN_PROGRESS(cargo-build:cargo:cGktbmF0aXZlcw…→ pi-natives).
3. Unique contract that should survive
The core idea is worth salvaging: after a sent-frame timeout, a caller needs durable reconciliation keyed by the idempotency identity, never blind retry of ordered work. #4098's broker diff (credential-free storedResponse persistence) does not supply post-send uncertainty lookups, so this is a genuine gap its stated scope leaves open. Salvage path:
- Re-target the lookup/reconciliation contract onto the surfaces #4098 does not touch:
lifecycle-ledger.tsandbroker/identity.tsare absent from #4098's file list and are the natural home for theoperationKey/fingerprintindex and lookup. - Keep error-code changes out of the released
SdkRequestTimeoutDetails/timeoutcontract (bridge-client 0.12.20, from merged #4067 which already closed #3986). #4186 silently replacescode: "timeout"+details.requestSentwithcode: "uncertain_after_send"+SdkSentRecord— a public DTO break for 0.12.20/0.12.21 consumers whosedetails.requestSent === truebranch now mis-fires into retry. If the rename is kept, it needs explicit versioning/deprecation documentation. - Changelog hygiene: the new bridge-client entry was inserted under released
## [0.12.16] - 2026-08-08; it must move to## [Unreleased].
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Findings
Major — reconciled lifecycle results no longer satisfy the original operation contract
packages/coding-agent/src/sdk/broker/broker.ts:1332-1340 returns lookup data as { operation, state, response, durableEffects }, but every new uncertainty handler returns that lookup frame directly (packages/coding-agent/src/sdk/acp/adapter.ts:229-243, packages/coding-agent/src/sdk/mcp/server.ts:257-275, packages/coding-agent/src/coordinator-mcp/server.ts:3195-3220). Existing consumers still expect the original lifecycle response at the top level; for example, coordinator create reads created.sessionId at packages/coding-agent/src/coordinator-mcp/server.ts:4643-4662. A reconciled terminal_ok therefore loses the session id under response, while accepted, effect_started, or awaiting_ready is returned as though the lifecycle call completed. Reconciliation must wait for a terminal state and then replay/throw the stored original response contract, rather than exposing the lookup envelope in place of it.
Major — explicit server rejections are misclassified as transport uncertainty
packages/bridge-client/src/client.ts:689-706 converts any sent SdkClientError whose code is unavailable into uncertain_after_send. That path also receives parsed ok:false response frames through packages/bridge-client/src/client.ts:629-633, so a known server rejection is indistinguishable from a lost response. The broker legitimately returns an explicit unavailable before admission at packages/coding-agent/src/sdk/broker/broker.ts:1269-1271 (and transport failures are serialized at packages/coding-agent/src/sdk/broker/transport.ts:148-152). Lifecycle callers then perform a lookup for work the broker explicitly rejected and replace the real error with not_found; non-lifecycle callers also receive the wrong error class. Only transport-originated failures should be wrapped as post-send uncertainty.
Major — one legacy ledger row permanently blocks unrelated future lifecycle requests
packages/coding-agent/src/sdk/broker/lifecycle-ledger.ts:679-689 migrates by appending the new identity but leaves the metadata-free legacy identity in the ledger, so hasLegacyIdentity() at lines 676-678 remains true after a successful migration. packages/coding-agent/src/sdk/broker/broker.ts:1355-1359 then rejects every later operation/key without an exact legacy identity match as idempotency_conflict. Any installation with prior lifecycle history can therefore become unable to create, resume, close, or delete unrelated sessions after upgrading. The new test at packages/coding-agent/test/sdk-lifecycle-idempotency-restart.test.ts:18-24 only asserts that a legacy row is detectable; it never exercises migration followed by a fresh request, so this lockout is missed.
Focused verification: packages/bridge-client/test/client.test.ts passed (14 tests) and packages/coding-agent/test/sdk-lifecycle-idempotency-restart.test.ts passed (22 tests). sdk-chat-daemon-worker.test.ts could not start because this checkout could not resolve @gajae-code/utils/native-process.
gajae.pr-review-verdict.v1: needs-human
3 participants
Summary
effect_startedfenceCloses #3986.
Verification
bun test packages/coding-agent/test/sdk-lifecycle-idempotency-restart.test.ts— 22 passedcargo fmt --checkcargo check -p gjc-sdk --all-targetscargo test -p gjc-sdk— 166 passedBoundaries
(operation, idempotencyKey)