feat(SUP-2219): add journal-backed session thread - #67
Conversation
|
Warning Review limit reached
More reviews will be available in 14 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds session thread persistence to the jarvos-agent-context module, enabling agents to maintain and retrieve checkpoint-style entries from structured Markdown notes. Two new MCP tools— ChangesSession Thread Feature
Sequence DiagramsequenceDiagram
participant Host
participant writeSessionThread
participant readSessionThread
participant NotesStorage
Host->>writeSessionThread: checkpoint entry (summary, nextStep, decision)
writeSessionThread->>NotesStorage: create/update note with metadata
NotesStorage-->>writeSessionThread: markdown view
writeSessionThread-->>Host: result with markdown
Host->>readSessionThread: thread_key, max_lines
readSessionThread->>NotesStorage: fetch note
NotesStorage-->>readSessionThread: raw content
readSessionThread-->>Host: result with rendered markdown
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 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 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 443c77e66e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ...threadOptions, | ||
| maxChars: Number(options.sessionThreadMaxChars || threadOptions.maxChars || 2200), | ||
| }); | ||
| parts.push('', '# Live Session Thread', '', thread.found ? thread.markdown : 'No live session thread found.'); |
There was a problem hiding this comment.
Fix hydration size accounting when no thread exists
When no session thread exists this still adds a new # Live Session Thread placeholder to every hydration packet, which pushes small maxChars calls into the final trim path; running npm --prefix modules/jarvos-agent-context test now fails the existing budget test because result.report.finalChars no longer matches the rendered markdown length (604 vs 606). Either avoid emitting the absent-thread section or recompute the final size after this extra content participates in trimming so hydration reports stay internally consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@modules/jarvos-agent-context/src/index.js`:
- Around line 374-414: writeSessionThread currently does a read-modify-write via
readIfExists + noteWriter.writeNoteFile which can clobber concurrent
checkpoints; update writeSessionThread (and the callsite of loadNoteWriter /
noteWriter.writeNoteFile) to perform an atomic update with retries: acquire an
exclusive lock on the target (e.g. lockfile based on thread.notePath or
journal), or implement a compare-and-swap loop that reads the current file
(readIfExists/stripFrontmatter), builds the new content/entry, writes to a temp
file and atomically renames into place only if the on-disk content hasn't
changed (or retry up to N times), and fall back to appending if the noteWriter
supports append semantics; ensure readSessionThread uses the final agreed
content and surface an error after max retries. Use the symbols
writeSessionThread, loadNoteWriter, noteWriter.writeNoteFile, readIfExists, and
readSessionThread to locate where to add locking/CAS and retry logic.
🪄 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: 0699d525-5625-416c-a71c-121914c9d71e
📒 Files selected for processing (4)
modules/jarvos-agent-context/README.mdmodules/jarvos-agent-context/scripts/jarvos-mcp.jsmodules/jarvos-agent-context/src/index.jsmodules/jarvos-agent-context/test/agent-context.test.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b50dbd13ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| process.env.JARVOS_SESSION_THREAD_ID, | ||
| process.env.PAPERCLIP_TASK_ID, |
There was a problem hiding this comment.
Prefer the Paperclip task when deriving the thread
The new MCP schemas advertise the implicit default as PAPERCLIP_TASK_ID/JARVOS_SESSION_THREAD_ID/default, but this order checks JARVOS_SESSION_THREAD_ID first. In sessions where a host sets a stable global JARVOS_SESSION_THREAD_ID and Paperclip also provides the current issue in PAPERCLIP_TASK_ID, calls that omit threadId will read and append to the global host thread instead of the issue-specific thread, so handoffs for the current issue are lost in the wrong note. Swap these two env fallbacks or make the tool/schema contract match the intended precedence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c3000c405
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| maxChars: Number(options.sessionThreadMaxChars || threadOptions.maxChars || 2200), | ||
| }); | ||
| if (thread.found) { | ||
| parts.push('', '# Live Session Thread', '', thread.markdown); |
There was a problem hiding this comment.
Avoid hydrating the session thread twice
When today's journal already contains the session-thread wikilink, which writeSessionThread adds on every write, this inserts the live thread here and the later collectLinkedNotes(journal.content, ...) pass reads the same note again under “Notes Linked From Today”. In sessions with an active thread this duplicates the handoff content and burns the hydration budget, which can force other journal-linked notes or ontology context to be trimmed; skip the current thread when collecting linked notes or avoid adding it twice.
Useful? React with 👍 / 👎.
Thinking Path
What Changed
readSessionThreadandwriteSessionThreadto@jarvos/agent-context, backed by@jarvos/secondbrainnote and journal helpers.jarvos_session_thread_readandjarvos_session_thread_write.jarvos_hydratewhen present.Verification
npm --prefix modules/jarvos-agent-context testgit diff --checknpm testRisks
jarvos_hydratenow attempts a session-thread read by default; missing threads are treated as absent and do not fail hydration.Model Used
OpenAI GPT-5.4 / Codex environment, code editing and local test execution.
Checklist
Summary by CodeRabbit
jarvos_session_thread_readandjarvos_session_thread_writefor managing agent session threads.