Skip to content

docs: clarify adapter session identity ownership - #1421

Merged
dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:docs/session-identity-boundary
Sep 25, 2026
Merged

dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:docs/session-identity-boundary

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

🔗 Linked Issue

Closes #1419

Related to #737, which remains the global documentation tracker.


🏷️ PR Type

  • type:bug — Bug fix
  • type:feature — New feature
  • type:question — Question requiring tracked work
  • type:docs — Documentation only
  • type:refactor — Code refactoring (no behavior change)
  • type:chore — Maintenance, dependencies, tooling
  • type:breaking-change — Breaking change

📝 Summary

  • Map Claude Code, Codex, and OpenCode runtime IDs to Engram sessions and state each adapter's current registration handoff behavior.
  • Link to the canonical Go-owned registration, project validation, and omitted-ID selection rules instead of duplicating them.

📂 Changes

File Change
docs/codebase/integrations.md Add a compact runtime session identity table and core/adapter boundary.

🧪 Test Plan

  • Unit tests pass locally: go test ./... (not run; CI owns the broad suite)
  • E2E tests pass locally: go test -tags e2e ./internal/server/... (not run; documentation-only)
  • Lint passes locally: make lint (not run; documentation-only)
  • Manually compared claims to Claude, Codex, and OpenCode adapters, Go session registration and MCP selection, and checked both DOCS.md links.

Focused verification passed:

  • node --test plugin/opencode/engram.test.mjs — 81 passed, 0 failed.
  • go test ./internal/mcp -run 'Test(OmittedSessionIDRejectsAmbiguousActiveSessions|HandleSaveResolvesActiveSessionFromStore)' -count=1 -timeout=90s — passed.
  • git diff --check — passed.

Unverified locally: go test ./internal/store ./internal/server ./internal/mcp ./plugin timed out twice (120s and 600s) without output. It was not retried again; a passing focused test is not a passing broad suite. CI owns the required full unit suite. No runtime behavior changed.


🤖 Automated Checks

CI is pending. All required checks must pass before merge.


✅ Contributor Checklist

  • I linked an approved issue above (Closes #1419)
  • I added exactly one type:* label to this PR
  • I ran unit tests locally: go test ./... (not run; CI owns the broad suite)
  • I ran e2e tests locally: go test -tags e2e ./internal/server/... (not run; documentation-only)
  • I ran lint locally: make lint (not run; documentation-only)
  • Docs updated
  • Commits follow conventional commits format
  • No Co-Authored-By trailers in commits
  • I checked every changed path against the Transient Artifact Policy (docs/codebase/integrations.md only)

💬 Notes for Reviewers

Review the adapter mapping table and registration acknowledgment column first. This describes current behavior, including OpenCode's lack of matching-ID validation on HTTP-success registration replies; it does not implement a fix. End-session semantics from open PR #1250 and archive lifecycle are intentionally out of scope.

Summary by CodeRabbit

  • Documentation
    • Added guidance on how Claude Code, Codex, and OpenCode map host session IDs to Engram sessions. The guide outlines registration checks before an ID is handed off, including Codex’s response validation and OpenCode’s root-session lookup and ownership recheck.
    • Clarified that root-session translation applies specifically to OpenCode, and that Go handles registration, persistence, validation, and behavior when no session ID is provided.

@dnlrsls dnlrsls added the type:docs Documentation only label Sep 25, 2026
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e9759162-1a81-4a78-8adb-5c2017f23ad2

📥 Commits

Reviewing files that changed from the base of the PR and between 38c45a5 and b5a58b3.

📒 Files selected for processing (1)
  • docs/codebase/integrations.md

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The integrations guide now documents session identity mapping and registration checks for Claude Code, Codex, and OpenCode. It also identifies which session rules Go owns.

Changes

Adapter session identity

Layer / File(s) Summary
Document adapter session identity
docs/codebase/integrations.md
The guide describes each adapter’s session identity source and registration checks. It distinguishes OpenCode root-session translation from the other adapters and points to Go-owned registration, persistence, validation, and omitted-ID cardinality.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: gentleman-programming

Merge Risk: ⚪ Minimal · up to b5a58

This documentation-only change accurately describes the adapters’ session identity and registration behavior, including OpenCode’s acknowledgment limitation. It is ready to merge.

Architecture Summary

Architecture risk: 🔵 Low · up to b5a58

The change affects 1 system.

Changed systems: docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — docs (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in docs/codebase/integrations.md: Added documentation of Claude Code, Codex, and OpenCode session identity mapping and registration gates. Claude Code does not inspect the registration response; Codex hands off the ID only after HTTP 201 with matching id and status: "created", otherwise instructing omission of session_id; OpenCode follows root-session links and blocks injection on failed requests, but does not verify identity in successful responses. The text limits root translation to OpenCode and assigns registration, persistence, validation, and omitted-ID cardinality to Go.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the documentation change about adapter session identity ownership.
Linked Issues check ✅ Passed The change implements the documentation requirements in #1419. It adds the runtime session identity section for Claude Code, Codex, and OpenCode. It documents identity sources, OpenCode root translati…
Out of Scope Changes check ✅ Passed The change is limited to docs/codebase/integrations.md. It documents shipped behavior and does not add runtime code, proposed end-session semantics, or work for #1300, #1192, or unrelated tracker obje…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dnlrsls

dnlrsls commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

@CodeRabbit resume

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
✅ Action performed

Reviews resumed and review finished.

@dnlrsls
dnlrsls added this pull request to the merge queue Sep 25, 2026
Merged via the queue into Gentleman-Programming:main with commit 0359b8d Sep 25, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:docs Documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: clarify adapter session identity ownership

1 participant