chore(rivetkit): remove deprecated conn tokens & sse support #3419

NathanFlurry · 2025-11-09T03:07:52Z

No description provided.

vercel · 2025-11-09T03:07:58Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-site	Ready	Preview	Comment	Nov 13, 2025 2:50am
rivetkit-serverless	Ready	Preview	Comment	Nov 13, 2025 2:50am

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 2:50am
rivet-inspector	Ignored	Preview		Nov 13, 2025 2:50am

NathanFlurry · 2025-11-09T03:08:08Z

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

claude · 2025-11-09T03:09:38Z

PR Review: Remove deprecated conn tokens & SSE support

Summary

This PR removes deprecated connection token authentication and SSE (Server-Sent Events) support from RivetKit, streamlining the connection protocol. The changes include protocol versioning updates and a migration path for persisted actor state.

Code Quality ✅

Strengths:

Well-structured migrations: The schema versioning system properly handles backward compatibility with migrations from v1→v2→v3
Clean refactoring: Consistent removal of deprecated code paths across the codebase (removed ~1200 lines)
Protocol evolution: Clear separation between v1 and v2 client protocols with proper migration logic

Areas for attention:

Migration logic complexity (rivetkit-typescript/packages/rivetkit/src/schemas/actor-persist/versioned.ts:29-81):
- The v2→v3 migration merges connections and hibernatableWebSocket arrays by matching hibernatableRequestId
- Uses Buffer.from().equals() for comparing ArrayBuffers
- Potential issue: If a connection has a hibernatableRequestId but no matching WebSocket in the array, it's silently dropped. Consider adding a warning log for debugging.
Bidirectional conversion mismatch (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2097-2148):
- #convertToBarePersisted and #convertFromBarePersisted handle the connection/hibernatable split differently
- In #convertFromBarePersisted, lastSeen is hardcoded to 0 which could cause issues with connection liveness checks
- Recommendation: Set lastSeen from lastSeenTimestamp for consistency

Potential Bugs 🐛

Lost subscription data (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2101-2131):
- The new v3 schema doesn't persist connection subscriptions
- Old code stored: subscriptions: conn.subscriptions.map((sub) => ({ eventName: sub.eventName }))
- New code omits subscriptions entirely from hibernatableConns
- Impact: After actor hibernation/wake, connections will lose their event subscriptions
- Severity: HIGH - This will break functionality for hibernatable WebSockets that use subscriptions
WebSocket URL path change (examples/cursors-raw-websocket/src/frontend/App.tsx:93):
- Changed from /raw/websocket to /websocket
- Verify: Ensure backend routing matches this change (constants updated in actor-router-consts.ts)
- ✅ Confirmed: PATH_WEBSOCKET_PREFIX changed accordingly
Typo in comment (rivetkit-typescript/packages/rivetkit/src/actor/router-endpoints.ts:373):
- // TODO: Remvoe any, Hono is being a dumbass → Should be "Remove"
- Minor issue but should be fixed for professionalism

Security Concerns 🔒

Connection token removal:
- ✅ POSITIVE: Removing connection tokens (generateConnToken, IncorrectConnToken error) simplifies the authentication model
- The PR properly removes token validation from the WebSocket reconnection path
- Connection ID is now the sole identifier (UUID-based)
Removed liveness checks (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:1305-1367):
- Old code: Periodic interval to check if connections are still alive and remove dead ones
- New code: Removes the entire #checkConnectionsLiveness mechanism
- Concern: Without liveness checks, zombie connections could accumulate
- Mitigation needed: Ensure hibernatable WebSocket cleanup still works correctly

Performance Considerations ⚡

Reduced overhead: Removing SSE driver and connection token validation reduces per-connection overhead
Migration cost: The v2→v3 migration involves array iteration and buffer comparisons - should be acceptable for typical actor sizes
Sleep timer logic (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:1911-1916):
- Current logic always returns CanSleep.ActiveConns when ANY connection exists
- TODO comment suggests this blocks hibernation entirely
- Recommendation: Address this TODO to properly enable hibernation for actors with only hibernatable connections

Test Coverage 🧪

Changes to test files:

