Cut legacy instance presence paths#36
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR removes the multi-instance heartbeat tracking infrastructure and multi-instance session linking, tightens relay guest authorization with a daemon-side gate, adds relay transport auth-failure detection and status, and simplifies runtime/session handling to local and offline sources. ChangesMulti-Instance Removal & Relay Guest Auth Enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/lib/relay-transport.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/relay-transport.test.ts`:
- Around line 36-47: The test setup enables fake timers (vi.useFakeTimers()) and
stubs globalThis.WebSocket with a MockWebSocket subclass while pushing instances
into sockets, but it doesn't guarantee teardown on assertion failures; wrap each
test's setup and assertions in a try/finally (or move teardown to afterEach) to
always restore state: ensure vi.useRealTimers() is called and
globalThis.WebSocket is reset to originalWebSocket, and clear/close any entries
in the sockets array (references: vi.useFakeTimers, MockWebSocket subclass,
sockets, globalThis.WebSocket) in the finally/afterEach block so subsequent
tests aren't affected.
In `@docs/multi-user-chat.md`:
- Around line 64-66: The guest-route allowlist is inconsistent: the first list
only shows GET /agents/output and GET /events while another section also allows
history routes; update both places to use the same explicit wording and route
set — e.g., list GET /agents/output, GET /events, and the agent history
endpoints (GET /agents/:id/history or equivalent history routes used elsewhere)
and use the same phrase “guest-readable routes” or “guest allowlist” in both
sections so the documentation matches exactly.
In `@src/remote-access.ts`:
- Around line 94-106: The shared-guest session check in
assertRemoteAccessAllowed currently accepts session OR sessionId but downstream
handlers (/events and /agents/output) only honor sessionId, allowing bypass when
only session= is provided; update the authorization to normalize the query so
downstream sees sessionId: when searchParams has "session" but not "sessionId",
copy/set that value into "sessionId" (or require "sessionId" explicitly) before
comparing to actor.shareSessionId, ensuring the handlers receive a sessionId
query param and per-session filtering cannot be bypassed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10f654f1-c187-4e35-8446-8b2e3037e80e
📒 Files selected for processing (36)
app/components/RelayIndicator.tsxapp/lib/relay-transport.test.tsapp/lib/relay-transport.tsdocs/multi-user-chat.mddocs/runtime-authority-dead-paths.mddocs/runtime-authority-inventory.mdrelay/src/relay-object.tsrelay/src/sharing.test.tsrelay/src/sharing.tssrc/config.tssrc/daemon.test.tssrc/daemon.tssrc/dashboard/index.tssrc/dashboard/session-registry.test.tssrc/dashboard/session-registry.tssrc/debug-state.test.tssrc/debug-state.tssrc/instance-directory.test.tssrc/instance-directory.tssrc/instance-registry.test.tssrc/instance-registry.tssrc/multiplexer/dashboard-control.tssrc/multiplexer/dashboard-interaction.tssrc/multiplexer/dashboard-model.tssrc/multiplexer/index.tssrc/multiplexer/runtime-lifecycle-methods.test.tssrc/multiplexer/runtime-lifecycle-methods.tssrc/multiplexer/runtime-state.test.tssrc/multiplexer/runtime-state.tssrc/multiplexer/runtime-sync.tssrc/multiplexer/session-launch.test.tssrc/multiplexer/session-launch.tssrc/paths.tssrc/project-scanner.tssrc/remote-access.tssrc/tui/screens/dashboard-renderers.ts
💤 Files with no reviewable changes (17)
- src/instance-directory.ts
- src/instance-registry.test.ts
- src/config.ts
- src/multiplexer/dashboard-model.ts
- src/multiplexer/session-launch.ts
- src/instance-registry.ts
- src/multiplexer/runtime-sync.ts
- src/instance-directory.test.ts
- src/dashboard/session-registry.ts
- src/multiplexer/index.ts
- src/paths.ts
- src/debug-state.ts
- src/dashboard/index.ts
- src/multiplexer/runtime-state.ts
- src/multiplexer/runtime-lifecycle-methods.ts
- src/multiplexer/runtime-state.test.ts
- src/multiplexer/session-launch.test.ts
Summary
Verification
Notes
Summary by CodeRabbit
New Features
Bug Fixes / Tests
Documentation
Chores