Skip to content

fix(client)!: one rule for host-callback exceptions, fix the adopted-save ghost, slim proposal 0009 - #55

Merged
sunib merged 4 commits into
feat/proposal-0010-gateway-api-cleanupfrom
fix/stream-review-feedback
Oct 5, 2026
Merged

sunib merged 4 commits into
feat/proposal-0010-gateway-api-cleanupfrom
fix/stream-review-feedback

Conversation

@sunib

@sunib sunib commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Stacked on #54, which is stacked on #53. This PR is based on feat/proposal-0010-gateway-api-cleanup and adds four commits. #53 and #54 are unchanged. Each PR retargets as the one below it merges.

It addresses the review of #53 and proposals 0009/0010.

Code

The adopted-save ghost, as its own fix: (9b092d6)
adoptSaved went through the stream's upsert path, which marks the UID as seen in an open snapshot. A save response adopted after a reconnect's reset therefore kept a deleted UID alive through an empty synced. Now only the stream's own upserts count towards a snapshot. adoptSaved and accepted guarded reads share a private update path that never marks membership. A new regression test reproduces the ghost; it fails before the fix and passes after.

Host-callback exceptions, one rule (40955b1)
Before, a subscribe callback or onError that threw had different effects depending on timing:

  • during a connection, it was swallowed as a network failure and retried until exhausted;
  • between connections, it rejected closed without publishing closed;
  • from the health timer's live, it escaped as an uncaught error.

Now the consumer, subscribers and onError all follow the consumer's rule. The first exception stops the stream without retrying, every subscriber still sees the state being published, the stream ends closed, and closed rejects with that exception after cleanup. The internal fetch loop no longer catches host code. Four new tests cover these cases, and all four fail against the previous connector.

Docs

  • Upgrade guide from 0.7 (585baa1, docs/migrating.md, linked from both READMEs):
    • Every removed browser and gateway name, mapped to its replacement.
    • Replacing onSynced with a first-snapshot promise.
    • Attaching .catch to closed.
    • Not using an async consumer: it type-checks, but live can then come before the snapshot is applied, and its exceptions are lost.
    • The nested StreamConfig literal. Assignments still work through field promotion.
  • The README states the exception rule and the async-consumer warning.
  • Proposal 0009 plan (0921657):
    • Change 2 is now the editor removals and fixes only.
    • The ghost fix ships ahead of it.
    • SnapshotTracker, ResourceStore with the read-only example, and the stream/editor entry points are deferred, each with an explicit trigger. The reference use for ResourceStore is the repository's own example, and the one known consumer edits.
    • The out-of-scope note on observer exceptions is replaced by the decision above.
  • Proposal 0010 records where it is implemented, the nested-literal cost and the one golden message line it changes.
  • Markdown wraps: both proposals are re-wrapped at 100 columns. Fragments such as state and }` sat on lines of their own, one of them inside a code span. Only whitespace changes; the text is identical with whitespace collapsed.

Not done here

Verification

task verify passes on 0921657: fixtures-check, lint, test (client 134/134, Vue 2/2, both Go modules), release-rehearsal, e2e-wire, e2e-browser (20/20) and pack-client. No Go code changes in this PR.

BEGIN_COMMIT_OVERRIDE
fix(client): never count an adopted save response as snapshot membership
fix(client)!: stop the stream when any host callback throws

BREAKING-CHANGE: an exception thrown by a subscribe callback or onError now stops the stream without retrying and rejects closed with that exception, as a consumer exception does.
docs: add an upgrade guide from 0.7
END_COMMIT_OVERRIDE

🤖 Generated with Claude Code

sunib and others added 4 commits October 4, 2026 20:45
adoptSaved went through the stream's upsert path, which marks the UID
as seen in an open snapshot cycle. A save response that landed after a
reconnect's reset therefore kept the object alive through a snapshot
that no longer contained it: a deleted UID survived an empty synced and
nothing would ever remove it.

Only the stream's own upserts count towards a snapshot now. adoptSaved
and accepted guarded reads share a private update path that never
marks membership; a response is still adopted, and synced prunes it if
the snapshot does not send it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the consumer's exceptions were handled precisely. A subscribe
callback or onError that threw during a connection was swallowed as a
network failure and retried until the budget ran out, so a buggy render
function became a reconnect storm ending in exhausted. One thrown
between connections rejected closed without publishing closed, and one
thrown by the health timer's live escaped as an uncaught error.

Every exception from host code now takes one path. The connector
records the first, aborts the connection and stops without retrying,
still notifies every subscriber of the state being published, ends in
closed (unless it had already ended terminal or exhausted) and rejects
closed with that exception after cleanup. The internal fetch loop no
longer catches host code at all.

The README says to attach a rejection handler to closed and not to pass
an async consumer, which type-checks but can let live precede the
applied synced and loses its exceptions.

BREAKING CHANGE: an exception thrown by a subscribe callback or onError now stops the stream without retrying and rejects closed with that exception, as a consumer exception does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One page maps every removed browser and gateway name to its
replacement: the connector's callback, state and subscribe in place of
its lifecycle callbacks, a first-snapshot promise without onSynced, and
the nested StreamConfig literal. It also says to attach .catch to
closed, not to pass an async consumer, and that a protocol mismatch is
terminal. The READMEs link to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review found change 2 bundled removals, the snapshot tracker, a new
ResourceStore, entry points and new option shapes, and that the case
for ResourceStore is thin: its reference use is the repository's own
example, and krm-foyer, the one known consumer, edits.

- Change 2 is now the editor removals and fixes only.
- The adopted-save membership bug ships ahead of it as a fix:.
- SnapshotTracker, ResourceStore and the read-only example, and the
  stream/editor entry points, are deferred with explicit triggers:
  a read-only view that should not load the merge engine, or an
  unbundled consumer that benefits from loading only the stream layer.
- Exceptions from subscribe callbacks and onError follow the consumer's
  rule (stop, no retry, reject closed), replacing the earlier
  out-of-scope note, and async consumers are documented as unsupported.
- Proposal 0010 records its implementation, the nested-literal cost and
  the one golden message line it changes.

Both proposals are re-wrapped at 100 columns. The previous reflow left
fragments such as `state` and `}` on lines of their own, one of them
splitting an inline code span; only whitespace changes there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 19 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b2228d44-409d-49aa-8b14-bb58c5e9e351
📥 Commits

Reviewing files that changed from the base of the PR and between 8bc56df and 0921657.

📒 Files selected for processing (10)
  • README.md
  • docs/migrating.md
  • docs/proposals/0009-stream-and-editor-separation.md
  • docs/proposals/0010-gateway-api-cleanup.md
  • packages/krm-stream/README.md
  • packages/krm-stream/src/connection.ts
  • packages/krm-stream/src/sse.ts
  • packages/krm-stream/src/store.ts
  • packages/krm-stream/test/connection.test.ts
  • packages/krm-stream/test/saving.test.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sunib
sunib added this pull request to stack #56 October 4, 2026 21:00
@sunib
sunib merged commit d7e0677 into main Oct 5, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant