chore(release): prepare 1.5.2#201
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds optional NERVE_PUBLIC_ORIGIN config; documents Kanban assigned-task execution using real child sessions (create/send) with parent reporting; updates origin-related troubleshooting and deployment docs; bumps package to 1.5.2; refactors SessionContext delayed-refresh/timeouts and updates tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Kanban as Kanban Service
participant Gateway as Gateway RPC
participant AssigneeRoot as Assignee Root Session
participant Child as Child Session
participant Store as Run Store
Kanban->>Gateway: sessions.create(parentSessionKey=AssigneeRoot)
Gateway-->>Child: create child session (childSessionKey)
Kanban->>Gateway: sessions.send(childSessionKey, task payload)
Gateway->>Child: deliver task message
Child->>Gateway: execute task -> on completion/failure send report
Gateway->>AssigneeRoot: sessions.send(parentSessionKey, completionReport/failureReport)
Gateway->>Store: store.completeRun(correlationKey, run result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/CONFIGURATION.md (1)
129-130: Consider tightening the troubleshooting note.The troubleshooting guidance is accurate but could be more concise:
Suggested refinement
-If remote workspace panels (Files, Memory, Config, Skills) fail with `origin not allowed` while chat still works, set `NERVE_PUBLIC_ORIGIN` to the exact browser origin and add that same origin to `gateway.controlUi.allowedOrigins` on the gateway. +If remote workspace panels fail with `origin not allowed` errors, set `NERVE_PUBLIC_ORIGIN` to your browser's origin (e.g., `https://nerve.example.com`) and add the same origin to `gateway.controlUi.allowedOrigins`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CONFIGURATION.md` around lines 129 - 130, The troubleshooting note is wordy; please shorten it to a concise single sentence that tells users to set NERVE_PUBLIC_ORIGIN to their exact browser origin and add that same origin to gateway.controlUi.allowedOrigins if remote workspace panels show "origin not allowed" while chat still works; reference the config keys NERVE_PUBLIC_ORIGIN and gateway.controlUi.allowedOrigins in the revised sentence for clarity.docs/API.md (1)
740-743: Consider clarifying the execution path description.The documentation accurately describes the new child-session execution flow, but the sentence structure on line 740 is quite dense. Consider breaking it into separate sentences for better readability:
Suggested clarification
-**Execution paths:** -- **Assigned tasks** create a real child session beneath the assignee's live root. Nerve verifies that the parent root exists, creates the child with `sessions.create(parentSessionKey=...)`, then sends the task into that child with `sessions.send`. +**Execution paths:** +- **Assigned tasks** create a real child session beneath the assignee's live root. + - Nerve first verifies that the parent root exists + - Creates the child session via `sessions.create(parentSessionKey=...)` + - Dispatches the task into that child via `sessions.send`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/API.md` around lines 740 - 743, The sentence describing the child-session execution flow is too dense; edit the documentation paragraph that talks about the "child-session execution flow" (the content block in the diff) by splitting the long sentence into two or three shorter sentences or a short bullet list that separates the key steps (what triggers a child-session, how control is passed, and the expected outcome), keep wording concise and preserve the original technical details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/API.md`:
- Around line 740-743: The sentence describing the child-session execution flow
is too dense; edit the documentation paragraph that talks about the
"child-session execution flow" (the content block in the diff) by splitting the
long sentence into two or three shorter sentences or a short bullet list that
separates the key steps (what triggers a child-session, how control is passed,
and the expected outcome), keep wording concise and preserve the original
technical details.
In `@docs/CONFIGURATION.md`:
- Around line 129-130: The troubleshooting note is wordy; please shorten it to a
concise single sentence that tells users to set NERVE_PUBLIC_ORIGIN to their
exact browser origin and add that same origin to
gateway.controlUi.allowedOrigins if remote workspace panels show "origin not
allowed" while chat still works; reference the config keys NERVE_PUBLIC_ORIGIN
and gateway.controlUi.allowedOrigins in the revised sentence for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24649a16-2d60-49cc-9fbb-1f5d0288ad79
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.env.exampleCHANGELOG.mddocs/API.mddocs/ARCHITECTURE.mddocs/CONFIGURATION.mddocs/DEPLOYMENT-C.mddocs/INSTALL.mddocs/TROUBLESHOOTING.mdpackage.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contexts/SessionContext.tsx (1)
667-683:⚠️ Potential issue | 🟠 MajorDon't let delayed refreshes outlive the current gateway.
doneTimeoutsRefis safe to keep until unmount, butdelayedRefreshTimeoutRefnow survivesrpc/connection changes even though its queued callback closes overrefreshSessions. Achat.finalevent scheduled right before reconnect can still callsessions.liston the old gateway 1.5s later, then overwritesessions/currentSessionafter the new connection is active. Please either store the latest refresh callback in a ref (likerpcRefin GatewayContext) or cancel this timeout wheneverrefreshSessionschanges. A quick repro ischat.final→ swaprpc/connectionStatebefore 1.5s elapses → assert no stale refresh wins afterward.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/contexts/SessionContext.tsx` around lines 667 - 683, The delayedRefreshTimeoutRef timeout can outlive gateway changes and call the stale refreshSessions closure; fix by ensuring the delayed timeout is cleared whenever refreshSessions (or the gateway/rpc) changes or by storing the latest refreshSessions callback in a ref and using that ref from the scheduled callback. Concretely, update the effect that currently has an empty dependency array to either include refreshSessions (or rpc/gateway/rpcRef) so its cleanup clears delayedRefreshTimeoutRef when refreshSessions changes, or add a refreshSessionsRef (analogous to rpcRef) and use refreshSessionsRef.current inside the scheduled callback so the latest refresh is executed; ensure delayedRefreshTimeoutRef is cleared in the cleanup and reset to null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/contexts/SessionContext.tsx`:
- Around line 667-683: The delayedRefreshTimeoutRef timeout can outlive gateway
changes and call the stale refreshSessions closure; fix by ensuring the delayed
timeout is cleared whenever refreshSessions (or the gateway/rpc) changes or by
storing the latest refreshSessions callback in a ref and using that ref from the
scheduled callback. Concretely, update the effect that currently has an empty
dependency array to either include refreshSessions (or rpc/gateway/rpcRef) so
its cleanup clears delayedRefreshTimeoutRef when refreshSessions changes, or add
a refreshSessionsRef (analogous to rpcRef) and use refreshSessionsRef.current
inside the scheduled callback so the latest refresh is executed; ensure
delayedRefreshTimeoutRef is cleared in the cleanup and reset to null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 349ebec5-0189-4eb4-9090-802d0646a809
📒 Files selected for processing (2)
src/contexts/SessionContext.test.tsxsrc/contexts/SessionContext.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/contexts/SessionContext.tsx`:
- Around line 500-503: The fallback setTimeout in updateSessionFromEvent uses
the stale refreshSessions closure; change any setTimeout callbacks that call
refreshSessions (e.g. setTimeout(() => refreshSessions(), 100)) to call the
ref-backed function refreshSessionsRef.current() instead so delayed refreshes
always use the latest callback; update both the 100ms fallback in
updateSessionFromEvent and the other delayed refresh path around the
scheduleDelayedRefresh logic (lines ~542-550) to route through
refreshSessionsRef.current().
🪄 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
Run ID: bd0e3ca9-0faf-4767-a23c-30e1255fcd71
📒 Files selected for processing (2)
src/contexts/SessionContext.test.tsxsrc/contexts/SessionContext.tsx
| const refreshSessionsRef = useRef(refreshSessions); | ||
| useEffect(() => { | ||
| refreshSessionsRef.current = refreshSessions; | ||
| }, [refreshSessions]); |
There was a problem hiding this comment.
Route every delayed refresh through refreshSessionsRef.
The new ref-backed path only covers scheduleDelayedRefresh(). The 100ms fallback in updateSessionFromEvent still does setTimeout(() => refreshSessions(), 100) at Line 512, so a gateway/rpc swap inside that window can still hit the stale callback. Reusing refreshSessionsRef.current() there keeps both delayed refresh paths consistent.
Possible fix
const updateSessionFromEvent = useCallback((sessionKey: string, updates: Partial<Session>) => {
setSessions(prev => {
const idx = prev.findIndex(s => getSessionKey(s) === sessionKey);
if (idx === -1) {
// New session appeared that we don't have - schedule a refresh
// Use setTimeout to avoid calling during render
- setTimeout(() => refreshSessions(), 100);
+ setTimeout(() => {
+ void refreshSessionsRef.current();
+ }, 100);
return prev;
}
@@
- }, [refreshSessions]);
+ }, []);Also applies to: 542-550
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/contexts/SessionContext.tsx` around lines 500 - 503, The fallback
setTimeout in updateSessionFromEvent uses the stale refreshSessions closure;
change any setTimeout callbacks that call refreshSessions (e.g. setTimeout(() =>
refreshSessions(), 100)) to call the ref-backed function
refreshSessionsRef.current() instead so delayed refreshes always use the latest
callback; update both the 100ms fallback in updateSessionFromEvent and the other
delayed refresh path around the scheduleDelayedRefresh logic (lines ~542-550) to
route through refreshSessionsRef.current().
Prepare the 1.5.2 release.
Summary
package.jsonandpackage-lock.jsonto1.5.21.5.2changelog entry covering the post-1.5.1release deltaNERVE_PUBLIC_ORIGINfor remote-workspace gateway RPC fallback and remote deploymentsValidation
git diff --checknpm run buildRelease checklist
1.5.2v1.5.2v1.5.2Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests
Chores