Skip to content

fix(sharing): preserve inviter policy during acceptance#1345

Merged
kunickiaj merged 1 commit into
mainfrom
07-22-fix_sharing_preserve_inviter_policy_on_acceptance
Jul 22, 2026
Merged

fix(sharing): preserve inviter policy during acceptance#1345
kunickiaj merged 1 commit into
mainfrom
07-22-fix_sharing_preserve_inviter_policy_on_acceptance

Conversation

@kunickiaj

@kunickiaj kunickiaj commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Description

Fixes the direct-Project acceptance and recipient-authority interaction discovered by the expanded project-sharing E2E.

  • persists reviewed inviter Identity/device/Project intent atomically with recipient intent
  • retains only the inviter devices already recorded by the reviewed operation; source-scope bystanders are never inherited
  • keeps migration-created compatible intent rows unchanged while rejecting cross-Identity or revoked bindings
  • preserves exact replay idempotency, local inviter validation, lifecycle state, trust, and no-Team-membership semantics
  • ensures recipient-policy reconciliation sees the complete owner-and-recipient graph before it can revoke or grant access

Type of Change

  • 🚀 Feature (new functionality)
  • 🐛 Bug fix (fixes an issue)
  • 📚 Documentation (docs-only change)
  • 🔧 Maintenance (refactor, chore, CI, etc.)
  • 🧪 Testing (test-only changes)

Testing

  • Relevant checks pass locally (pnpm run tsc, pnpm run lint, pnpm run test)
  • Added/updated tests for changes
  • Manually verified changes work as expected

Validation:

  • focused onboarding/share-operation/viewer tests: 272 passed
  • pnpm run check on the completed stack: 3,019 passed, 3 todo
  • project-sharing E2E proves inviter retention plus exact existing/future recipient delivery
  • independent security/correctness and maintainability reviews: no remaining blockers

Checklist

  • Code follows project style (pnpm run lint passes for touched files)
  • Self-review completed
  • Documentation updated (not needed; the approved design and ADR already define this invariant)
  • No new warnings introduced

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f904594ac1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/core/src/recipient-policy-onboarding.ts
@kunickiaj
kunickiaj force-pushed the 07-22-feat_sharing_add_recipient_onboarding_journeys branch from d2fa61f to 05eae62 Compare July 22, 2026 18:29
@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from f904594 to 287f691 Compare July 22, 2026 18:29
@kunickiaj
kunickiaj force-pushed the 07-22-feat_sharing_add_recipient_onboarding_journeys branch from 05eae62 to de48cb5 Compare July 22, 2026 19:37
@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from 287f691 to 20d70c6 Compare July 22, 2026 19:37
@kunickiaj
kunickiaj force-pushed the 07-22-feat_sharing_add_recipient_onboarding_journeys branch from de48cb5 to d043ba1 Compare July 22, 2026 19:55
@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from 20d70c6 to 12809f0 Compare July 22, 2026 19:55
@kunickiaj
kunickiaj force-pushed the 07-22-feat_sharing_add_recipient_onboarding_journeys branch from d043ba1 to e77a38e Compare July 22, 2026 21:17
@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from 12809f0 to 1498df3 Compare July 22, 2026 21:17

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1498df31ca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +688 to +691
const hasRecipientKeyBinding =
keyMatch.provenance === "recipient_invite" && expected.provenance === "recipient_invite";
const hasConflictingFingerprint =
hasRecipientKeyBinding && keyMatch.source_fingerprint !== expected.source_fingerprint;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject unkeyed inviter rows for recipient bindings

When the existing row was first projected by a direct-share acceptance as an inviter device (provenance = 'exact_project_invite'), its source_fingerprint is based only on identity/device and does not include the device public key. A later recipient/team/add-device onboarding for the same identity and device but a different devicePublicKey reaches this branch with expected.provenance = 'recipient_invite'; because hasRecipientKeyBinding is false, the code returns success without checking or storing the key fingerprint. Fresh evidence in this version is that commitDirectProjectSharePolicyInTransaction now creates those exact_project_invite device rows, so a projected inviter device can bypass the key-binding conflict path.

Useful? React with 👍 / 👎.

kunickiaj commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Merge activity

  • Jul 22, 9:25 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 22, 9:56 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 22, 9:58 PM UTC: Graphite couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'TypeScript Test').
  • Jul 22, 10:17 PM UTC: @kunickiaj merged this pull request with Graphite.

@kunickiaj
kunickiaj changed the base branch from 07-22-feat_sharing_add_recipient_onboarding_journeys to graphite-base/1345 July 22, 2026 21:46
@kunickiaj
kunickiaj changed the base branch from graphite-base/1345 to main July 22, 2026 21:54
@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from 1498df3 to 587ad58 Compare July 22, 2026 21:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 587ad5885a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

deviceId,
displayName: inviterDeviceDisplayName(db, deviceId),
}));
commitDirectProjectSharePolicyInTransaction(db, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Classify policy-commit conflicts as terminal

When automatic maintenance advances a waiting share whose coordinator payload has been accepted, this new in-transaction policy commit can throw local conflicts such as device_binding_conflict, intent_conflict, or inviter_identity_conflict. Those errors are not in TERMINAL_RECONCILIATION_ERRORS, so after the transaction rolls back the operation is still waiting_for_acceptance and advancePendingProjectShares records a retry with operation_read_failed instead of moving the invite to needs_attention; conflicting accepted invites will keep retrying every cooldown rather than surfacing the actionable local-policy conflict. Please classify or map these new terminal errors before they escape reconciliation.

Useful? React with 👍 / 👎.

@kunickiaj
kunickiaj force-pushed the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch from 587ad58 to 40c8009 Compare July 22, 2026 22:13
@kunickiaj
kunickiaj merged commit 0137d0d into main Jul 22, 2026
11 checks passed
@kunickiaj
kunickiaj deleted the 07-22-fix_sharing_preserve_inviter_policy_on_acceptance branch July 22, 2026 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant