fix(agents): accept root writer tokens with integration grants - #4496
Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 39 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughRoot run event-writer tokens now allow up to 32 KiB. Child tokens retain the 4 KiB limit. Tests cover a 4,657-character root token and rejection above 32 KiB. ChangesRoot token validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The root-token limit is enforced at 32 KiB while child tokens remain limited to 4 KiB, with no unresolved merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
|
|
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
Review score: 88/100 — good, minor suggestions
Focused, well-tested bug fix for a real production regression (4,657-byte root token rejected at the old 4 KiB cap).
Strengths:
- Root cause is clear and the fix is narrowly scoped: only the root writer-token check (
createHostedRunEventWriterCapability) gets the raised 32 KiB ceiling. I tracedparseRunEventToken(used for the child-token exchange response) and confirmed it still callsisValidRunEventWriterToken(token)with the default 4 KiB limit — no accidental widening of the child path. - Test changes are precise: the old oversize-rejection test now asserts on the correct new boundary (
32 * 1024 + 1) instead of the stale5_000, and a new test locks in the real-world 4,657-byte payload from the staging regression. - PR description is exemplary — includes the exact error, a red/green validation summary, and a link to the tracking issue.
- No auth/scope semantics changed, just a size ceiling — low blast radius.
- Inline comment on the new constant explains why 32 KiB (integration grants), not just what.
Minor suggestions (non-blocking):
- No test asserts acceptance exactly at the new 32 KiB boundary (only well under it at 4,657 bytes, and rejection at 32 KiB + 1). Worth adding for symmetry with the existing boundary-style tests in this file.
- 32 KiB is still a fixed magic number chosen to cover the observed worst case (Gmail+Outlook+ServiceNow grants) rather than derived from a hard upper bound on integration-grant size — fine for now, but if the number of default integrations grows this could need revisiting again. Not a blocker.
- PR description notes the API-side fix is "already deployed" and issue closure is pending a retest — worth double-checking CI finishes green and the linked issue is updated once this merges.
No security or design concerns beyond the note above.
Generated by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 011838f6d2
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a4891e13
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
1 participant
Default chat fails before executing any tool when its valid integration grant makes the root run-event writer token larger than 4 KiB. A staging project with Gmail, Outlook and ServiceNow produced a 4,657-byte token and failed with
Unable to initialize durable child event persistence.Allow up to 32 KiB for the verified root writer credential, which carries integration grants. Keep child-token exchange at 4 KiB and its response body at 16 KiB. No token scopes or authorization checks change.
Related: https://github.com/veryfront/veryfront-issue-inbox/issues/1358#issuecomment-5660218175
Validation:
The API grant/tool/auth-park fix is already deployed. After this framework change publishes, the Agent pin and deployed attachment acceptance retest will follow; issue closure waits for that evidence.
Summary by CodeRabbit
New Features
Tests