Skip to content

chore(review): track prompt inbox identity foundation chain - #1464

Open
dnlrsls wants to merge 91 commits into
mainfrom
feat/prompt-inbox-foundation-tracker
Open

dnlrsls wants to merge 91 commits into
mainfrom
feat/prompt-inbox-foundation-tracker

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

🔗 Linked Issue

Closes #1458. This tracker is the sole PR to main for the completed foundation chain; children #1465, #1466, and #1469 were merged into this branch in order, not into main.

🏷️ PR Type

  • type:feature — New feature

📝 Summary

📂 Changes

File Change
internal/store/store.go, internal/store/store_test.go, internal/store/export_project_query_test.go Store identity, sync/backup, deletion tombstones and regressions.
internal/server/server.go, internal/server/server_test.go HTTP replay and deleted-key 409 response without write notification.
docs/ARCHITECTURE.md Identity and deletion contract.

🧪 Test Plan

  • Focused regressions: each child PR recorded RED→GREEN cases and affected package tests, including retries, legacy schema, import and pulled deletion.
  • Affected package tests: F3 final head passed CODEX_HOME='' go test ./internal/store ./internal/sync ./internal/server -count=1 and complete CODEX_HOME='' go test ./... -count=1 locally; F1/F2 recorded their independent tests.
  • Other checks: F3 final diff-scoped lint reported 0 issues and diff check passed; child heads received fresh CI and substantive CodeRabbit full review. Native per-commit reviews approved and acknowledged.
  • Fresh tracker-head CI and substantive review on 37b7f07e after fix(store): guard prompt owner during deletes and identity adoption #1505: pending; do not enter main merge queue until verified. Prior tracker head 79a5d0fa passed CI but full review found two bugs corrected in fix(store): preserve prompt deletion ownership across sparse sync #1492.

🤖 Automated Checks

Prior integrated head 79a5d0fa passed CI and substantive CodeRabbit review identified sparse project backup and malformed pulled delete. #1492 corrected these and full backup roundtrips; its final head passed CI and full review with no new inline finding. Fresh tracker CI and substantive review on 880f6b47 remain pending.

✅ Contributor Checklist

💬 Notes for Reviewers

The combined tracker diff is 2,264 lines because it accumulates three separately reviewed and size-authorized slices plus separately reviewed 234-, 50-, 147-, 48-, and 279-line integration corrections. Please review child PRs for the bounded units and this tracker for integration only. Merge order was #1465 → #1466 → #1469 → #1492 → #1495 → #1496 → #1504 → #1505. Do not merge #1240 as part of this PR.

Chain Context

Field Value
Chain Durable prompt inbox identity (#1458)
Tracker PR #1464 (this PR)
Position Final integration after 3 of 3 child slices
Base main
Depends on #1465, #1466, #1469, #1492, #1495, #1496, #1504, #1505 — all merged into tracker in order
Follow-up #1240 separately by normal merge after foundation reaches main
Review budget Integrated 2,264 lines; F1 318, F2 567, F3 667 (exceptions explicitly authorized), corrections 234 + 50 + 147 + 48 + 279
Starts at main baseline
Ends with Prompt identity foundation on main via merge queue

Chain Overview

main ← 📍 #1464 integrated tracker ← #1505 owner/adoption guard ← #1504 project guard ← #1496 tombstone guard ← #1495 quarantine correction ← #1492 correction ← #1469 F3 ← #1466 F2 ← #1465 F1

Scope

Summary by CodeRabbit

  • New Features
    • Prompt identities are preserved across synchronization, backups, and imports, including after prompts are deleted.
    • Replaying a prompt with the same identity, session, and project returns the existing prompt without creating a duplicate.
  • Bug Fixes
    • Reusing a deleted prompt identity returns a conflict. Using an identity with a different project is rejected.
    • Conflicting prompt deletions received through synchronization are quarantined, while conflicting backup imports are rejected.
    • Older payloads and legacy deletions without prompt identities remain supported.

Final integration follow-up

Full tracker review on 37b7f07e found malformed pulled deletes could stall the sync cursor. #1495 quarantines that identity with raw dead-letter evidence instead of silently discarding it, and passed its CI, native review, and substantive full CodeRabbit review (no new actionable comments). Tracker 880f6b47 needs fresh CI and substantive review before entering the main merge queue.

Tombstone identity follow-up

The substantive tracker review of 880f6b47 identified a retained medium risk: a conflicting pulled delete or import could rebind an established tombstone key. #1496 prevents rebinding, quarantines pulled conflicts and rejects conflicting imports before deleting matched prompts. Its 147-line head passed CI, native review, and a full CodeRabbit review with minimal merge risk and no architecture-level retained concern. Fresh integrated CI and full review on 70e5a45b are required before queue entry.

Project ownership follow-up

The full integrated review of 70e5a45b found that a same-identity repeat import could rebind the tombstone to a different project, losing it in the original scoped backup. #1504 rejects this established-project conflict and verifies scoped export → restore → replay protection. Its head passed full CI and substantive full CodeRabbit review with minimal merge risk. Native review start remained unavailable because the consent binding expired repeatedly before lineage creation; no native approval is claimed. Fresh integrated CI and full review on c5926826 required before merge queue entry.

Owner and adoption follow-up

The full tracker review of c5926826 identified an import-adoption sync journal gap and two wrong-owner delete paths. #1505 prevents cross-project backup deletion, quarantines spoofed inbox pairs on pulled deletes, and queues a canonical follow-up mutation on identity adoption so acknowledged/in-flight older upserts cannot erase delivery. Its final 279-line head passed CI and substantive full CodeRabbit review with minimal merge risk and no retained architecture concern. Fresh integrated CI and review of 890b96ce remain required.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 123606d6-b904-4a8b-948d-857957d588b9

📥 Commits

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

📒 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; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change adds optional source_inbox_id identity to prompt storage and HTTP writes. It carries that identity through sync and backups, and preserves it in deletion tombstones. Replays reuse matching prompts, while deleted identities are rejected.

Changes

Prompt Inbox Identity

Layer / File(s) Summary
Identity storage and local writes
internal/store/store.go, internal/server/server.go, internal/server/server_test.go, internal/store/store_test.go, docs/ARCHITECTURE.md
Prompt rows and writes carry source_inbox_id. Matching replays return the existing prompt, deleted identities are rejected, and the HTTP handler notifies only when a prompt is inserted. Tests cover replay, project mismatch, and deleted-identity responses.
Deletion tombstones and backup import
internal/store/store.go, internal/store/store_test.go
Prompt tombstones retain inbox identity through deletion and backup export and import. Import restores tombstones before prompts, skips deleted identities, and checks identity and project conflicts.
Sync identity propagation and validation
internal/store/store.go, internal/store/export_project_query_test.go, internal/store/store_test.go
Sync payloads, mutation repair, and backfill carry inbox identity. Pulled upserts and deletes preserve it, check tombstones, and reject conflicting ownership.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant handleAddPrompt
  participant Store
  participant WriteNotification
  Client->>handleAddPrompt: Submit prompt with source_inbox_id
  handleAddPrompt->>Store: AddPromptWithResult
  Store-->>handleAddPrompt: Prompt ID and insertion status
  alt New prompt inserted
    handleAddPrompt->>WriteNotification: Notify write
  else Existing prompt replayed
    handleAddPrompt-->>Client: Return existing prompt response
  end
  handleAddPrompt-->>Client: Return HTTP response
Loading

Suggested reviewers: gentleman-programming, alan-thegentleman

Merge Risk: 🟡 Moderate · up to ca797

A delete or backup from another project can reserve an inbox identity and prevent a legitimate prompt from being restored or written. Resolve the identity-authority policy before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to ca797

Deleted prompt identities can now block later writes across several data paths. The review found paths that can record a deletion identity without establishing its ownership, while the new cloud authority registry is not yet connected to those paths. External exploitability is not fully established.

Retained concerns

  • Medium · security · inferred: An orphan pulled delete can establish a session/inbox tombstone without proving ownership of that pair. Because later admission checks the pair without project, an admitted unverified delete could prevent a legitimate write or pulled upsert. Actual cross-project reachability depends on upstream cloud admission, which is not established here.
  • Medium · security · observed: Backup import checks project ownership for matched prompts but records an unmatched, caller-supplied keyed tombstone without that check. An imported orphan identity can therefore block later prompts for the same session/inbox pair. The HTTP import path is token-gated when a token is configured, limiting who can submit this input.
Security review details

Security Blast Radius

  • inferred — The independently affected unit is a session/inbox pair in a local store, including later writes and pulled upserts. Cross-project cloud exposure cannot be quantified without the upstream admission and deployment boundary.

Security Findings and Attack Paths

  • inferred — A party able to submit an orphan keyed backup tombstone, or an admitted pulled delete with such an identity, can cause a persistent pair-level rejection or skip. This does not establish that an unauthenticated remote party can submit either input.

Trust Boundaries and Controls

  • observed — HTTP backup import requires a token if configured, but defaults to open access when none is set. The cloud registration API validates nonblank fields and delegates actor authentication and project authorization to its caller; the documented server-enforced admission operations are not implemented routes.

Resilience and Maintainability Implications

  • observed — The ownership check, deletion, and tombstone recording for matched backup prompts occur within the import transaction, so a detected conflict rolls back that import. The unmatched-tombstone path does not perform the ownership check.

Hardening Proposals

  • proposed — Before an orphan keyed delete becomes an authoritative pair reservation, require evidence binding its session, inbox ID, sync ID, and project to authorized ownership; keep unverified or out-of-order evidence distinguishable during retry and recovery.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR adds cloud_session_authority, its migration, RegisterSessionAuthority, GetSessionAuthority, and dedicated tests. These APIs are not required by [#1458], and no prompt, sync, import, or de… Remove or split the cloud_session_authority migration, APIs, and tests into a separately scoped change. Keep this PR limited to the [#1458] store, server, sync, backup, tombstone, test, and documentation changes.
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 12 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: tracking prompt inbox identity as foundational review work. It is concise and related to the store, server, sync, backup, and deletion changes.
Linked Issues check ✅ Passed The changes satisfy the coding requirements in [#1458]. Store and /prompts writes carry optional source_inbox_id, deduplicate repeated nonempty (session_id, source_inbox_id) writes, and retain a…
Full details: Out of Scope Changes check

Explanation

The PR adds cloud_session_authority, its migration, RegisterSessionAuthority, GetSessionAuthority, and dedicated tests. These APIs are not required by [#1458], and no prompt, sync, import, or delete path uses them. The provenance RFC describes this authority work as a separate proposed implementation slice and states that current cloud behavior does not enforce it. This is a separate cloud authorization feature, not supporting verification for the linked issue's implemented identity flow.

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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:chore Maintenance/tooling label Sep 26, 2026
@dnlrsls
dnlrsls marked this pull request as ready for review September 26, 2026 21:04
@dnlrsls

dnlrsls commented Sep 26, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Alan-TheGentleman added a commit that referenced this pull request Sep 29, 2026
Capture OpenCode 2.x prompts from user items of session.inbox.enqueued and
send the inboxID as source_inbox_id, so a server with prompt inbox identity
support (#1464) treats replays as no-ops, keeps equal-text items distinct,
and refuses deleted identities. V1 chat.message and V2 share one capture
helper; the identity-less V2 prompt hook is no longer used for capture.
Adds a real-server regression for replay, restart, and delete.
feat(cloudserver): authorize explicit prompt source attestations
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.

feat(store): preserve admitted prompt identity through sync and deletion

1 participant