Conversation
…e_key resolveSessionKey could emit keys up to 140 chars (issue strategy) / 97 (agent), but Hermes forwards them as the Codex/OpenAI prompt_cache_key which caps at 64 — silently losing prompt caching and session continuity. Compact over-length keys to a stable, namespaced sha256 hash so continuity and per-issue isolation are preserved; within-limit keys pass through unchanged. Fixes #8713 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hey @rsaulo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Greptile SummaryThis PR fixes silent prompt-cache loss in the
Confidence Score: 4/5The code change is correct and safe to merge; the only outstanding item is the missing PR template sections (Thinking Path, Model Used, Checklist). The compaction logic is mathematically sound: SHA-256 hex truncated to 48 chars gives a 60-char output that reliably fits the 64-char limit, is stable for the same input, and distinguishes different scopes. Tests cover all the meaningful cases. Existing live sessions re-key once on the next wake, which the PR author correctly calls out as harmless. The only gap is that the PR description doesn't follow the required template format — Thinking Path, Model Used, and the Checklist are absent. No files require special attention; both changed files are straightforward and correct. Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/adapters/hermes/src/gateway/server/execute.ts:159-163
**PR template sections missing**
The PR description doesn't follow the required template from `CONTRIBUTING.md`. Specifically, it's missing:
- A **Thinking Path** in blockquote format tracing from project context down to this change (e.g., `> - Paperclip is… > - The hermes-gateway adapter… > - Session keys up to 140 chars exceed the 64-char prompt_cache_key limit… > - This PR adds compactSessionKey()… > - The benefit is…`)
- A **Model Used** section (the footer says "Generated with Claude Code" but the template requires the provider, model ID/version, and capability details)
- The **Checklist** from the template
Please update the PR description to follow the template at `.github/PULL_REQUEST_TEMPLATE.md`.
Reviews (1): Last reviewed commit: "fix(hermes-gateway): compact over-length..." | Re-trigger Greptile |
| function compactSessionKey(key: string): string { | ||
| if (key.length <= MAX_SESSION_KEY_CHARS) return key; | ||
| const digest = createHash("sha256").update(key).digest("hex").slice(0, 48); | ||
| return `paperclip:h:${digest}`; | ||
| } |
There was a problem hiding this comment.
The PR description doesn't follow the required template from CONTRIBUTING.md. Specifically, it's missing:
- A Thinking Path in blockquote format tracing from project context down to this change (e.g.,
> - Paperclip is… > - The hermes-gateway adapter… > - Session keys up to 140 chars exceed the 64-char prompt_cache_key limit… > - This PR adds compactSessionKey()… > - The benefit is…) - A Model Used section (the footer says "Generated with Claude Code" but the template requires the provider, model ID/version, and capability details)
- The Checklist from the template
Please update the PR description to follow the template at .github/PULL_REQUEST_TEMPLATE.md.
Context Used: Contribution guidelines (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/adapters/hermes/src/gateway/server/execute.ts
Line: 159-163
Comment:
**PR template sections missing**
The PR description doesn't follow the required template from `CONTRIBUTING.md`. Specifically, it's missing:
- A **Thinking Path** in blockquote format tracing from project context down to this change (e.g., `> - Paperclip is… > - The hermes-gateway adapter… > - Session keys up to 140 chars exceed the 64-char prompt_cache_key limit… > - This PR adds compactSessionKey()… > - The benefit is…`)
- A **Model Used** section (the footer says "Generated with Claude Code" but the template requires the provider, model ID/version, and capability details)
- The **Checklist** from the template
Please update the PR description to follow the template at `.github/PULL_REQUEST_TEMPLATE.md`.
**Context Used:** Contribution guidelines ([source](https://app.greptile.com/paperclip-org-3/-/custom-context?memory=a595932a-f6ed-448b-899b-5ccac43f9148))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Thanks for the review! Updated the PR description to follow |
Thinking Path
Linked Issues or Issue Description
Fixes #8713.
Related (add the
hermes_gatewayadapter; neither addresses the key-length issue): #2363, #4359.What Changed
compactSessionKey()inpackages/adapters/hermes/src/gateway/server/execute.ts: when a key exceeds 64 chars, returnpaperclip:h:<first-48-hex-of-sha256>(60 chars); otherwise return the key unchanged.resolveSessionKeyreturn path (agent,run, and theissue/runfallback).resolveSessionKeysuite covering: over-lengthissueandagentkeys compacting to ≤64 and matching^paperclip:h:[0-9a-f]{48}$, stability (same scope → same key), isolation (distinct issues → distinct keys), and pass-through for within-limit keys.Verification
Character counts that motivate the fix (UUID company/agent/issue ids):
issue= 140,agent= 97,run= 50,none= null. After the change all emitted keys are ≤ 64.Risks
Low. Keys already within the limit — including the
runstrategy and short ids — pass through byte-for-byte, so existing behavior is unchanged for them. Only over-length keys change shape; any live session keyed off an over-length value re-keys once on its next wake, which is harmless (a single fresh session, no data loss). No schema migration, no API surface change, no config change.Model Used
claude-opus-4-8)The change was authored and locally verified by the model; a human operator reviewed and approved opening the PR.
Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived details🤖 Generated with Claude Code