Add Codex app startup hydration hook - #31
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 (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR integrates a new hydration workflow into jarvOS that aggregates current work, journal entries, linked notes, and ontology content into a working context packet. The packet is exposed as an MCP tool, CLI command, and Codex session hook, with automatic trust establishment via the Codex app-server. ChangesjarvOS Hydration and Codex Session Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 docstrings
🧪 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: 671b7a6df5
ℹ️ 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".
| const normalized = title.replace(/[\\/]/g, path.sep); | ||
| const direct = path.join(notesDir, `${normalized}.md`); | ||
| if (fs.existsSync(direct)) return direct; |
There was a problem hiding this comment.
When today's journal contains a wikilink with .. path components, this direct path is normalized by path.join and can resolve outside notesDir before readIfExists injects the file into the startup hydration packet. For example, [[../../private/session-notes]] would read a sibling markdown file if it exists, bypassing the intended “notes linked from today” scope and potentially exposing unrelated local content to Codex startup context; reject traversal or verify the resolved path remains under notesDir before reading it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
runtimes/codex/README.md (1)
46-46: ⚡ Quick winMake the budget value more precise.
The phrase "about 12,000 characters" is imprecise for a technical specification. If this is a configurable value, state that it's the default. If it's a hard-coded constant, provide the exact value.
📝 Suggested precision improvement
-Default budget is about 12,000 characters. The packet includes: +Default budget is 12,000 characters (configurable via JARVOS_MAX_HYDRATION_CHARS). The packet includes:or if it's a hard-coded value:
-Default budget is about 12,000 characters. The packet includes: +The packet is capped at 12,000 characters and includes:🤖 Prompt for 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. In `@runtimes/codex/README.md` at line 46, Replace the imprecise phrase "Default budget is about 12,000 characters" in the README with an exact value and clarify whether it is a default or a hard-coded constant: update the sentence that currently reads "Default budget is about 12,000 characters" to either "Default budget: X characters (configurable via <config_key>)" if it's configurable, or "Budget: X characters (hard-coded constant)" if not, and ensure the exact numeric value X matches the implementation where the budget is defined; edit the README line containing "Default budget is about 12,000 characters. The packet includes:" accordingly.
🤖 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 474-487: The current two-stage truncation can still exceed
maxChars if the hydration report itself is too large; add a final safety check
after computing markdown (and after replacing the "Final size" text) that
ensures markdown.length <= maxChars by trimming the body portion (or falling
back to truncating markdown) and pushing an omission like `final packet forcibly
trimmed from X to maxChars chars` into report.omissions; update
report.finalChars to the new length and re-run the `Final size:` replacement so
the final markdown returned by the logic (which uses variables body, maxChars,
renderHydrationReport, report, and markdown) is guaranteed to be <= maxChars.
In `@runtimes/codex/jarvos-session-start-hook.js`:
- Line 31: The environment variable JARVOS_HYDRATION_MAX_CHARS is being passed
through Number(...) directly into hydrate(), which can produce NaN or
non-positive values and silently change behavior; before calling hydrate({
maxChars: ... }) validate and normalize the env: read
process.env.JARVOS_HYDRATION_MAX_CHARS, parse as an integer (e.g., parseInt),
ensure it's a finite positive integer, clamp or reject out-of-range values, and
if invalid fall back to DEFAULT_MAX_CHARS; then call hydrate({ maxChars:
validatedMaxChars }) so hydrate() always receives a safe positive integer.
In `@runtimes/codex/README.md`:
- Line 48: Update the README.md sentence that currently says Paperclip issues
are filtered by `in_progress` and `in_review` to accurately list the statuses
used by the currentWork() filter: `in_progress`, `todo`, and `blocked` (these
are defined by DEFAULT_CURRENT_WORK_STATUSES and used by currentWork()); remove
`in_review` from that description and, if needed, add a short note that
`in_review` is used for hydration filtering rather than current work.
In `@runtimes/codex/setup.sh`:
- Around line 121-134: The current logic removes all SessionStart entries and
inserts only sessionStartLine; change it to upsert only the jarvOS SessionStart
while preserving other user SessionStart hooks: locate the [hooks] block using
the existing start/end detection (variables start, end and array filtered), scan
filtered between start+1 and end for lines matching /^\s*SessionStart\s*=/, if a
SessionStart line already contains the jarvOS command (check for a unique token
from sessionStartLine) replace that line with sessionStartLine; if no jarvOS
SessionStart exists, insert sessionStartLine at the end of the hooks block
(before end) without removing other SessionStart lines. Use the existing
variables filtered, start, end, and sessionStartLine to implement this upsert
behavior.
In `@runtimes/codex/trust-session-start-hook.js`:
- Around line 121-125: The final trust-status check currently treats only
'trusted' as success but earlier logic accepts 'managed' as trusted-enough;
update the condition that calls finish to treat both 'trusted' and 'managed' as
successful by changing the check on hook.trustStatus in the
trust-session-start-hook.js end block (the branch that calls finish) to consider
hook.trustStatus === 'trusted' || hook.trustStatus === 'managed' and call
finish(0) in that case, otherwise call finish(1, hook.trustStatus).
---
Nitpick comments:
In `@runtimes/codex/README.md`:
- Line 46: Replace the imprecise phrase "Default budget is about 12,000
characters" in the README with an exact value and clarify whether it is a
default or a hard-coded constant: update the sentence that currently reads
"Default budget is about 12,000 characters" to either "Default budget: X
characters (configurable via <config_key>)" if it's configurable, or "Budget: X
characters (hard-coded constant)" if not, and ensure the exact numeric value X
matches the implementation where the budget is defined; edit the README line
containing "Default budget is about 12,000 characters. The packet includes:"
accordingly.
🪄 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: 1b3e4fb2-0806-4ad4-865d-5120c915f65d
📒 Files selected for processing (8)
modules/jarvos-agent-context/scripts/jarvos-mcp.jsmodules/jarvos-agent-context/src/index.jsmodules/jarvos-agent-context/test/agent-context.test.jsruntimes/codex/README.mdruntimes/codex/hooks.jsonruntimes/codex/jarvos-session-start-hook.jsruntimes/codex/setup.shruntimes/codex/trust-session-start-hook.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c06f9ec3ac
ℹ️ 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".
| report.finalChars = markdown.length; | ||
| markdown = markdown.replace(/Final size: \d+ chars/, `Final size: ${report.finalChars} chars`); |
There was a problem hiding this comment.
Running npm test in modules/jarvos-agent-context fails on the new budget test because report.finalChars is assigned before the Final size: 0 chars placeholder is replaced. When the replacement increases the string length, the returned metadata stays stale (for example, 604 while result.markdown.length is 606), so the hydration report and consumers relying on report.finalChars get an incorrect size.
Useful? React with 👍 / 👎.
Summary
Verification
npm testinmodules/jarvos-agent-contextpasses: 11/11Notes
Summary by CodeRabbit
New Features
jarvos_hydratetool that generates a "jarvOS Working Context Packet" by aggregating current work items, journal entries, linked notes, and ontology contextDocumentation