Updated driver tests to remove connId and connToken parameters
Modified WebSocket URL paths in tests
Removed deprecated protocol tests

Recommendations:

Add migration tests for v2→v3 schema conversion, especially edge cases:
- Connection with hibernatableRequestId but missing WebSocket
- Multiple connections with same request ID
Test subscription persistence after hibernation/wake cycle
Verify backward compatibility with clients still using v1 protocol

Breaking Changes 🔴

This PR introduces breaking changes:

Connection tokens are no longer supported (clients using them will fail)
SSE connections are removed entirely
WebSocket URL paths changed from /raw/websocket to /websocket
Init message no longer includes connectionToken field

Migration path: The protocol versioning should handle older clients, but verify all client SDKs are updated.

Action Items 📋

Critical:

Fix subscription loss in v3 schema - add subscriptions to PersistedHibernatableConn
Set lastSeen correctly in #convertFromBarePersisted

Recommended:

Add logging when connections are dropped during v2→v3 migration
Address hibernation TODO to prevent blocking sleep with hibernatable connections
Fix typo: "Remvoe" → "Remove"
Add tests for schema migration edge cases
Update client SDKs and documentation for breaking changes

Overall Assessment

This is a well-intentioned cleanup that simplifies the codebase, but has one critical bug (subscription loss) that needs to be fixed before merging. The schema versioning and migration approach is solid, but needs additional test coverage.

Recommendation: Request changes to fix the subscription persistence issue, then approve after verification.

pkg-pr-new · 2025-11-09T03:10:36Z

More templates

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/actor@3419

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3419

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/core@3419

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3419

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3419

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3419

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3419

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3419

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3419

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3419

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3419

commit: e109ee5

claude · 2025-11-13T01:40:12Z

Pull Request Review: Remove Deprecated Connection Tokens & SSE Support

Overview

This PR successfully removes deprecated connection token authentication and SSE (Server-Sent Events) support from RivetKit, simplifying the connection model by focusing solely on hibernatable WebSockets. The changes are substantial (436 additions, 1227 deletions) but generally well-structured.

Positive Aspects

1. Protocol Simplification

✅ New protocol v2 schemas are clean and well-documented
✅ Removal of connectionToken from Init messages (rivetkit-typescript/packages/rivetkit/schemas/client-protocol/v2.bare:4) simplifies authentication flow
✅ Migration strategy properly handles v1 → v2 protocol upgrade (rivetkit-typescript/packages/rivetkit/src/schemas/client-protocol/versioned.ts:20-36)

2. Schema Evolution

✅ New v3 persist schema consolidates hibernatable connections elegantly (rivetkit-typescript/packages/rivetkit/schemas/actor-persist/v3.bare)
✅ Flattened structure removes redundancy between connections and hibernatableWebSocket arrays

3. Code Cleanup

✅ Removed 1227 lines of deprecated code
✅ Eliminated SSE driver complexity (rivetkit-typescript/packages/rivetkit/src/actor/conn-drivers.ts:29-179)
✅ Removed connection liveness checks that are no longer needed (rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:420-447)

Issues & Concerns

1. Critical: Data Loss Risk in Conversion Logic ⚠️

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2100-2128

#convertToBarePersisted(persist: PersistedActor<S, CP, CS, I>): bareSchema.PersistedActor {
    const hibernatableConns: bareSchema.PersistedHibernatableConn[] = [];
    
    for (const conn of persist.connections) {
        if (conn.hibernatableRequestId) {
            const ws = persist.hibernatableWebSocket.find((ws) =>
                arrayBuffersEqual(ws.requestId, conn.hibernatableRequestId!),
            );
            
            if (ws) {  // ← What happens to connections without matching ws?
                hibernatableConns.push({...});
            }
        }
    }
}

Problem:

Non-hibernatable connections are silently dropped during conversion
Connections with hibernatableRequestId but no matching WebSocket are also silently dropped
No logging or error handling for these edge cases

Recommendation: Add logging for dropped connections or throw an error if data integrity is critical:

