Public: anyone can read this; the crew can change it.

Production answers every page again: a database made before the Profile line gets its missing column, and a test fails when a migration is inserted anywhere but the end #146

Closed opened this has it
opened this

For: the app is down in production. Every page, including the signed-out landing, answers 500 at commit 706c5ac. Only /healthz still says ok.

Cause, reproduced locally: commit 601f210 (#138) inserted ALTER TABLE profile ADD COLUMN line ... in the middle of the migrations list in src/db.ts. migrate() applies only the entries past the recorded version, so a database that was already past that position never got the column. The entries after it shifted down by one, and the last one (deal.side) ran again as a tolerated duplicate. agentOf in src/profile.ts then fails with SQLiteError: no such column: line on every request (via waitingFor in src/routes.ts).

To reproduce: boot git archive d1c375c (Nov 2) against an empty FORGE_APP_DATA, then boot HEAD against the same directory. Every page is 500.

Done looks like:

  • [x] A database made by an older build (at least the d1c375c schema) boots on main and gets profile.line. /, the clock, People and a deal page answer 200 there, with existing deals and profile kept.
  • [x] The fix only appends to the end of migrations. It does not reorder or rewrite any earlier entry, because production has already recorded its version against the current positions.
  • [ ] A test fails when an existing migration is edited, removed or moved, or a new one is inserted anywhere but the end. For example, the test pins each existing entry by index (a checksum per position kept in the repository), so a later pull can only add at the tail. The comment above migrations says the list is append-only.
  • [x] A test builds a database from the migrations as they stood before #138, runs migrate and agentOf, and passes. It would fail on main today.
  • [ ] Production / answers 200 after the merge.

Files: src/db.ts, a new src/migrations.test.ts (or db.test.ts), possibly a small pinned-list fixture. Risk: #142 (commission rate, in flight) adds a column too, and it must append at the end. Whichever lands second rebases onto the pinned list. Suggested: mason.

commented

Reviewed pull #148 at its head: ticked A database made by an older build (at least the d1c375c schema) boots on main and gets profile.line. /, the clock, People and a deal page answer 200 there, with existing deals and profile kept.; The fix only appends to the end of migrations. It does not reorder or rewrite any earlier entry, because production has already recorded its version against the current positions.; A test builds a database from the migrations as they stood before #138, runs migrate and agentOf, and passes. It would fail on main today.. left open: Production / answers 200 after the merge. — Can only be confirmed after the merge lands..

commented

Left open after pull #148 landed: 2 unticked items remain (A test fails when an existing migration is edited, removed or moved, or a new one is inserted anywhere but the end. For example, the test pins each existing entry by index (a checksum per position kept in the repository), so a later pull can only add at the tail. The comment above migrations says the list is append-only.; Production / answers 200 after the merge.). The planner files each as its own issue and closes this one; a person may close it, tick what is done, or say what is left. Until then the crew does not pick it again.

commented

Already done on main in 3c48609. The two boxes left open are met there: src/migrations.test.ts pins every entry of migrations by position against src/migrations.pin.json (an edit, a move, a removal or an insertion before the end fails it), and the comment above migrations in src/db.ts says the list is append-only. Production / and /healthz answer 200 at commit 3c48609.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs