Skip to content

feat(cloudstore): persist explicit session ownership claims - #1516

Merged
dnlrsls merged 1 commit into
Gentleman-Programming:feat/prompt-inbox-foundation-trackerfrom
dnlrsls:feat/prompt-session-authority-store
Sep 28, 2026
Merged

dnlrsls merged 1 commit into
Gentleman-Programming:feat/prompt-inbox-foundation-trackerfrom
dnlrsls:feat/prompt-session-authority-store

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

🔗 Linked Issue

Closes #1458

🏷️ PR Type

  • type:feature — New feature

📝 Summary

  • Persist explicit globally unique session-to-owner-project registration, independently of chunk-derived session indexes.
  • Make same-owner replay idempotent and conflicting owner registration immutable under concurrent writes.
  • No public registration route or prompt-delete enforcement in this slice.

📂 Changes

File Change
internal/cloud/cloudstore/cloudstore.go Add additive authority table migration.
internal/cloud/cloudstore/session_authority.go Add explicit register/lookup persistence boundary.
internal/cloud/cloudstore/session_authority_test.go Postgres isolation, replay, conflict, reopen, and concurrency coverage.

🧪 Test Plan

  • Focused: CLOUDSTORE_TEST_DSN=postgres://Blackie@127.0.0.1:55451/engram_test?sslmode=disable go test ./internal/cloud/cloudstore -run ^TestSessionAuthority -count=10 — passed against ephemeral local Postgres; RED before API existed.
  • Affected: same DSN go test ./internal/cloud/cloudstore ./internal/cloud/cloudserver -count=1 — passed.
  • Other: golangci-lint run --new-from-rev=HEAD — zero changed-line findings; git diff --cached --check — passed.
  • GitHub CI full unit/E2E/lint/platform checks — pending on exact PR head.

🤖 Automated Checks

Pending until CI completes on this PR.

✅ Contributor Checklist

💬 Notes for Reviewers

Feature branch chain: main ← tracker #1464 ← 📍 this 178-line persistence slice, after RFC #1515. Subsequent slices add authenticated server registration, client origin/reauthorization and pair/delete enforcement. This table is NOT populated by chunks and MUST NOT be treated as an auth grant until an authorized caller is wired. Rollback boundary is authority table/method/tests only. Native review-5f4a2ba88abbbab7 approved and acknowledged. #1464 remains outside merge queue, #1240 separate.

Summary by CodeRabbit

  • New Features
    • Session ownership can now be recorded and retrieved, including the owning project and registration details.
    • Repeated registrations for the same session and project preserve the original record. Attempts to assign an existing session to a different project are rejected.
    • Session records remain available after the application’s storage is closed and reopened.

@coderabbitai

coderabbitai Bot commented Sep 28, 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: f3112868-0d9e-4212-9650-58929f6deca2

📥 Commits

Reviewing files that changed from the base of the PR and between b2eb348 and eb6766b.

📒 Files selected for processing (3)
  • internal/cloud/cloudstore/cloudstore.go
  • internal/cloud/cloudstore/session_authority.go
  • internal/cloud/cloudstore/session_authority_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

CloudStore now stores session authority records and exposes APIs to register and retrieve them. Registration validates inputs, preserves existing registrations for the same owner, and reports conflicts for a different owner.

Changes

Session authority persistence

Layer / File(s) Summary
Authority storage and APIs
internal/cloud/cloudstore/cloudstore.go, internal/cloud/cloudstore/session_authority.go, internal/cloud/cloudstore/session_authority_test.go
CloudStore creates the authority table and adds registration and lookup APIs. Tests cover validation, same-owner replay, conflicting owners, concurrent registrations, and persistence after reopening the store.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to eb676

The registration test already checks that a conflict leaves the stored authority unchanged. No merge-blocking issue remains from this review.

Security Architecture Review

Security architecture risk: 🔵 Low · up to eb676

The new ownership record is protected against conflicting registrations, and no user-facing registration path is added. Its future security value depends on verifying who is allowed to make a claim before calling the storage API.

Retained concerns

  • Low · security · inferred: The durable ownership claim is not bound to a verified caller identity at the persistence boundary. Any future registration caller must establish authority over the supplied project and session before writing; no attacker-reachable caller is established in this PR.
Security review details

Security Blast Radius

  • inferred — A wrongly authorized registration would persist for that session ID and resist replacement through this API. No route exposing registration to an attacker is established in the current change.

Trust Boundaries and Controls

  • observed — The database primary key and conflict path control concurrent replacement, but neither verifies the registering actor or project entitlement. Those checks remain a requirement of any future caller.

Resilience and Maintainability Implications

  • inferred — A lost response after a successful insert may leave the caller uncertain whether registration committed; repeating the same-owner request can recover the result without replacing the stored registrar. Database fault paths remain untested.

Hardening Proposals

  • proposed — Before connecting registration to a route or ownership-based operation, derive the actor from authenticated identity, verify entitlement to the project and session, and define how pre-existing indexed sessions obtain authority.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #1458 requires optional source_inbox_id prompt writes, replay identity keyed by (session_id, source_inbox_id), identity propagation through sync and export/import, deletion tombstones that b… Implement the #1458 prompt identity contract across store, local HTTP, sync, and export/import. Add deletion tombstone handling, focused regression tests, and the required documentation. If this PR is intended to remain a session-authority …
Out of Scope Changes check ⚠️ Warning The new cloud_session_authority migration, RegisterSessionAuthority, GetSessionAuthority, conflict error, and concurrency tests address explicit session-to-owner-project authority. Issue #1458 a… Remove or separately track the session-authority migration, APIs, and tests under an issue for session ownership authority. Keep this PR limited to changes that support #1458, unless a direct coding dependency is documented and implemented.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 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 main change: adding persistent, explicit session ownership claims to CloudStore.
Full details: Linked Issues check

Explanation

Issue #1458 requires optional source_inbox_id prompt writes, replay identity keyed by (session_id, source_inbox_id), identity propagation through sync and export/import, deletion tombstones that block replay, focused store/server regression tests, and HTTP/deletion documentation. The reviewed change adds cloud_session_authority persistence and session-owner registration/lookup only. It does not implement or test the required prompt-identity behavior.

Resolution

Implement the #1458 prompt identity contract across store, local HTTP, sync, and export/import. Add deletion tombstone handling, focused regression tests, and the required documentation. If this PR is intended to remain a session-authority slice, remove the #1458 link or defer the issue requirements to a PR that implements them.

Full details: Out of Scope Changes check

Explanation

The new cloud_session_authority migration, RegisterSessionAuthority, GetSessionAuthority, conflict error, and concurrency tests address explicit session-to-owner-project authority. Issue #1458 addresses admitted prompt identity, sync, export/import, and deletion. The session-authority changes have no demonstrated connection to those objectives and are outside the linked issue scope.

  • Fix all pre-merge checks with AI
✨ 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 added the type:feature New feature label Sep 28, 2026
@dnlrsls

dnlrsls commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@dnlrsls
dnlrsls merged commit ca79717 into Gentleman-Programming:feat/prompt-inbox-foundation-tracker Sep 28, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant