Repository navigation
fix(memory): forward-compatible migration stubs for v55-v58 - #8068
DaBlitzStein wants to merge 3 commits into
Conversation
Idempotent stubs so rolling back from a v58 DB to a v54 binary does not fail on schema-version mismatch. Closes librefang#8066
houko
left a comment
There was a problem hiding this comment.
This cannot achieve what #8066 asks for, and merging it would make the four PRs whose schemas it guesses unshippable. Please close it and take the rollback problem head-on instead.
Blocking
1. The stated goal is unreachable from this side of the rollback.
#8066's scenario is: a newer binary applied v55-v58, the operator rolls back to a v54 binary, and that binary refuses to boot. Adding stubs to main changes what main's binary does; it cannot change what the already-shipped v54 binary does. That binary still has SCHEMA_VERSION = 54, still reads user_version = 58, and still hits the guard at migration.rs:16-28 — which is deliberate:
// Refuse to run if the DB was created by a newer binary. Silently
// downgrading `user_version` would corrupt v(N+1)+ columns/indexes.If that policy should change, the change is to the guard (accept user_version > SCHEMA_VERSION when every unknown version is additive, or add a documented --allow-newer-schema escape hatch), not four speculative migrations. Either way it only helps binaries built after the change.
2. Pre-claiming 55-58 makes the real migrations unreachable.
run_step! is if current_version < $version (migration.rs:95-104). Once a DB has run these stubs it sits at user_version = 58, so when #8047 (template/manifest version history), #7991 (session parent lineage) and #7974 (per-task claim deadline) land with their own migrate_v55 / migrate_v56, none of them will ever run on that DB. The feature code then queries a schema that came from a guess in this PR. A silently-diverged schema with no error at any point is the one failure a migration ladder must never allow.
The guesses are already visible: v56 adds task_queue.timeout_secs, while #7974's subject is "per-task priority and claim deadline". If that PR names the column claim_deadline_secs — or stores an absolute timestamp instead of a duration — this leaves a permanent dead column and a missing real one.
The convention this repo already uses is the right one, and the migrate_v54 comment records it (migration.rs:276): each schema change ships in the PR that consumes it and takes the next free number at merge time. The 51-53 contention noted there resolved exactly that way.
3. v57 and v58 are copies of v54 and v55.
migrate_v57 is migrate_v54 line for line — same two ALTER TABLE agents guarded by try_column_exists, same idx_agents_parent_id. migrate_v58 is the first half of the migrate_v55 added in this same diff. Two version numbers are burned to re-run DDL that has already run in the same run_migrations call. Even granting the premise, these two do nothing.
4. manifest_versions breaks the agent-delete cascade — CI is red on it.
assertion `left == right` failed: cascade missed target rows in agent-keyed table 'manifest_versions' (col=agent_id)
— add a `DELETE FROM manifest_versions WHERE agent_id = ?1` line in execute_structured_agent_deletes
(structured::tests::agent_cascade_purges_every_agent_keyed_table, structured.rs:1690)
Deleting an agent would leave its entire manifest history behind — and a manifest carries workspace paths, the env var names holding provider credentials, command allowlists and system-prompt text. That guard exists to stop precisely this, and it is worth noting #7986 ("purge every trace of an agent") is in flight against the same invariant. Any PR creating manifest_versions owns the cascade line, which is a further argument for the table arriving with #8047 rather than here.
5. v56 assumes tables exist that a ladder step must not assume.
conn.execute("CREATE INDEX IF NOT EXISTS idx_sessions_parent ON sessions(parent_session_id) WHERE parent_session_id IS NOT NULL", [])?;IF NOT EXISTS guards the index, not the table: this hard-errors when sessions is absent, and try_column_exists(conn, "task_queue", …) has the same exposure. That is the second CI failure — v54_marks_pre_existing_agent_rows_as_lineage_unknown (migration.rs:4157) builds a pre-v54 fixture holding only agents, memories, group_roster and migrations, and run_migrations(&conn).unwrap() now panics inside v56. migrate_v54 guards every ALTER for exactly this reason; a step that reaches a table it did not create has to check the table too.
If the rollback problem is real
Say what actually happened — which build produced a v55+ DB, and what the operator had to do — and fix it where it lives:
- Relax the guard at
migration.rs:16-28to tolerate a neweruser_versionwhen the binary can prove the extra versions are additive, and cover it with a test that opens a v58 DB withSCHEMA_VERSION = 54and boots. - Or document the supported recovery (restore from backup) and close #8066, since the message at
migration.rs:24-26already tells the operator to do that.
Both are one focused change and neither reserves numbers that belong to other PRs.
|
Closing per review. This approach cannot achieve #8066: a rollback to an already-shipped v54 binary cannot be fixed by changing main's binary, which still has SCHEMA_VERSION=54 and still hits the guard at migration.rs:16-28. Pre-claiming v55–v58 also makes the real migrations in #8047/#7991/#7974 unreachable (run_step! is version-gated), v57/v58 are line-for-line copies of v54/v55, and manifest_versions breaks the agent-delete cascade. The rollback problem should be fixed at the guard itself or documented as restore-from-backup, not via speculative stubs. |
After merging origin/main the branch ladder was 54 -> 55 -> 57 with a hole at 56, and the ladder guards require every version 1..=SCHEMA_VERSION to have a migrations audit row, so the hole fails them. Renumbered all four sites in one commit: run_step!, fn migrate_v57 -> migrate_v56, SCHEMA_VERSION, and the migrations audit row (the easy miss — migrate_v50 recorded itself as 49, librefang#7924/librefang#7925). Landing-order note: librefang#8041 (agents.manifest_versions) and librefang#7991 (sessions.parent_session_id) also claim v56 in their branches; whichever of the three lands second renumbers on its next rebase. Do not fill the hole with a stub migration — librefang#8068 proposed exactly that and should not land. Also updates the stuck-task sweeper doc comment in accessors.rs that referenced the old v57 numbering.
…8148) * docs(operations): document the sanctioned binary-downgrade recovery Issue #8066 asked for forward-compatible migration stubs so an older binary could boot a database a newer binary had already migrated. The stub approach was closed (#8068): a stub claims a ladder version, run_step! skips the real migration behind that number, and the schema diverges silently. The fail-loud guard that fires instead is the fix that already landed upstream (#3962, closing #3656, which documented the pre-guard silent corruption). What was missing was the operator-facing half: what the refusal means and what the supported way back is. Adds docs/operations/downgrade-recovery.md: the guard's verbatim error, why it refuses by design, what the backup feature covers (BACKUP_LAYOUT components including the SQLite databases under data/, minus the -shm index sidecar), the rollback procedure (restore the pre-upgrade archive, then restart on the old binary), the manual unzip fallback for a down daemon, and the honest no-backup options. Carries a changelog fragment under changelog.d/documentation/. * docs(changelog): reference PR 8148 in the downgrade-recovery fragment * docs: make the downgrade rollback an offline restore Review of #8148 flagged an unflagged hazard window between steps 3 and 4 of the sanctioned procedure: `POST /api/restore` ran inside a live daemon holding an open pool on `data/librefang.db`, and the archive carries that database's `-wal` (`is_sqlite_shared_memory_index` excludes the `-shm` and nothing else), so the restore replaced both files under connections still mapped to them and the daemon could checkpoint its stale WAL over the restored database. Promote what was the "if the daemon is down" fallback to the recommended path — stop every daemon process, unzip the archive over the home directory, delete the leftover `-shm`, start the older binary — so no live daemon exists at any point. The endpoint keeps a section of its own explaining why it is the wrong tool here, and what to do when it is the only way in. The old fallback claimed unzipping was "the same write the endpoint performs, minus the `-shm` filtering", which was wrong: `restore_root` also re-roots the `agents/` prefix onto the agent workspaces directory, and extracting it to `<home_dir>/agents/` strands the archived workspaces in the legacy layout. Both corrections are now spelled out, since the manual path is the sanctioned one. Also add the missing trailing newline to the document and its changelog fragment. * docs(operations): fix four wrong assumptions in the downgrade procedure Review of #8148 found the page asserts things that are only true on a default layout, and quotes an error line the daemon never prints. - The whole procedure assumed the SQLite databases sit under `<home_dir>/data/`. Both `data_dir` and `[memory] sqlite_path` move them, while `backup_source` hard-codes `home_dir.join("data")`, so on such a deployment the archive holds no database and the rollback is a no-op that still ends in the version refusal. Added an explicit precondition section and referenced it from step 3. - The quoted guard message was the format string from `migration.rs`, not the journal line: `LibreFangError::memory` and `BootFailed` prefix it with `Memory init failed: Memory error: `. An ops page is used by grepping, so the quote now carries both prefixes and names the substring to search for. - The `-shm` bullet said nothing about a stale `-wal`, which is the file that can silently undo the rollback: `unzip -o` cannot overwrite an entry the archive does not carry, and the newer binary's WAL frames replay page 1 (and `user_version`) back over the restored database. - "Configuration, agent workspaces, skills and workflows survive" was false for `cron_jobs.json`, `hand_state.json` and `custom_models.json`, which `BACKUP_LAYOUT` names separately but which live inside the `data/` tree the "start fresh" option moves aside. They are plain JSON, so the page now says to copy them back out. The changelog fragment advertised "the backup/restore feature the daemon already ships", which reads as `POST /api/restore` — the one path the page argues against. Reworded to name the offline restore. --------- Co-authored-by: Evan <suzukaze.haduki@gmail.com>
After merging origin/main the branch ladder was 54 -> 55 -> 57 with a hole at 56, and the ladder guards require every version 1..=SCHEMA_VERSION to have a migrations audit row, so the hole fails them. Renumbered all four sites in one commit: run_step!, fn migrate_v57 -> migrate_v56, SCHEMA_VERSION, and the migrations audit row (the easy miss — migrate_v50 recorded itself as 49, librefang#7924/librefang#7925). Landing-order note: librefang#8041 (agents.manifest_versions) and librefang#7991 (sessions.parent_session_id) also claim v56 in their branches; whichever of the three lands second renumbers on its next rebase. Do not fill the hole with a stub migration — librefang#8068 proposed exactly that and should not land. Also updates the stuck-task sweeper doc comment in accessors.rs that referenced the old v57 numbering.
After merging origin/main the branch ladder was 54 -> 55 -> 57 with a hole at 56, and the ladder guards require every version 1..=SCHEMA_VERSION to have a migrations audit row, so the hole fails them. Renumbered all four sites in one commit: run_step!, fn migrate_v57 -> migrate_v56, SCHEMA_VERSION, and the migrations audit row (the easy miss — migrate_v50 recorded itself as 49, librefang#7924/librefang#7925). Landing-order note: librefang#8041 (agents.manifest_versions) and librefang#7991 (sessions.parent_session_id) also claim v56 in their branches; whichever of the three lands second renumbers on its next rebase. Do not fill the hole with a stub migration — librefang#8068 proposed exactly that and should not land. Also updates the stuck-task sweeper doc comment in accessors.rs that referenced the old v57 numbering.
…m deadline (#7974) * feat(tasks): validate the assignee and enforce per-task priority and claim deadline - `task_post` now rejects an unknown `assigned_to` with `AgentNotFound` instead of silently storing a row nothing can claim. - `TaskPostOptions { priority, timeout_secs }` struct lets the API surface expose claim-queue ordering and a per-task TTL override. - `priority` orders the claim queue (higher first, ties by age). - `timeout_secs` overrides the global `claim_ttl_secs` for a single row: the stuck-task sweeper now reads `COALESCE(timeout_secs, ?)`. - Schema migration v55 adds the `timeout_secs` column (NULL = global). - Integration tests: unknown assignee → 400, priority round-trip, timeout_secs stored and retrievable, valid assignee succeeds. * chore: regenerate openapi.json, SDK codegen and schema baselines * fix(memory): guard v55 migration against missing task_queue table * fix(memory): give the v54 fixture test a task_queue table for v55 to touch `v54_marks_pre_existing_agent_rows_as_lineage_unknown` hand-rolls a pre-v54 database with only the tables v51/v52 touch. Migration v55 (#task-validation) now ALTERs `task_queue` unconditionally, and that table is normally created by `migrate_v1` — absent here because the fixture starts post-v1 with a partial schema. Add it, matching what a real database at user_version 50 actually has, and cover v55 itself with its own idempotency + populated-board tests. * refactor(memory): drop the table_exists guard from migrate_v55 task_queue is created unconditionally by migrate_v1, so every migration from v2 onward assumes it exists — none of the other run_step! functions check for their base table before ALTERing it. The guard papered over the actual gap, which was a hand-rolled test fixture missing the table (fixed separately); the production code path never needed it. * fix(memory): renumber the task-queue TTL migration to v57 Four open PRs each claimed SCHEMA_VERSION = 55 with different bodies, so on any database that had already run one of them the other three steps never executed and the feature code hit a missing column. The ladder order settled for the four PRs is 55 = #7991 (sessions.parent_session_id), 56 = #8041 (agents.manifest_versions), 57 = this PR (task_queue.timeout_secs), 58 = #8047 (template_versions). The two ladder guards that require every version 1..=user_version to have an audit row fail while 55/56 are held by the pending PRs; they go green once those land and this branch merges main again. Also documents in the task_post validation that a registered but stopped assignee is accepted on purpose, and adds the changelog fragment for the breaking change. * fix(memory): renumber the task-queue TTL migration to v56 After merging origin/main the branch ladder was 54 -> 55 -> 57 with a hole at 56, and the ladder guards require every version 1..=SCHEMA_VERSION to have a migrations audit row, so the hole fails them. Renumbered all four sites in one commit: run_step!, fn migrate_v57 -> migrate_v56, SCHEMA_VERSION, and the migrations audit row (the easy miss — migrate_v50 recorded itself as 49, #7924/#7925). Landing-order note: #8041 (agents.manifest_versions) and #7991 (sessions.parent_session_id) also claim v56 in their branches; whichever of the three lands second renumbers on its next rebase. Do not fill the hole with a stub migration — #8068 proposed exactly that and should not land. Also updates the stuck-task sweeper doc comment in accessors.rs that referenced the old v57 numbering. * fix(runtime): match the TaskQueue::task_post signature in the vision gate test double The merge with origin/main brought the trait's new sixth TaskPostOptions parameter, and the VisionKernel double still declared the five-parameter shape, so cargo clippy --workspace --all-targets failed with E0050 in the runtime's test build. Adds the unused-options parameter to the double; the body already returned a not-used error. * fix(tasks): the claim-ttl docs must tell the truth and the assignee picker must agree with the kernel Two follow-ups from the review, both on files this PR does not itself touch. The documented meaning of `claim_ttl_secs = 0` still read "disables the sweeper entirely" — the opposite of the semantics this PR implements. An operator running a human-in-the-loop board could set 0 on that promise, then post a task with `timeout_secs = 300` for an unrelated reason and watch the sweeper reclaim it from under the human. The struct doc, the field doc, the config reference table, the dashboard help strings in five locales, and the schema golden fixture now all say the same thing as accessors.rs: 0 disables the *global* clock, a task that declared its own timeout still expires. The new-task assignee picker was built from the `assigned_to` values of existing tasks, so it offered deleted agents (selecting one is a 400 under this PR's validation) and fell through to free text on an empty board, where every typo became a 400. It is now sourced from `GET /api/agents`, so the dropdown agrees with the registry the kernel checks. The filter dropdown still derives from the tasks themselves: filtering by a historical assignee is a read over rows that still exist, not a write the kernel can reject. * fix(api): return 400, not 500, for an unknown comms/task assignee POST /api/comms/task shared the task queue with POST /api/tasks but not its validation: it caught every kernel error with a generic 500 scrub and had no arm for AgentNotFound, so posting a task to an assignee that does not exist crashed the request instead of naming the offending field, the way the /api/tasks sibling route already does. * fix(memory): stop flooring the stuck-task sweep's claim age to a whole second task_reset_stuck compared claim age with strftime('%s', claimed_at), which truncates the fractional second on both sides of the comparison before doing the epoch arithmetic. A claim whose fractional second happened to land just past a whole-second boundary read as a full second older than it really was, which could reclaim an in_progress task from a worker that had barely started. Switched the comparison to julianday(), which keeps the fractional part. Also corrected the sweep-loop comment above the call site, which claimed the whole query was "a single indexed query" that "costs nothing" — only the status = 'in_progress' filter is covered by idx_task_status_claimed_at; the per-row deadline comparison runs on claimed_at directly and cannot use that index, which is fine only because the in_progress set it runs against stays small. * test(api): de-flake the per-task claim-TTL override test task_timeout_secs_overrides_the_global_claim_ttl raced a real 1s deadline: it asserted "not yet stuck" immediately after the claim with no margin, then slept 2.1s and asserted the reclaim. Any stall between the claim and that first assert — a loaded CI runner, SQLite lock contention — could tip real elapsed time past the 1s budget and fail the test on an assertion that has nothing to do with the behavior under test. Replaced both timing points with a direct claimed_at back-date (the same pattern already used elsewhere in this file), so the test no longer depends on the runner's real-time speed at all. * docs(memory): fix the migration range cited in the v54 test fixture comment The comment said "Steps 51-55 all fire from 50" and named migrate_v55 as the one that alters task_queue. v55 adds the template_versions table and never touches task_queue; it is v56 (per-task claim TTL) that adds task_queue's column, and it also fires from a user_version-50 fixture. Range and migration number both corrected to 51-56 / 56. * feat(dashboard): expose task priority and claim timeout in the New Task dialog tool_runner/task.rs already documented these as "operator controls... from the dashboard", and POST /api/tasks already accepted both, but CreateTaskPayload only carried title/description/assigned_to/created_by, so the dashboard itself had no way to set either. Added both fields to the New Task form and to the task card (shown only when set, to avoid noise on the common default board). * docs(changelog): add the fragment for the claim_ttl_secs=0 documentation fix The struct doc, field doc, config reference table, dashboard help strings and schema golden fixture were all corrected in 8f498f3 (this branch) to stop saying claim_ttl_secs = 0 disables the sweeper entirely, but no changelog fragment was written for a fix that changes what operators believe a production knob does. * fix(memory): the stuck-task sweep's regression test passed on the pre-fix query too test_task_reset_stuck_is_subsecond_precise_not_floored_to_a_second used a claimed_at fractional second of .999999999 to reproduce the strftime floor bug fixed in eda0965. SQLite does not truncate that fraction — it rounds to the nearest millisecond, half up, before truncating to a whole second (computeJD: p->iJD += ... + (s*1000 + 0.5)) — so .999999999 rounds up to the next second and happens to cancel the bug out. Verified against both the literal pre-fix and post-fix queries: the fixture reads "not stuck" on both, so reverting the fix would leave this test green. The fixture was also flaky by construction: it derived claimed_at from chrono::Utc::now().timestamp(), which discards the fractional second of the test's own clock read, leaving up to ~1s of uncontrolled slack against the sweep's own full-precision Utc::now() call. Replaced it with a fully deterministic test that checks the two literal boolean expressions (old floor-based comparison vs. the julianday one shipped today) against a fixed claimed_at/now pair, sidestepping the wall clock entirely. .999 sits under SQLite's .9995 rounding threshold and is verified to discriminate: the old expression flags the fixture as stuck, the new one does not. Also corrected the production comment above the query, which claimed the julianday arithmetic is "exact" (it is precise to the millisecond, not exact) and that strftime "truncates" the fraction (true only below .9995). * fix(api): document comms/task's real 201/400 responses in the OpenAPI contract The #[utoipa::path] block for comms_task declared only a 200, when the handler returns 201 on success (preexisting) and, since eda0965's sibling fix, 400 for an unresolvable assignee. Neither reached the generated spec, so a client generated from it would not know to expect either. Brought it in line with task_queue_post_root's equivalent block and regenerated openapi.json. * fix(dashboard): a zero-second timeout badge read as the opposite of what it means TaskCard showed the timeout badge whenever timeout_secs != null, which lets 0 through — and 0 means "never reclaim" per the sweep's own guard (COALESCE(timeout_secs, ?1) > 0) and the New Task modal's own placeholder text. A task posted with timeout_secs: 0 rendered "TTL 0s", which reads as "reclaimed immediately", the opposite of the guarantee it actually carries. Gave the zero case its own label instead of hiding it: an explicit "never reclaim" override is more worth surfacing to an operator than the absence of any override, not less. Also added the test coverage the priority/timeout dashboard wiring shipped without: the conditional payload spread (priority.trim() / timeout_secs.trim()) had nothing pinning it, so a change like `priority.trim() ?` -> `priority ?` would have started dropping the field from every submission silently, and the two new badges had no fixture exercising them at all. * docs(changelog): put the fragment attribution on the last sentence's line The (#7974) (@DaBlitzStein) tag sat on its own trailing line, which the repo convention rejects: attribution must close the bullet's last non-empty line, not sit as a separate one. check-changelog-attribution.py does not enforce this particular shape, so it merged without complaint. * fix(dashboard): the New Task assignee field could silently post a value the operator no longer saw The assignee field swapped between a free-text <input> and a <select> depending on whether the agent registry had loaded, gated on `agents.length > 0`. A value typed while the registry was still empty stayed in state after that swap: the <select> that replaced the input a moment later had no matching <option> for it, so it rendered as unselected while the component still held the typed value, and a submit right after sent it anyway. Replaced both widgets with a single free-text <input> backed by a <datalist> of registry suggestions, so there is no mode to swap and nothing to desync — typing a name outside the suggestion list still works exactly as it did before the registry loaded. That single input also fixes the two narrower gaps in what the suggestion list itself offered: `useAgents()` now passes `includeHands: true`, since GET /api/agents excludes hand agents by default and the kernel accepts them as assignees; and a non-admin caller is still scoped to their own agents by GET /api/agents (deliberately, per #6753's authorization boundary, and not something to relax here), so the same free-text path is what makes an agent authored by someone else still reachable by typing its name. Also named the endpoint's `limit: "500"` as `AGENT_LIST_LIMIT` rather than an inline literal repeated at every call site, documenting that it is a silent ceiling, not a page size — pagination itself is a separate change this does not make. * docs: the task_claim TTL docs still promised a single global number Two more places stated `[task_board] claim_ttl_secs` as *the* TTL for a claimed task, unqualified — true before this branch, false since a task can now carry its own `timeout_secs` override that wins over the global value. agent/tools/page.mdx (and its zh mirror) described task_claim's deadline this way; an agent reading it would not know that a task it claimed through the API or dashboard, rather than through task_post, might already carry a tighter deadline than the documented default. zh/configuration/core/page.mdx's claim_ttl_secs row was the Chinese mirror of the exact English row 8f498f3's docs pass corrected — the English side now says "0 disables the global clock only", the Chinese side still said "0 completely disables the sweep", left behind when the five dashboard locales and the English config page were updated but this mirror was not. * fix(api): POST /api/comms/task discarded the caller's priority and timeout_secs The route hardcoded TaskPostOptions::default(), so a client sending either field got a 201 for a task queued at priority 0 with no per-task deadline, and no way to tell it had been ignored. CommsTaskRequest now carries the same two controls as POST /api/tasks, typed rather than absent so a wrong-typed field is rejected by the deserializer instead of dropped. The stuck-task sweep logged its global TTL under the name ttl_secs while task_reset_stuck resolves the deadline per row via COALESCE(timeout_secs, ?1). With claim_ttl_secs = 0 and a task carrying timeout_secs = 300 that printed 0 next to a reclaim at 300s, pointing the operator at the one knob their config says is off. Renamed to global_ttl_secs and the message says which applied. * fix(memory): renumber the task-queue TTL migration to v58 The branch was cut when main's ladder topped out at v54 and has been renumbered twice since (55 to 57 to 56), each time against a main that moved again before it merged. Main now holds v55 (`template_versions`), v56 (`workflow_runs.total_steps`) and v57 (the `memories_fts_au` WHEN guard), so the current 56 is wrong in two separate ways. The compile error is the obvious half: main already defines `migrate_v56` as the `workflow_runs.total_steps` ALTER, so merging this branch as it stood produced a duplicate function definition. The half that would have reached a deployment is `SCHEMA_VERSION`. Lowering it from 57 to 56 makes every database already at 57 refuse to boot with "Database schema version 57 is newer than this binary supports (56). Downgrade is not supported." We took that outage on 2026-09-04 from hand-numbered migrations in an integration branch; this is the same shape. v58 is the next free number above main's 57. Nothing else in this tree defines `migrate_v58`. The number stays contiguous rather than skipping ahead to reserve room for the other open schema PRs, and that is deliberate. `run_step!` gates on `current_version < N` with `current_version` read once at boot, so a database that reaches N through a binary with a gap below it will never run the skipped migrations — and the audit backfill at the end of `run_migrations` writes their rows anyway, so the skew is silent and permanent. Three other open PRs also want 58; whichever merges first keeps it and the rest renumber to 59, 60, 61 on rebase. Also here, all of it surfaced by the rebase: - Dropped the stale ladder-order comment that claimed 55 = #7991, 56 = #8041, 57 = this PR, 58 = #8047. Main has since taken 55, 56 and 57 for unrelated migrations, so following it would reintroduce the collision. - `task_status_counts_agrees_with_listing_every_row`, a test main added under #8219, called `task_post` with the pre-PR four-argument form. This branch widens that signature with `priority` and `timeout_secs`, so the rebase left the crate not compiling. Passing `0, None` matches every other call site in the file and keeps the test's meaning — it counts statuses and cares about neither field. - Renamed the two existing migration tests to match the new number, and confirmed `migrate_v58_is_idempotent` still discriminates: removing the `try_column_exists` guard makes it fail with "duplicate column name: timeout_secs", which is what a boot would print. - Fixed the `pre-v56` comment in `kernel/accessors.rs` left behind by the previous renumber. * fix(dashboard): adopt the registry-backed assignee select on the task board Both this PR and `feat/dashboard-extensions` fix the same bug in the new-task assignee field, in incompatible ways, and integrating the two left seven failing tests in `TasksPage.test.tsx`. This adopts the other PR's shape so the two stop contending. The bug in main is `agents.length > 0 ? <select> : <input>`, a two-widget swap over a list derived from the `assigned_to` of tasks that already exist. A value typed while `agents` was still `[]` ended up under a dropdown that had since re-rendered empty; a brand-new agent was unreachable until someone had already assigned it something; a deleted one lingered forever. This PR fixed it with a single free-text input plus a `<datalist>` — one widget, so no mode to swap. That is a real fix for the swap, but it does not fix the stale list, and a name-valued control cannot survive a rename. The registry-backed `<select>`, valued by agent id, fixes all three, so it is the one to keep. The datalist is gone. What this PR still owns is untouched: `priority` and `timeout_secs` on the create payload and their two form fields, the badges on the task card, and the `task_queue.timeout_secs` migration at v58. One line deliberately differs from `feat/dashboard-extensions`: `useAgents({ includeHands: true })` rather than `useAgents()`. The kernel accepts hand agents as assignees, and now that the field is a select rather than a suggestion list, an agent the list omits is not merely unsuggested — it is unreachable. The other PR should take this too. Tests keep their intent rather than being deleted: - "keeps a typed assignee once the agent registry loads instead of losing it to a widget swap" becomes "keeps the selected agent id when the registry changes under it, including across a rename". The widget swap it guarded against cannot happen now — there is no second control — so the surviving guarantee is asserted instead: a chosen assignee is not lost when the registry changes beneath it. The rename half is new, and is the case the datalist could never have passed. Verified it discriminates: valuing the options by name instead of id turns it red. - "includes priority and timeout_secs in the payload when set" is unchanged and still discriminates — dropping the two spread entries from the payload builder turns it red. - The other five failures were resolved by the merge itself and needed no edits. Locale keys follow the widget: `assignee_none`, `no_agents_hint` and `field_description_hint` added to all five locales from the sibling branch, and `field_assignee_placeholder` removed from all five — it was the datalist input's placeholder and now has no referent. * fix(memory): renumber the task-queue TTL migration to v61 The v2026.9.14 release moved main's ladder from 57 to 60: v58 creates `manifest_versions`, v59 re-adds `workflow_runs.total_steps` and v60 re-creates `manifest_versions` for databases a pre-release build stamped past 58 without it. This branch's `task_queue.timeout_secs` step was sitting on 58, which main now occupies with different DDL, so it moves to 61 — the next free number — and `SCHEMA_VERSION` follows. The number stays contiguous rather than skipping ahead to reserve room for the other open schema PR. `run_step!` gates on `current_version < N` with `current_version` read once at boot, so a database that reaches N through a binary with a gap below it never runs the skipped migration, and the audit backfill at the end of `run_migrations` writes its row anyway — the skew is silent and self-concealing. `test_every_migration_records_audit_row` pins this: it asserts a fresh database needs zero backfill placeholders, so a ladder that jumped to 62 while leaving 61 empty would fail it. `try_column_exists` still guards the `ALTER TABLE`, which is what makes the renumber safe against instances that already carry the column from an integration build. Earlier commit messages on this branch still say "v58" where they describe this step; they record what was true when they were written. * test(memory): pin the post-migration assertions to SCHEMA_VERSION, not 60 This branch takes the ladder to 61 and left three assertions reading `get_schema_version(&conn).unwrap(), 60`, so `cargo test -p librefang-memory` is red on the branch itself: - a_database_a_pre_release_build_stamped_at_59_still_opens - the_forward_compat_steps_are_no_ops_when_their_changes_are_already_present - a_database_stamped_past_58_without_manifest_versions_gets_it What each asserts is "and is carried the rest of the way", not "to exactly 60", so the constant is the honest right-hand side — and it is already the spelling the same file uses two hundred lines earlier. Written this way the next migration cannot make them stale again. That matters here specifically: #7991 also claims 61, so whichever of the two lands second renumbers, and a literal would go stale a second time on the way in. `cargo nextest run -p librefang-memory`: 472 passed, 0 failed. * test(tasks): register the assignee labels the backpressure tests post to #7974 rejects an `assigned_to` that names no registered agent, which broke three tests that #8373 had already landed on main: `a_zero_cap_is_unlimited`, `per_agent_depth_cap_is_scoped_to_the_assignee` and `assignee_filter_narrows_both_the_page_and_the_total` all address their tasks to `alice` and `bob`, and the 400 discarded every one of those tasks before the queue saw them. Those names are buckets the assertions compare *across*, not agents anything in the file asserts about: the per-agent cap has to see one assignee saturated and another untouched, and the filter has to see two populations to narrow between. Registering them in the harness leaves every assertion and every test body untouched, which is the point — these are backpressure tests, and assignee resolution is not what they measure. `assignee_wake` is off on those two manifests because a default manifest can claim its own tasks: leaving it on would start a turn for every task posted here — 25 of them in the zero-cap test alone — against a provider CI does not have, and against a real one on a workstation running Ollama. * chore(memory): allow the argument count task_post reaches once main lands `task_post` takes one parameter per queue column, which puts it at clippy's seven-argument ceiling; the `caps` argument main already carries makes it eight, and the workspace denies the lint, so the method fails the gate on the merged tree even though this branch compiles clean on its own. A parameter struct would touch every caller in the API, kernel and runtime for a lint that four sibling methods in this file already silence, so this takes the same allowance they do. Verified with clippy-driver on both pinned toolchains: 7 parameters warn nothing, 8 warn "8/7", and the allowance silences the 8. * chore(codegen): refresh the openapi baseline hash after rebasing onto main The rebase conflict on xtask/baselines/openapi.sha256 was resolved with the incoming branch's stale hash, because at that point in the chain the tree does not yet compile and cargo xtask codegen cannot run. Regenerating from the rebased source at the tip produces a different hash, so the committed baseline now matches what the source actually generates. * fix(dashboard): drop the duplicated tail the merge left in the task locales Five keys at the end of the `tasks` object were duplicated (status_failed, assignee_none, no_agents_hint, field_description_hint) and the object then ended without a comma before the closing brace, so all five locale files were invalid JSON — tsc reports "',' expected" and the dashboard cannot build. The duplicated keys already exist earlier in the same object; removing the copies leaves the object ending exactly as main has it, with the seven keys this PR adds as pure additions. * style(memory): wrap the task_post call sites rustfmt wants multiline Resolving the rebase conflict left five task_post test call sites on one line; with seven arguments they exceed rustfmt's fn_call_width, so the pre-commit hook would reject any later commit touching this file. Formatting only: same arguments, same order. * fix(memory): pass priority and timeout_secs at the depth-cap test call sites The task queue's task_post grew from five arguments to seven in this PR, and the depth-cap tests that landed on main afterwards call it with five. Neither side conflicts with the other -- one adds calls, the other changes the signature -- so the merge left seventeen call sites that do not compile, with no conflict marker and no entry anywhere to show it. Inserts the same neutral 0, None the rest of the file already passes. Fix anchored on the compiler's own E0061 spans, so exactly the broken calls changed. * fix(kernel): pass opts at the depth-cap test call sites Same class as the memory ones: main's #8373 added these kernel-level task_post calls against the trait's old four-argument shape, and this PR makes opts the fifth. Nothing conflicts, so the merge left five calls that do not compile. Anchored on the compiler's five E0061 spans. * fix(memory): land the v61 tests on v61, and the last assertion on the schema it asserts `Test / Unit (lib+bin)` fails on this branch with one test: migration::tests::v60_reconciles_a_table_a_pre_release_build_created_with_fewer_columns assertion `left == right` failed left: 61 right: 60 Three of that fixture's four `get_schema_version` assertions were moved onto `SCHEMA_VERSION` when this branch raised it to 61; the fourth kept the literal, so the test asserts the version the branch left behind. Two more of the same shape turned up while looking at that one: - `migrate_v58_is_idempotent` called `migrate_v58` — the agent-manifest version-history migration — while its comment described replaying the step that re-adds `task_queue.timeout_secs`. It passed, and would have kept passing with that guard deleted, because it never reached the migration it named. It is `migrate_v61_is_idempotent` now and calls `migrate_v61`; with the `try_column_exists` guard removed it fails with `duplicate column name: timeout_secs`, which is the failure the guard exists to prevent. - The two new tests and their section comment said `v58`, which is a different and older migration — `migrate_v58` still numbers the manifest history table — so the name collided with a real step. They say `v61` now, which is the step they exercise. Refs #7974 * fix(dashboard): restrict task priority and timeout inputs to whole numbers The New Task dialog accepted fractional values in both fields. The server stores an integer (priority i64, timeout_secs u32) and answers 400, so the fraction could only ever come back as a form error the browser was already able to catch. step={1} states the contract on the input; min={0} on the timeout already matched the server's as_u64() rejection of negatives. * docs(api): document the 400 vs 422 contract on the task routes POST /api/comms/task deserializes a typed CommsTaskRequest, so a body that is valid JSON but the wrong type — a fractional priority or timeout_secs — is refused by the extractor as 422, while semantic mistakes (empty title, unknown assignee) stay the handler's 400. Sibling POST /api/tasks reads an untyped body and answers 400 for the same inputs. The utoipa block and the handler doc now state which side each route is on, and integration tests pin both ends: a decimal is a 400 there and a 422 here. * fix(memory): reconcile the real v61 on a database the pre-renumber build stamped at 61 A build that ran the timeout_secs step while it was numbered 61 left the database stamped at 61 with the column present — and therefore with sessions.parent_session_id absent, because run_step! gates on current_version < 61 and skips main's v61 for that stamp. The try_column_exists guard tolerated the rerun but not the missing column. migrate_v62 now applies migrate_v61 (idempotent) on that footprint and moves the audit row, so version 61 describes the step that now owns the number and the timeout history is re-recorded under 62. Covered by migrate_v62_reconciles_a_database_stamped_at_61_by_the_pre_renumber_build, checked red with the repair removed. * docs(kernel): make the sweep comment migration-number-agnostic "what every pre-v58 row means" survived the timeout_secs migration's renumber to v62; naming the column instead of a version keeps the next renumber from leaving the number behind again. * test(api): cover both edges of the per-task claim-ttl override Extend task_timeout_secs_overrides_the_global_claim_ttl: with claim_ttl_secs = 0 a row that carries its own timeout is still swept and a row that inherits the global clock is not; an explicit per-task timeout_secs = 0 is never reclaimed past a non-zero global TTL. Rewrite both #7974 changelog fragments to match the shipped behaviour: claim_ttl_secs = 0 is now global-only, and the task_post tool fails with a tool error (not an HTTP 400) while /api/comms/task gains the same AgentNotFound -> 400 arm as /api/tasks. --------- Co-authored-by: Evan <suzukaze.haduki@gmail.com>
Summary
SCHEMA_VERSIONfrom 54 to 58Migrations added
template_versions+manifest_versionstablesIF NOT EXISTStask_queue.timeout_secs+sessions.parent_session_id+ indextry_column_existsagents.parent_id+agents.parent_recorded+ indextry_column_exists(idempotent over v54)template_versionstableIF NOT EXISTS(idempotent over v55)Verification
cargo check -p librefang-memorypasses cleanCloses #8066