RATE_MEMORIES: directive names its session id; rating tools accept it - #128
Merged
Conversation
Three tasks: directive names its session id, rating tools accept an explicit session_id (explicit > env > marker), skill and docs pass it through. Fixes the counter/list mismatch diagnosed 2026-08-31. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
memory.list_session_exposures and memory.apply_session_ratings now accept an optional session_id argument that overrides env/marker resolution when non-empty (whitespace-only values fall back to the existing resolution). This lets the RATE_MEMORIES directive pass the session id it already resolved instead of relying on the handler re-deriving it independently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tool.description is typed str | None, so a bare `"session_id" in tool.description` trips pyright's reportOperatorIssue (typecheck.yml runs pyright on tests/**). Add the `is not None` guard already used elsewhere in this file before the two new session_id description checks. Also rename test_tool_schema_has_no_required_properties to test_tool_schema_shape_and_optional_session_id — it now asserts the session_id property's shape too, not just the absence of required fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
This PR adds an explicit optional session_id parameter to the two rating MCP tools and threads it through the Stop-hook directive and skill. The handler logic, schema changes, hook directive construction, and doc updates all match the accompanying tests and plan precisely; no defects were found in the added/modified lines.
No bugs were detected in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the session-id split-brain that made the RATE_MEMORIES Stop-hook directive impossible to satisfy: the hook counts unrated exposures by its stdin-payload session id, while the MCP rating tools resolved a different id (spawn-env var after MCP respawns; per-project marker file that loses last-writer-wins races between concurrent sessions).
session_close: the directive now printsSession: <id>as its second line — the id it actually counted with. Line survives the 8 KB truncation by construction.memory.list_session_exposures/memory.apply_session_ratings: optionalsession_idargument (explicit stripped non-empty > env/marker resolution; whitespace = absent; both sqlite and agentcore branches). Backward compatible — omitted param behaves exactly as before, including apply's ValueError-when-unresolvable contract.rate-session-memoriesskill: reads theSession:line and passes it to both tools, with a no-Session:-line fallback for older hooks.Live-validated mid-development: both stranded unrated pools on the reference machine (7 hook-counted rows + 9 server-side rows under a divergent env id) were rated using the directive-named id through the same service path this PR wires up.
Test plan
🤖 Generated with Claude Code