Skip to content

fix: declare unique_constraint/3 on audit_log pkey in update_and_log / log paths (OPS-4604)#62

Closed
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4604-add-unique-constraint-audit-pkey
Closed

fix: declare unique_constraint/3 on audit_log pkey in update_and_log / log paths (OPS-4604)#62
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4604-add-unique-constraint-audit-pkey

Conversation

@palantir-valiot

Copy link
Copy Markdown

Description

update_and_log (and the shared log_changes / log_changes_alone helpers used by all *_and_log and log* entrypoints) performed a raw repo.insert/1 of an audit_log (or ${table_name}) row without declaring a unique_constraint/3 for the primary key. On pkey collisions (sequence rewind, concurrent writers, or test fixtures that reset sequences) this surfaces as an unhandled Ecto.ConstraintError instead of a changeset error:

(Ecto.ConstraintError) constraint error when attempting to insert struct:

    * "audit_logs_pkey" (unique_constraint)
...
    (ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:435: EctoTrail.log_changes/5
    (ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:315: anonymous fn/4 in EctoTrail.update_and_log/4

Summary of change

  • Added Changeset.unique_constraint(:id, name: audit_log_pkey_constraint()) immediately before the two repo.insert/1 calls that create audit rows (log_changes/5 and log_changes_alone/6).
  • Added a tiny private helper audit_log_pkey_constraint/0 that computes the constraint name from the configured table name (defaults to "audit_log_pkey"; supports the :table_name config override).
  • Added a regression test under describe "update_and_log/3" that forces a pkey collision by rewinding the sequence and asserts that update_and_log still succeeds (the existing error path already swallows the resulting changeset error and only logs it).
  • Bumped version 1.0.31.0.4 and added a concise changelog entry.

Files changed:

  • lib/ecto_trail/ecto_trail.ex
  • test/unit/ecto_trail_test.exs
  • mix.exs
  • CHANGELOG.md

Why

See Linear OPS-4604. The triage diagnosis was definitive: the call stack points straight at the two blind inserts in first-party ecto_trail. The fix is the exact mitigation the error message itself recommends (unique_constraint/3 on the changeset with the constraint :name).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • mix format --check-formatted (clean)
  • mix compile (clean, only pre-existing redundant-clause warning)
  • New TDD test written first; it exercises the exact update_and_log -> log_changes -> insert path that was raising.
  • In a real Postgres environment the new test forces a pkey collision via ALTER SEQUENCE … RESTART WITH and asserts the public API does not raise Ecto.ConstraintError.
  • mix test and mix credo --strict are the repo's prescribed checks (per .travis.yml / historical CI); both would be run by CI on the PR. The agent environment has no reachable Postgres, so full test execution is blocked by DB connectivity only (expected and non-blocking for this change).

Test Configuration (agent pod):

  • Elixir 1.20 / OTP 26 (via mise)
  • No production impact; change is additive and only affects the audit-log insert changeset.

Checklist:

  • My code follows the style guidelines of this project (mix format)
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (CHANGELOG)
  • My changes generate no new warnings (pre-existing warning untouched)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (modulo DB availability in this runner)
  • Version bumped and CHANGELOG updated per repo conventions

Closes OPS-4604

…og paths to prevent Ecto.ConstraintError on collision (OPS-4604)
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4604

@acrogenesis

Copy link
Copy Markdown
Member

Closing as a duplicate of #24 — same bug: Ecto.ConstraintError on audit_logs_pkey in EctoTrail.log_changes/5. These were generated from a backlog of duplicate Linear issues created by a log-agent dedup gap (now fixed in palantir 38438d6; no new duplicates are being filed). Consolidating on #24.

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