internal/adapters/store/migrate_test.go exercises individual migrations by stepping a fixed number of times down from HEAD and back up (e.g. m.Steps(-1) assumed to be down-014). The step counts assume a specific latest migration. Adding a new migration shifts every count by one, so unrelated tests go red with confusing off-by-one symptoms.
Observed when adding migration 015 (ADR-021): TestMigration010/011/014 failed with messages like channel_title must be gone after the down migration, default is FALSE after down, login_events must be gone — none of which were actually about 015.
Fixed forward in that commit by inserting a leading Steps(-1) and bumping the restore Steps(+N) in each affected test, but this recurs on every new migration.
Suggested fix
Make the tests robust to new migrations on top:
Capture the current version, step down to (target-1) by version rather than by a hard-coded count, assert, then Up() back to HEAD.
Or add a small helper that migrates to a named target version and back, so a new migration above the target needs no test edits.
Why it matters
Every migration author currently has to edit 3 unrelated tests; the failures look like real regressions. Low priority, but it is a predictable papercut.
Filed via gitea REST API (gitea-mcp per-repo tools are down — see gitea-mcp issue 36).
## Problem
`internal/adapters/store/migrate_test.go` exercises individual migrations by stepping a fixed number of times down from HEAD and back up (e.g. `m.Steps(-1)` assumed to be down-014). The step counts assume a specific latest migration. Adding a new migration shifts every count by one, so unrelated tests go red with confusing off-by-one symptoms.
Observed when adding migration 015 (ADR-021): `TestMigration010/011/014` failed with messages like `channel_title must be gone after the down migration`, `default is FALSE after down`, `login_events must be gone` — none of which were actually about 015.
Fixed forward in that commit by inserting a leading `Steps(-1)` and bumping the restore `Steps(+N)` in each affected test, but this recurs on every new migration.
## Suggested fix
Make the tests robust to new migrations on top:
- Capture the current version, step down to (target-1) by version rather than by a hard-coded count, assert, then `Up()` back to HEAD.
- Or add a small helper that migrates to a named target version and back, so a new migration above the target needs no test edits.
## Why it matters
Every migration author currently has to edit 3 unrelated tests; the failures look like real regressions. Low priority, but it is a predictable papercut.
Filed via gitea REST API (gitea-mcp per-repo tools are down — see gitea-mcp issue 36).
This is the koala-session counterpart to tapir#20 (which ran on flamingo) — same shakedown intent, different task, so we can compare host/session behavior rather than confound it with task-specific luck. tapir#20 found two things worth checking again here: (1) direct-push vs PR — tapir's CLAUDE.md/ADR-009 says commit directly to main for solo/agent work, confirm that still applies; (2) the first report-back attempt there was thin (no real build/test output, no verdict tag) until re-prompted — this time, please match the format below on the first attempt.
When done, comment on this issue with:
Branch + PR link, or confirm direct-push-to-main per repo policy (state which, and why)
Actual go test output for the affected package(s) — real output, not "pass"
Anything that didn't go as the issue described — friction, wrong assumption, anything not stated that you had to figure out (e.g. tapir#20 hit a Linux-vs-macOS sed -i difference — that class of thing)
No brain capture needed unless something genuinely durable comes up.
## Report-back (required)
This is the koala-session counterpart to tapir#20 (which ran on flamingo) — same shakedown intent, different task, so we can compare host/session behavior rather than confound it with task-specific luck. tapir#20 found two things worth checking again here: (1) direct-push vs PR — tapir's `CLAUDE.md`/ADR-009 says commit directly to `main` for solo/agent work, confirm that still applies; (2) the first report-back attempt there was thin (no real build/test output, no verdict tag) until re-prompted — this time, please match the format below on the first attempt.
When done, comment on this issue with:
- **Branch + PR link**, or confirm direct-push-to-main per repo policy (state which, and why)
- **Actual `go test` output** for the affected package(s) — real output, not "pass"
- **Anything that didn't go as the issue described** — friction, wrong assumption, anything not stated that you had to figure out (e.g. tapir#20 hit a Linux-vs-macOS `sed -i` difference — that class of thing)
- **One-line verdict:** `clean-cut / needed-intervention / blocked-on-<X>`
No brain capture needed unless something genuinely durable comes up.
Direct-push to main — per ADR-009 / CLAUDE.md (solo/agent work commits directly to main, no PR). No other agent active on this repo. Commit 72cb251, tag v0.15.2, pushed with --follow-tags.
Fix:internal/adapters/store/migrate_test.go now addresses migrations by version number via two helpers — headVersion(t, m) (captures HEAD from m.Version()) and migrateTo(t, m, version) (m.Migrate(version), ErrNoChange tolerated). Each test steps to target-1, asserts the down effect, steps to target, asserts the up effect, then restores to the captured HEAD. Adding a migration on top shifts no count → no test edits. All four per-step Steps(±N)-from-HEAD chains removed.
Robustness proof (what the issue asked for): added a throwaway migration 017 on top → all four tests stayed green with zero edits; removed it, git status clean.
Actual go test output:
=== RUN TestMigration010LoginEventsUpDown
--- PASS: TestMigration010LoginEventsUpDown (0.05s)
=== RUN TestMigration011AutoSummarizeDefaultUpDown
--- PASS: TestMigration011AutoSummarizeDefaultUpDown (0.04s)
=== RUN TestMigration014VideoChannelTitleUpDown
--- PASS: TestMigration014VideoChannelTitleUpDown (0.03s)
=== RUN TestMigration012FixAutoSummarizeRLS
--- PASS: TestMigration012FixAutoSummarizeRLS (0.03s)
ok git.d-ma.be/mathias/tapir/internal/adapters/store 1.365s
Full task check green (fmt, vet, lint 0 issues, go test -p 1 ./... all ok).
Didn't go as described / friction:
The old tests also asserted the target survived each intermediate down step (e.g. "login_events intact after down 016/015/..."). Version-addressing drops that per-step granularity — you jump straight to target-1. Deliberate trade: those intermediate assertions were the brittle part, and the core "migration N is reversible" claim is fully preserved. Noted so it's a choice, not an accidental loss of coverage.
Doc drift: CLAUDE.md says last tag v0.15.0; actual was v0.15.1 (→ bumped to v0.15.2). Bumped off the real git describe, not the doc.
Updated the brain failure note tapir-migration-and-rls-test-infra-gotchas — its section 1 said "fix forward, edit 3 tests" which is now obsolete; rewrote it to describe the version-addressed pattern so future migration authors don't reintroduce Steps(±N).
Verdict:clean-cut
## Report-back
**Direct-push to `main`** — per ADR-009 / CLAUDE.md (solo/agent work commits directly to main, no PR). No other agent active on this repo. Commit `72cb251`, tag `v0.15.2`, pushed with `--follow-tags`.
**Fix:** `internal/adapters/store/migrate_test.go` now addresses migrations by *version number* via two helpers — `headVersion(t, m)` (captures HEAD from `m.Version()`) and `migrateTo(t, m, version)` (`m.Migrate(version)`, ErrNoChange tolerated). Each test steps to `target-1`, asserts the down effect, steps to `target`, asserts the up effect, then restores to the captured HEAD. Adding a migration on top shifts no count → no test edits. All four per-step `Steps(±N)`-from-HEAD chains removed.
**Robustness proof** (what the issue asked for): added a throwaway migration `017` on top → all four tests stayed green with **zero edits**; removed it, `git status` clean.
**Actual `go test` output:**
```
=== RUN TestMigration010LoginEventsUpDown
--- PASS: TestMigration010LoginEventsUpDown (0.05s)
=== RUN TestMigration011AutoSummarizeDefaultUpDown
--- PASS: TestMigration011AutoSummarizeDefaultUpDown (0.04s)
=== RUN TestMigration014VideoChannelTitleUpDown
--- PASS: TestMigration014VideoChannelTitleUpDown (0.03s)
=== RUN TestMigration012FixAutoSummarizeRLS
--- PASS: TestMigration012FixAutoSummarizeRLS (0.03s)
ok git.d-ma.be/mathias/tapir/internal/adapters/store 1.365s
```
Full `task check` green (fmt, vet, lint 0 issues, `go test -p 1 ./...` all ok).
**Didn't go as described / friction:**
- The old tests also asserted the target survived *each* intermediate down step (e.g. "login_events intact after down 016/015/..."). Version-addressing drops that per-step granularity — you jump straight to `target-1`. Deliberate trade: those intermediate assertions *were* the brittle part, and the core "migration N is reversible" claim is fully preserved. Noted so it's a choice, not an accidental loss of coverage.
- Doc drift: `CLAUDE.md` says last tag `v0.15.0`; actual was `v0.15.1` (→ bumped to `v0.15.2`). Bumped off the real `git describe`, not the doc.
- Updated the brain failure note `tapir-migration-and-rls-test-infra-gotchas` — its section 1 said "fix forward, edit 3 tests" which is now obsolete; rewrote it to describe the version-addressed pattern so future migration authors don't reintroduce `Steps(±N)`.
**Verdict:** `clean-cut`
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
internal/adapters/store/migrate_test.goexercises individual migrations by stepping a fixed number of times down from HEAD and back up (e.g.m.Steps(-1)assumed to be down-014). The step counts assume a specific latest migration. Adding a new migration shifts every count by one, so unrelated tests go red with confusing off-by-one symptoms.Observed when adding migration 015 (ADR-021):
TestMigration010/011/014failed with messages likechannel_title must be gone after the down migration,default is FALSE after down,login_events must be gone— none of which were actually about 015.Fixed forward in that commit by inserting a leading
Steps(-1)and bumping the restoreSteps(+N)in each affected test, but this recurs on every new migration.Suggested fix
Make the tests robust to new migrations on top:
Up()back to HEAD.Why it matters
Every migration author currently has to edit 3 unrelated tests; the failures look like real regressions. Low priority, but it is a predictable papercut.
Filed via gitea REST API (gitea-mcp per-repo tools are down — see gitea-mcp issue 36).
Report-back (required)
This is the koala-session counterpart to tapir#20 (which ran on flamingo) — same shakedown intent, different task, so we can compare host/session behavior rather than confound it with task-specific luck. tapir#20 found two things worth checking again here: (1) direct-push vs PR — tapir's
CLAUDE.md/ADR-009 says commit directly tomainfor solo/agent work, confirm that still applies; (2) the first report-back attempt there was thin (no real build/test output, no verdict tag) until re-prompted — this time, please match the format below on the first attempt.When done, comment on this issue with:
go testoutput for the affected package(s) — real output, not "pass"sed -idifference — that class of thing)clean-cut / needed-intervention / blocked-on-<X>No brain capture needed unless something genuinely durable comes up.
Report-back
Direct-push to
main— per ADR-009 / CLAUDE.md (solo/agent work commits directly to main, no PR). No other agent active on this repo. Commit72cb251, tagv0.15.2, pushed with--follow-tags.Fix:
internal/adapters/store/migrate_test.gonow addresses migrations by version number via two helpers —headVersion(t, m)(captures HEAD fromm.Version()) andmigrateTo(t, m, version)(m.Migrate(version), ErrNoChange tolerated). Each test steps totarget-1, asserts the down effect, steps totarget, asserts the up effect, then restores to the captured HEAD. Adding a migration on top shifts no count → no test edits. All four per-stepSteps(±N)-from-HEAD chains removed.Robustness proof (what the issue asked for): added a throwaway migration
017on top → all four tests stayed green with zero edits; removed it,git statusclean.Actual
go testoutput:Full
task checkgreen (fmt, vet, lint 0 issues,go test -p 1 ./...all ok).Didn't go as described / friction:
target-1. Deliberate trade: those intermediate assertions were the brittle part, and the core "migration N is reversible" claim is fully preserved. Noted so it's a choice, not an accidental loss of coverage.CLAUDE.mdsays last tagv0.15.0; actual wasv0.15.1(→ bumped tov0.15.2). Bumped off the realgit describe, not the doc.tapir-migration-and-rls-test-infra-gotchas— its section 1 said "fix forward, edit 3 tests" which is now obsolete; rewrote it to describe the version-addressed pattern so future migration authors don't reintroduceSteps(±N).Verdict:
clean-cut