for (const conn of persist.connections) {
    if (conn.hibernatableRequestId) {
        const ws = persist.hibernatableWebSocket.find((ws) =>
            arrayBuffersEqual(ws.requestId, conn.hibernatableRequestId!),
        );
        
        if (ws) {
            hibernatableConns.push({...});
        } else {
            this.#rLog.warn({
                msg: "hibernatable connection has no matching websocket during persist conversion",
                connId: conn.connId,
            });
        }
    } else {
        this.#rLog.debug({
            msg: "skipping non-hibernatable connection during persist conversion",
            connId: conn.connId,
        });
    }
}

2. Bug: Incorrect lastSeen Initialization 🐛

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2160

connections.push({
    connId: conn.id,
    params: cbor.decode(new Uint8Array(conn.parameters)),
    state: cbor.decode(new Uint8Array(conn.state)),
    subscriptions: [],
    lastSeen: 0, // ← Should use lastSeenTimestamp, not 0!
    hibernatableRequestId: conn.hibernatableRequestId,
});

Problem: Setting lastSeen: 0 means connections will appear to have been seen at Unix epoch (1970), which breaks any logic that depends on accurate timestamps.

Fix:

lastSeen: Number(conn.lastSeenTimestamp),

3. Potential Issue: Empty Subscriptions Array ⚠️

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2159

The new persist format doesn't include subscriptions, but the old format did. When converting from v3 → internal format, all connections get subscriptions: [], which means:

Active subscriptions will be lost during actor hibernation/wake cycles
This may be intentional if subscriptions are ephemeral, but should be documented

Recommendation: Add a comment explaining this behavior or implement subscription persistence if needed.

4. Incomplete TODO Comment

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:995

// TODO: Remove this for ws hibernation v2 since we don't receive an open message for ws

This TODO suggests the current implementation is temporary. If this PR is blocked on ws hibernation v2, that should be documented in the PR description. If not, this TODO should be clarified or removed.

5. Sleep Logic Commented Out

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2090-2096

// TODO: Enable this when hibernation is implemented...
// if (!conn.isHibernatable) return CanSleep.ActiveConns;
return CanSleep.ActiveConns;

Problem: The code unconditionally prevents sleeping when any connection exists, even if all are hibernatable. This defeats the purpose of hibernatable connections.

Recommendation: Either enable the proper logic or add a detailed comment explaining why this is temporarily disabled.

6. Typo in Schema Comment

Location: rivetkit-typescript/packages/rivetkit/schemas/actor-persist/v3.bare:11

# Last seem message index for this WebSocket

Should be "seen" not "seem".

Testing Concerns

1. Migration Path Not Tested

The PR removes 7 test files/changes but doesn't add tests for:

v1 → v2 protocol migration
v2 → v3 persist schema migration
Backward compatibility with existing persisted actors

Recommendation: Add integration tests for migration scenarios.

2. Edge Cases

Consider adding tests for:

Actor wake-up with persisted hibernatable connections
Connection reconnection after hibernation
Handling of connections that don't match WebSocket data during conversion

Minor Issues

1. Inconsistent Path Naming

Old: PATH_CONNECT_WEBSOCKET, PATH_RAW_WEBSOCKET_PREFIX
New: PATH_CONNECT, PATH_WEBSOCKET_PREFIX

While cleaner, this is a breaking API change. Ensure:

✅ Documentation is updated (appears done in website/src/content/docs/actors/fetch-and-websocket-handler.mdx)
✅ All clients are migrated
❓ Consider deprecation period vs hard break

2. Removed Errors Not Used Elsewhere

Verify that ConnNotFound and IncorrectConnToken errors (rivetkit-typescript/packages/rivetkit/src/actor/errors.ts:136-149) aren't referenced anywhere:

# Recommend running this check
grep -r "ConnNotFound\|IncorrectConnToken" rivetkit-typescript/

3. Version Bump Clarity

The OpenAPI version changed from 2.0.22 → 2.0.24-rc.1. The -rc.1 suffix suggests this is a release candidate. Ensure:

Semantic versioning is followed (breaking changes should bump major version)
The -rc.1 is intentional and will be removed before merge

Performance Considerations

✅ Positive Changes

Removed Connection Liveness Polling: Eliminates periodic interval checks, reducing CPU usage
Simplified Connection State: Fewer lookups and state synchronization
Reduced Message Size: Removing connectionToken from Init messages saves bandwidth

No Major Concerns

The conversion logic adds some overhead during persistence, but:

It's only called during save/load operations
The find operation is O(n*m) but n and m are typically small (connections count)
Could optimize with a Map if profiling shows issues

Security Considerations

✅ Positive Changes

Removed Token-Based Auth: Eliminates potential token leakage/reuse attacks
Cleaner Security Model: Relying on framework-level WebSocket authentication

Questions

Authentication After Token Removal: How are connections authenticated now? Is it purely based on WebSocket protocol headers or session context?
Connection Hijacking: Without tokens, what prevents connection hijacking if connectionId is leaked?

Recommendation: Add a security section to the PR description explaining the new authentication model.

Recommendations Summary

Must Fix Before Merge (🔴)

Fix lastSeen: 0 bug (should use lastSeenTimestamp)
Add logging/error handling for dropped connections in conversion
Re-enable or document the commented sleep logic

Should Fix (🟡)

Add migration tests
Clarify/remove the "ws hibernation v2" TODO
Document subscription persistence behavior
Fix typo in schema comment

Nice to Have (🟢)

Add edge case tests
Verify removed errors aren't used elsewhere
Document new authentication model
Consider Map optimization for conversion if profiling shows issues

Conclusion

This is a well-structured refactoring that significantly simplifies the RivetKit connection model. The removal of 1200+ lines of deprecated code is commendable. However, there are critical bugs in the conversion logic that must be addressed before merging:

Data loss risk (silent connection dropping)
Incorrect timestamp initialization
Potentially broken sleep logic

Once these issues are resolved and testing is improved, this PR will be ready to merge.

Overall Assessment: Approve with required changes ✅🔧

Generated with Claude Code

claude · 2025-11-13T02:59:17Z

Code Review: Remove deprecated conn tokens & SSE support

Summary

This PR removes deprecated connection token/ID-based reconnection logic and SSE (Server-Sent Events) support, simplifying the connection management system. The changes move toward relying exclusively on hibernatable WebSockets for connection persistence.

Positive Changes

Architecture Improvements

Simplified connection model: Removed complex connection token/ID authentication which reduces attack surface and maintenance burden
Cleaner protocol: The new v2 client protocol removes connectionToken from the Init message, making the handshake simpler
Hibernation-focused design: The new v3 persistence schema (PersistedHibernatableConn) consolidates connection data specifically for hibernatable WebSockets
Removed SSE driver: Eliminating SSE support removes an entire code path that likely had limited usage

Code Quality

Reduced complexity: Removed ~1400 lines while adding only ~400, significantly reducing cognitive load
Better schema versioning: Proper migrations in versioned.ts files for both actor-persist (v3) and client-protocol (v2)
Path simplification: Changed from /connect/websocket to /connect and /raw/websocket to /websocket

Issues & Concerns

Critical: Data Loss Bug in `#convertFromBarePersisted`

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance.ts:2160

lastSeen: 0, // Will be set from lastSeenTimestamp

This sets lastSeen to 0 but never actually sets it from lastSeenTimestamp. This should be:

lastSeen: Number(conn.lastSeenTimestamp),

Impact: This could cause issues with connection lifecycle management since lastSeen is used to track connection activity.

Potential Breaking Changes

Removed connection liveness checks: The entire #checkConnectionsLiveness interval and logic was removed (instance.ts:1308-1362). This means:
- Connections in "reconnecting" state will never be cleaned up
- The comment at instance.ts:1912 indicates this was intentional but needs hibernation v2 support
- Risk: Memory leaks from stale connections if hibernation doesn't handle this
Sleep behavior changed: At instance.ts:2011-2013, the code now returns CanSleep.ActiveConns for ANY connection, not just non-hibernatable ones:
```
// if (!conn.isHibernatable) return CanSleep.ActiveConns;
return CanSleep.ActiveConns;
```
This appears to disable actor sleep entirely when any connections exist, which may have performance/cost implications.
Removed connection status: The ConnectionStatus type and conn.status property were removed, which is a breaking API change for any code checking connection state.

Security Improvements

Removed token-based auth: Removing generateConnToken() and token validation eliminates a potential authentication bypass vector
Removed deprecated errors: ConnNotFound and IncorrectConnToken errors are no longer needed

Test Coverage Concerns

Deleted reconnection tests: The entire actor-reconnect.ts test file (170 lines) was deleted without replacement
- This removes validation of reconnection behavior
- The PR should add tests for the new hibernation-based reconnection model
Updated test references: Several tests were updated to remove connId/connToken parameters, which is good cleanup

Migration Safety

The schema migrations look correct:

v1→v2 migration (actor-persist): Adds hibernatableRequestId and hibernatableWebSocket array ✅
v2→v3 migration (actor-persist): Properly merges connections with hibernatable WebSocket data ✅
v1→v2 migration (client-protocol): Removes connectionToken from Init message ✅

However, the migration logic uses Buffer.from().equals() which works in Node but may fail in browser/edge environments. Consider using the existing arrayBuffersEqual utility consistently.

Recommendations

Must Fix

Fix the data loss bug in #convertFromBarePersisted - set lastSeen from lastSeenTimestamp

Should Address

Clarify sleep behavior: The commented-out hibernation check needs either:
- Documentation explaining when hibernation v2 will be ready
- A feature flag or TODO with tracking issue
Add hibernation tests: Replace the deleted reconnection tests with tests for the new hibernation-based model
Document breaking changes: Update CHANGELOG or migration guide for:
- Removed ConnectionStatus and conn.status
- Changed WebSocket paths
- Removed SSE support

Nice to Have

Connection cleanup strategy: Document how stale connections will be handled without liveness checks
Consider keeping connection count metric: The inspector connection info removed status field - ensure this doesn't break monitoring

Conventional Commit Compliance

The commit message follows the repository's conventional commit format ✅

Overall Assessment

This is a solid architectural improvement that simplifies the codebase significantly. The main concern is the critical data loss bug in the persistence layer that must be fixed before merge. The removal of connection liveness checks should also be carefully considered - either implement hibernation v2 support or add it back with a clear migration path.

Recommendation: Fix the lastSeen bug, add tests for hibernation reconnection, and document the breaking changes. Then this should be good to merge.

graphite-app · 2025-11-13T20:47:10Z

Merge activity

Nov 13, 8:47 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Nov 13, 8:48 PM UTC: CI is running for this pull request on a draft pull request (#3463) due to your merge queue CI optimization settings.
Nov 13, 8:50 PM UTC: Merged by the Graphite merge queue via draft PR: #3463.

NathanFlurry mentioned this pull request Nov 9, 2025

chore(rivetkit): move conns to separate persisted kv keys #3420

Closed

vercel bot deployed to Preview – rivetkit-serverless November 9, 2025 03:09 View deployment

vercel bot deployed to Preview – rivet-site November 9, 2025 03:11 View deployment

NathanFlurry mentioned this pull request Nov 9, 2025

chore(rivetkit): split ActorInstance logic in to multiple classes #3421

Closed

NathanFlurry marked this pull request as ready for review November 9, 2025 05:44

chore(rivetkit): remove deprecated conn tokens & sse support

e109ee5

NathanFlurry force-pushed the 11-08-chore_rivetkit_remove_deprecated_conn_tokens_sse_support branch from 06d93f5 to e109ee5 Compare November 13, 2025 02:47

vercel bot deployed to Preview – rivetkit-serverless November 13, 2025 02:48 View deployment

vercel bot deployed to Preview – rivet-site November 13, 2025 02:50 View deployment

This was referenced Nov 13, 2025

chore(rivetkit-typescript): remove dependency on node modules #3460

Merged

chore(rivetkit): switch dynamic node imports to use require #3461

Merged

chore(rivetkit): fix type checks #3462

Merged

This was referenced Nov 13, 2025

feat(gas): implement smart pulling for horizontally scaled workers #3417

Merged

fix(guard): handle websocket tasks during shutdown #3448

Merged

fix: improve sigterm handling for the entire runtime #3443

Merged

NathanFlurry mentioned this pull request Nov 13, 2025

feat(gateway): websocket hibernation #3400

Merged

This was referenced Nov 13, 2025

fix(pb): dont immediately reallocate actors on runner stopping #3413

Merged

fix(guard): dont send open msg for hibernating ws, hibernating ws keepalive #3449

Merged

NathanFlurry mentioned this pull request Nov 13, 2025

chore(gas): rename wake sub to bump sub #3408

Merged

MasterPtato approved these changes Nov 13, 2025

View reviewed changes

graphite-app bot pushed a commit that referenced this pull request Nov 13, 2025

chore(rivetkit): remove deprecated conn tokens & sse support (#3419)

3a5214a

graphite-app bot closed this Nov 13, 2025

graphite-app bot deleted the 11-08-chore_rivetkit_remove_deprecated_conn_tokens_sse_support branch November 13, 2025 20:50

This was referenced Nov 13, 2025

chore(rivetkit): implement new hibernating ws protocol #3464

Open

feat(cloudflare-workers): add createInlineClient #3466

Open

NicholasKissel mentioned this pull request Nov 15, 2025

chre: v4 website changes #3472

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(rivetkit): remove deprecated conn tokens & sse support #3419

chore(rivetkit): remove deprecated conn tokens & sse support #3419

Uh oh!

NathanFlurry commented Nov 9, 2025

Uh oh!

vercel bot commented Nov 9, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Nov 9, 2025 •

edited by jog1t

Loading

Uh oh!

claude bot commented Nov 9, 2025

Uh oh!

pkg-pr-new bot commented Nov 9, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

graphite-app bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore(rivetkit): remove deprecated conn tokens & sse support #3419

chore(rivetkit): remove deprecated conn tokens & sse support #3419

Uh oh!

Conversation

NathanFlurry commented Nov 9, 2025

Uh oh!

vercel bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NathanFlurry commented Nov 9, 2025 • edited by jog1t Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Nov 9, 2025

PR Review: Remove deprecated conn tokens & SSE support

Summary

Code Quality ✅

Potential Bugs 🐛

Security Concerns 🔒

Performance Considerations ⚡

Test Coverage 🧪

Breaking Changes 🔴

Action Items 📋

Overall Assessment

Uh oh!

pkg-pr-new bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 13, 2025

Pull Request Review: Remove Deprecated Connection Tokens & SSE Support

Overview

Positive Aspects

1. Protocol Simplification

2. Schema Evolution

3. Code Cleanup

Issues & Concerns

1. Critical: Data Loss Risk in Conversion Logic ⚠️

2. Bug: Incorrect lastSeen Initialization 🐛

3. Potential Issue: Empty Subscriptions Array ⚠️

4. Incomplete TODO Comment

5. Sleep Logic Commented Out

6. Typo in Schema Comment

Testing Concerns

1. Migration Path Not Tested

2. Edge Cases

Minor Issues

1. Inconsistent Path Naming

2. Removed Errors Not Used Elsewhere

3. Version Bump Clarity

Performance Considerations

✅ Positive Changes

No Major Concerns

Security Considerations

✅ Positive Changes

Questions

Recommendations Summary

Must Fix Before Merge (🔴)

Should Fix (🟡)

Nice to Have (🟢)

Conclusion

Uh oh!

claude bot commented Nov 13, 2025

Code Review: Remove deprecated conn tokens & SSE support

Summary

Positive Changes

Architecture Improvements

Code Quality

Issues & Concerns

Critical: Data Loss Bug in #convertFromBarePersisted

Potential Breaking Changes

Security Improvements

Test Coverage Concerns

Migration Safety

Recommendations

Must Fix

Should Address

Nice to Have

Conventional Commit Compliance

Overall Assessment

Uh oh!

vercel bot commented Nov 9, 2025 •

edited

Loading

NathanFlurry commented Nov 9, 2025 •

edited by jog1t

Loading

pkg-pr-new bot commented Nov 9, 2025 •

edited

Loading

Critical: Data Loss Bug in `#convertFromBarePersisted`

graphite-app bot commented Nov 13, 2025 •

edited

Loading