Skip to content

feat(api): ticket merge — a moved external ref records its origin ticket (#10120) - #10287

Open
lennonflima wants to merge 2 commits into
LanternOps:mainfrom
lennonflima:feature/10120-moved-ref-origin
Open

lennonflima wants to merge 2 commits into
LanternOps:mainfrom
lennonflima:feature/10120-moved-ref-origin

Conversation

@lennonflima

Copy link
Copy Markdown
Contributor

Summary

  • When a ticket merge moves a tombstone's ticket_external_refs row onto the survivor, the row now records the ticket it came from in a new origin_ticket_id column. This is the design accepted on Ticket merge: record the origin ticket on a moved ticket_external_refs row #10120.
  • The ticket importer uses that column. A source id whose ref was moved off one of this principal's imported tombstones now gets the answer merged_in_breeze, the same answer as a ref that is still on the tombstone. Before, it was refused as live_ticket / other_principal_ticket on every pass. This is the follow-up the issue mentions. It is included here because it is about 10 lines and only needs code that is already on main (W0).

Linked Issues

Closes #10120

Refs #8972, #8313, #10034

Merge Danger

Door: two-way. The migration only adds a nullable column and its FK. It writes no rows and does no backfill.

Blast radius: ticket merge (the moved-ref UPDATE), Partner API ref upsert, ticket import resume answer.

Type of Change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • CI / build / tooling
  • Other:

Changes

Migration apps/api/migrations/2027-01-08-120000-ticket-external-refs-origin-ticket.sql

  • Adds ticket_external_refs.origin_ticket_id uuid, a nullable plain FK to tickets(id) with ON DELETE SET NULL. It has the same shape as ticket_comments.merged_from_ticket_id in 2027-01-04-110000-ticket-merge.sql.
  • It is not composite with org_id, so it does not need to be DEFERRABLE.
  • It has no index, following the precedent of merged_from_ticket_id.
  • It is idempotent: ADD COLUMN IF NOT EXISTS and DROP CONSTRAINT IF EXISTS / ADD. It has no inner BEGIN/COMMIT and writes no rows.

Export policy classification: ticket_external_refs.origin_ticket_id is classified included, as one ticket id. The ticket_external_refs line in CORE_TENANT_EXPORT_POLICY is updated.

Other registrations:

  • The table is already in the org cascade, org merge and both org movers.
  • The new FK is SET NULL on a nullable column, so orgCascadeFkOnDelete classifies it as safe without needing a ledger entry.

services/ticketMerge/repoint.ts

  • The moved-ref UPDATE sets origin_ticket_id = COALESCE(origin_ticket_id, <source>). A ref moved A→B→C therefore keeps A, the way comments keep merged_from_ticket_id.
  • A ref that stays on the tombstone (same-principal collision) keeps NULL. So does a ref written directly.

services/ticketExternalRefs.ts (upsertTicketExternalRef)

  • The issue does not name this case, so please confirm it.
  • The Partner API's upsert conflicts on (principal, ticket_id). It can therefore change the external_id of a moved ref that now sits on the survivor.
  • If the external_id stays the same, the upsert keeps origin_ticket_id. If it changes, the upsert sets origin_ticket_id to NULL, because the new external id was written directly. Without this, the importer could answer merged_in_breeze for an id that never belonged to the tombstone.

services/ticketImport/importWriter.ts

  • When the plan reads the ref and it has an origin_ticket_id, the plan reads that origin ticket in the window's context.
  • If the origin is a tombstone that this principal imported, the result is merged_in_breeze with that tombstone and its survivor. Nothing is written or charged.
  • Otherwise the existing checks run unchanged. This covers an origin hidden by RLS, another principal's import, and a live ticket.
  • A ref moved before this column existed has no origin, so it keeps the conflict answer. No backfill is needed.
  • The "known limit" in the docblock is updated to match.

Testing

  • Existing tests pass
  • Added new tests
  • Manual testing

New tests:

  • ticketMerge.integration.test.ts (external refs case) checks that:
    • a moved ref gets origin_ticket_id = source;
    • a collided ref and a ref written directly on the survivor stay NULL;
    • a second merge B→C keeps the earliest origin;
    • the Partner API upsert keeps the origin for the same external id and clears it for a new one.
  • ticketMergeSchema.integration.test.ts: hard-deleting the origin ticket sets origin_ticket_id to NULL.
  • ticketImport.integration.test.ts (merged-in-Breeze case):
    • a ref moved onto a live ticket is now merged_in_breeze with its origin;
    • a moved ref with no origin (pre-column) is still refused as other_principal_ticket;
    • nothing is written or charged.
  • importWriter.flow.test.ts:
    • a moved ref whose origin is this principal's tombstone is skipped;
    • a moved ref is refused when its origin is another principal's tombstone, a live tombstone, or hidden from the window.

Commands (Node 22.23.2, local per-worktree test stack):

  • cd apps/api && NODE_OPTIONS=--max-old-space-size=11264 npx tsc --build tsconfig.src.json tsconfig.tests.json: clean. It hit an out-of-memory error at a 9216 MB heap on this machine, so I used 11264.
  • npx vitest run src/services/ticketMerge src/services/ticketImport src/db/migration src/services/ticketExternalRefs src/services/tenantExport src/services/ticketChildWriters src/routes/partnerApi --maxWorkers 2: 64 files, 3476 tests passed.
  • pnpm test-stack up, then npx vitest run --config vitest.integration.config.ts ticketMerge ticketImport.integration partnerApiTicketMerge portalTicketMerge ticketExternalRefsOrgMove ticketChildLockOrder partnerApiTickets --maxWorkers 2: 13 files, 164 tests passed.
  • npx vitest run --config vitest.integration.config.ts ascade rasure erge xport enant FkOnDelete rls --maxWorkers 2 (the cascade / erasure / merge / export / tenant / FK-on-delete / RLS contract filters): 215 files, 2207 tests passed.
  • DATABASE_URL=<test stack> pnpm db:check-drift: no drift, all 1258 migration files match the ledger. I also re-applied the migration a second time by hand to check idempotency: the column add is skipped with a NOTICE, and the constraint is dropped and re-added with no error.

Checklist

  • My code follows the project's code style
  • I have reviewed my own changes
  • I have tested on the relevant platforms
  • No secrets, credentials, or personal data included
  • If this PR touches pages/settings/** or a *Settings* component: n/a

🤖 Generated with Claude Code

https://claude.ai/code/session_01F8mQQSv7pUuBL2gR5wsd9M

lennonflima and others added 2 commits October 26, 2026 18:57
…came off (LanternOps#10120)

ticket_external_refs.origin_ticket_id: a plain FK to tickets ON DELETE SET
NULL, the shape of ticket_comments.merged_from_ticket_id. Set only by the
moved-ref UPDATE in ticketMerge/repoint.ts, COALESCEd so A→B→C keeps A;
NULL for a ref written directly, no backfill. The Partner API upsert keeps
it for the same external id and drops it when the external id changes.
Export policy: included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8mQQSv7pUuBL2gR5wsd9M
…ze from its origin (LanternOps#10120)

When the principal's ref names an origin_ticket_id that is this principal's
imported tombstone, the source id is skipped as merged_in_breeze with that
tombstone and its survivor, as for a ref still on the tombstone. A moved ref
with no origin (moved before the column) keeps the conflict answer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8mQQSv7pUuBL2gR5wsd9M
@strix-security

strix-security Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Strix Security Review

✅ No open security findings.

PR summary: Adds `ticket_external_refs.origin_ticket_id` so a ticket merge records which tombstone a moved external ref came from, and teaches the ticket importer to answer `merged_in_breeze` for a source id whose ref was moved off one of the same principal's imported tombstones.

Review summary

This PR adds a nullable origin_ticket_id column to ticket_external_refs, stamps it from the moved-ref merge UPDATE, preserves or clears it in the Partner API upsert, and consumes it in the ticket importer so a ref a merge moved onto a survivor answers merged_in_breeze instead of a 409. I traced every writer of the new column (merge repoint is same-org by construction and enforced both in eligibility checks and by the composite merged-into FK; the upsert's insert path never sets it and its conflict branch only preserves/clears; no request field reaches it), the importer's origin read (executed inside the window's partner-scope DB access context, so tickets FORCE RLS bounds it to the window's org set, and the branch additionally requires the origin to be a tombstone from this principal's own import), and the export-policy classification (origin can only name a ticket in the ref's own org). The new branch is strictly additive before the existing refusals and returns before any transaction, so it cannot write, charge, or grant access. Only non-security observations remain: missing explicit window-org assertion that RLS already enforces, a single-column rather than composite FK whose same-org guarantee is indirect, and migration lock hygiene.

Ruled out (3)
  • Can the new origin branch of the importer answer merged_in_breeze for a tombstone outside the window because it skips the explicit org-in-window check?
    The origin read always runs in the window's partner-scope access context, where tickets FORCE RLS and breeze_has_org_access bound visibility to exactly the window's org set, making the app-level check redundant and the branch write- and charge-free.
  • Does the new migration's validating foreign key without lock_timeout expose a blocking or lock-queueing risk at deploy time?
    The validated column is brand new and entirely NULL, so the scan covers only the small child table, does not block plain ticket reads, and this is deployment lock hygiene rather than an attacker-reachable security issue.
  • Can a cross-org origin be recorded, or a stale/intermediate survivor be reported after a second merge?
    Merge is same-org both in eligibility checks and via the composite merged-into FK with no unmerge path, org movers carry tombstone and refs together, and path compression makes the origin always name the final live survivor in the same transaction.

Review 1 on this PR · b81e7bd · requested Oct 26 22:49 UTC when the PR was opened


Reviewed by Strix
Re-run review · Configure security review settings

This branch has not been deployed

No deployments
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.

Ticket merge: record the origin ticket on a moved ticket_external_refs row

1 participant