Skip to content

Document (and test) that SettlementRepository.peekNextId() is advisory-only and unsafe as a concurrency guard #164

Description

@Jagadeeshftw

📌 Description

SettlementRepository.peekNextId() returns the id that will be assigned to the next create() call, but nothing prevents another create() call from happening between a caller reading peekNextId() and acting on it. In this single-process, synchronous, in-memory store that's not currently a real race (Node's event loop serializes synchronous code), but the method's name and existence invite misuse if the repository were ever backed by something asynchronous (e.g. a real database) in the future.

🧩 Requirements and context

  • Add a doc comment to peekNextId() explicitly stating it must not be used to predict/reserve an id across an awaited boundary, and is safe only because create() is itself synchronous.
  • Add a test demonstrating peekNextId() followed immediately by create() yields the previewed id, to lock in the current (correct, synchronous) guarantee.
  • Add a note to docs/ARCHITECTURE.md (if it exists per the prior seeding round) or a code comment flagging this as a risk if the repository is ever swapped for an async-backed implementation.

🛠️ Suggested execution

  • Add the doc comment to src/repositories/settlementRepository.ts.
  • Add the locking-in test to src/repositories/settlementRepository.test.ts.
  • Add the architectural note wherever the repository-swap-for-persistence intent is already documented (several existing files, e.g. liquidityRepository.ts, mention being "swappable for a persistent... store later").

✅ Acceptance criteria

  • peekNextId() carries an explicit doc-comment warning about its synchronous-only safety guarantee.
  • A test locks in the current correct behavior.
  • A note exists flagging the risk for any future async-backed repository implementation.

🔒 Security notes

N/A — documentation and test clarifying an existing safe assumption, aimed at preventing a future concurrency bug if the storage backend changes.

📋 Guidelines

  • Minimum 95% test coverage
  • Clear documentation
  • Timeframe: 96 hours

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions