Skip to content

Adopt a conflict-resistant strategy for session DB migrations from parallel branches #3968

Description

@aheritier

Problem

The session-store migration ledger (pkg/session/migrations.go) assigns each migration a sequential integer ID and a unique name, and is protected by a content-pinned digest test (pkg/session/migrations_pinned_test.go) that fails the build if an existing entry's fields change. That guards against accidental edits within a single branch, but it does nothing to prevent two independently developed branches from each appending a different migration under the same next ID (or the same name). Whichever branch merges last silently collides with a database that already recorded the other branch's migration under that ID.

Today, sqlitestore.New reacts to most migration failures — including this kind of ledger conflict — by moving the existing database aside to <path>.bak and starting fresh (pkg/session/sqlitestore/sqlitestore.go). For a session database this is a destructive, silent data-loss path: the user's history is preserved only by accident, in a .bak file most users don't know to look for or how to recover.

Desired invariants

  • Migration identity is globally conflict-resistant, not just sequential-and-pinned per branch.
  • Migrations apply in a well-defined, dependency-safe order even when authored on parallel branches.
  • The catalogue and the on-disk ledger are validated against each other exactly (no silent partial application, no implicit normalization of drifted descriptions).
  • Migration failures caused by an identity conflict fail closed — never an automatic reset/backup-and-recreate — since that discards session history.
  • Recovery from a genuine conflict has a defined, documented procedure instead of being left to ad hoc manual repair.

Options to evaluate (not selecting one yet)

  1. Merge-time reservation/registry — a checked-in table or CI check that reserves the next migration ID/name at PR-merge time, so collisions are caught before merge rather than at runtime.
  2. Timestamp/UUID/content-addressed identities — replace or augment the sequential integer with an identity that two branches can't independently collide on.
  3. Dependency graph — express migrations as a DAG instead of a flat sequence, so ordering doesn't depend on both branches agreeing on the same next integer.

Acceptance criteria (for whichever option is chosen)

  • CI catches an identity conflict between two branches before merge, or the runtime detects and fails closed on one without resetting the database.
  • Tests cover: two branches introducing conflicting IDs/names, a dependency-graph cycle (if applicable), and the existing append-only content-pin behavior continuing to work.
  • A documented upgrade path for existing on-disk databases that already used the old sequential scheme.
  • Backward compatibility: older binaries encountering a newer scheme still fail closed with a clear error (as ErrNewerDatabase does today), never silently truncate or reset data.
  • Documentation (troubleshooting guide, session docs) describes the new conflict-detection/recovery behavior.

See also #3021 for related prior discussion.

Activity

  1. added
    area/sessionsFor features/issues/fixes related to session lifecycle (resume, persistence, export)
    on Aug 12, 2026
  2. self-assigned this
    on Aug 12, 2026
  3. added
    status/needs-designRequires architectural discussion or design review
    area/coreCore agent runtime, session management
    on Aug 12, 2026
  4. added
    area/ciCI/CD workflows and pipeline
    area/testingTest infrastructure, CI/CD, test runners, evaluation
    on Aug 12, 2026
  5. rumpl commented on Aug 13, 2026

    @rumpl
    Member

    My 2 cents: you can define the database it uses with a flag, while developing you should use it. I would actually like us to switch too goose or some other migration tool and stop using a bespoke code for this

  6. aheritier commented on Aug 13, 2026

    @aheritier
    CollaboratorAuthor

    yes, it is what I realized (too late). In fact I didn't know that I would need to introduce migration and moreover that my branch could stay alive a long time enough to conflict with another migration applied on main. We'll need to see together how we can improve the situation but I don't consider it's a critical improvement to do. It's a risk only for developers and we can take care of using a dedicated DB during our developments

  7. aheritier commented on Aug 18, 2026

    @aheritier
    CollaboratorAuthor

    Not sure if we should address this one @Sayt-0

  8. lmilbaum commented on Oct 11, 2026

    @lmilbaum

    Is this issue still relevant?

  9. aheritier commented on Oct 13, 2026

    @aheritier
    CollaboratorAuthor

    technically it is but it's something impacting mostly docker agent developers and not classical users thus the impact is limited and the priority very low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area/ciCI/CD workflows and pipelinearea/coreCore agent runtime, session managementarea/sessionsFor features/issues/fixes related to session lifecycle (resume, persistence, export)area/testingTest infrastructure, CI/CD, test runners, evaluationstatus/needs-designRequires architectural discussion or design review

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions