Repository navigation
feat(api,memory): template version history and restore - #8047
Conversation
|
Migration version note: This PR claims |
houko
left a comment
There was a problem hiding this comment.
This is the well-built one of the version-history cluster: openapi.rs, openapi.json, all four SDKs and xtask/baselines/openapi.sha256 move together, delete_agent_type cascades the history away, the store dedups against the latest snapshot and trims in-transaction, and all five dashboard locales get the same keys. CI is green.
Four things block it, and two of them are cross-PR.
1. No integration test for two new routes
CLAUDE.md makes this a hard gate — "Add a #[tokio::test] against TestServer" for any route change, and reviewers gate PRs on it — and there is nothing under crates/librefang-api/tests/ in this diff.
crates/librefang-api/tests/agent_types_routes_integration.rs is the obvious home; it already has write_agent_type, boot(), get/post helpers and a cleanup.
The recording side is what makes this more than box-ticking. record_template_version is called from inside create_agent_type and update_agent_type, and the failure mode there is silent: a PUT that succeeds while the snapshot never lands looks identical to a working feature until someone opens the history modal. That is verbatim the last gotcha in CLAUDE.md — "write the integration test at the injection site, not just the implementation site".
Minimum coverage I'd want to see: POST then GET .../history shows one entry with change_source: "create"; PUT adds a second; a second identical PUT adds none (the dedup); POST .../history/{id}/restore returns the older spec and appends a "restore" entry; 404 for an unknown template; 409 for a live-agent name; the version-belongs-to-another-template 400.
2. The new handlers bypass the crate's i18n and hardcode English
Every other handler in agent_templates.rs renders its messages through ErrorTranslator against crates/librefang-types/locales/*/errors.ftl — api-error-template-invalid-name, api-error-agent-type-not-editable, and so on, in all nine languages.
The new ones emit raw English:
"invalid template name"format!("Template '{name}' not found")"that name belongs to a live agent; restore it through /api/agents""version not found"/"version does not belong to this template""stored version is corrupt and cannot be restored"
No locales/*/errors.ftl file is in this diff. This is a regression against the pattern in the file you are editing, and the existing keys already cover two of these cases (api-error-template-invalid-name, api-error-agent-type-not-editable) so most of the work is reuse, not new strings.
3. Restore does not write back the TOML the operator was shown
The changelog fragment says the endpoint "writes the stored TOML back to disk". It does not — it parses the snapshot into an AgentManifest and calls persist_agent_type, which re-serializes from the struct.
AgentManifest is #[serde(default)] without deny_unknown_fields, so an old snapshot is accepted even if it names fields the struct no longer has — and those fields are silently dropped on the way back out. So a restore of an old version can produce a file that differs from the TOML shown in the history view, in exactly the direction #7740's whole design is defensive about.
Re-serializing is arguably the safer choice (it guarantees the result parses), but then the fragment and the API docs should say "restores the manifest recorded in that version", and the dashboard should show what will actually be written. Please pick one and make all three agree.
4. SCHEMA_VERSION = 55 collides with #8041
#8041 also bumps SCHEMA_VERSION to 55 and also defines a migrate_v55, for a different table (manifest_versions there, template_versions here).
Git catches the text conflict; the dangerous part is the resolution. run_step! is version-gated, so a database that already applied either PR's v55 has user_version = 55 and will never run the other's step — an installation that upgrades through both releases ends up missing one of the two tables, and the feature fails at runtime against a table that does not exist.
Whichever merges second must renumber to 56 (with its own INSERT row, since the #3538 audit ties user_version to the count of distinct migrations rows) and be re-run through CI on the post-merge base.
5. #8052 is a second, independent implementation of this PR's dashboard half
#8052 adds TemplateHistoryModal.tsx plus its own client functions, query hook and key factory entry for these same two endpoints, and has no backend of its own — it depends on the routes this PR adds.
The two do not merely conflict textually, they disagree on design:
| #8047 (this PR) | #8052 | |
|---|---|---|
| query hook | useAgentTypeHistory |
useTemplateHistory |
| key shape | agentTypeKeys.detail(name) + "history" |
agentTypeKeys.histories() + name |
| restore hook | useRestoreTemplateVersion |
useRestoreTemplateVersion (same export name) |
The key shapes are the substantive difference: nesting under detail(name) means invalidateQueries({ queryKey: agentTypeKeys.detail(name) }) — which every existing agent-type mutation already calls — also invalidates the history, while a sibling histories() branch does not. Both are defensible; they cannot both exist.
Please settle it before either merges: keep the backend here, and either drop this PR's dashboard layer in favour of #8052's, or close #8052. Also worth flagging that AgentTypesPage.tsx and api.ts's AgentTypeDetail are being edited concurrently by #8027, #8028, #8042, #8043, #8052, #8053 and #8054 as well.
Smaller
- The changelog fragment ends
(@DaBlitzStein)with no(#8047).xtask'scurated_pr_refsreads the last(#N)group on a bullet's last line to decide which generated line to suppress, and documents that "a bullet naming no PR fails open on its own... so that PR keeps its generated line" — so at release the notes will carry both your prose and an auto-generated- feat(api,memory): template version history and restore (#8047) (@DaBlitzStein). Also rename the file to8047-template-version-history.mdperchangelog.d/README.md. change_sourceforPUTis recorded as"dashboard", but that endpoint is equally reachable from the SDKs, the CLI and agents."api"would be honest. Note #8041 makes the opposite choice and labels its one call site"api"— the two stores should share a vocabulary, ideally a typed enum inlibrefang-memoryrather than free strings in two crates.- Recording only happens in the two HTTP handlers.
agent_type_create(the agent-facing tool) andsave-as-agent-typealso write into this store and produce no snapshot, so the history has holes the fragment does not mention. Path<(String, i64)>means a non-numericversion_idis rejected by axum with a plain-text 400, not the JSONApiErrorResponsethe#[utoipa::path]documents.limit=0is honoured literally and returns an empty list, which reads as "no history".
5cfadc9 to
3e14596
Compare
…d UI
Every create, edit, delete and restore of a template is snapshotted in
SQLite (migration v55, `template_versions` table) so operators can see
how a template changed over time and one-click restore a prior
configuration from the dashboard.
Backend:
- `TemplateVersionStore` in librefang-memory with record, list, get and
cascade-delete; deduplication on insert (skip if latest TOML is
byte-identical); retention cap at 50 versions per template.
- `GET /api/templates/{name}/history` lists snapshots.
- `POST /api/templates/{name}/history/{id}/restore` writes the stored
TOML back to disk and records the restore as a new version.
- Version recording is best-effort (never blocks the operation that
triggered it).
Dashboard:
- History modal on AgentTypesPage with expand/collapse TOML viewer and
per-entry Restore button.
- Query/mutation hooks following the data-layer conventions (factory keys,
colocated invalidation).
Skills already tracked version history via `.evolution.json` and are
unchanged.
…and regenerate artifacts
Head branch was pushed to by a user without write access
3e14596 to
f7dadb9
Compare
houko
left a comment
There was a problem hiding this comment.
This is the right shape for the feature and by far the best of the three PRs implementing it — migration, store with unit tests, careful restore handler (name validation, v.template_name == name ownership check, TOML parsed before it is written), SDK updates, OpenAPI entry and a changelog fragment. A few things to fix, and a coordination problem with two sibling PRs.
Blocking
1. No TestServer integration test for the two new routes.
The six #[test]s are store-level, over a test_pool(). CLAUDE.md is explicit that a route change needs a #[tokio::test] against the real router in crates/librefang-api/tests/, and that reviewers gate on it — precisely because that is what catches a missing server.rs registration or kernel↔API type drift. Please add: create an agent type, PUT an edit, GET /api/templates/{name}/history and assert two rows with the expected JSON keys, then POST .../history/{id}/restore and assert the type's TOML came back. The negative cases are worth pinning too — a version id belonging to another template (400 version_mismatch) and a name that resolves to a live agent (409 template_not_editable), since both are logic this handler owns and nothing else covers.
2. #8052 and #8073 implement clients against a response shape that does not match this one.
This endpoint returns {"versions": [{"id", "template_name", "timestamp", "manifest_toml", "change_source"}]}, and this PR's own api.ts matches it.
- #8052 declares
{ id: string, template_name, toml_snapshot, source, created_at }and reads a baredata.versions— three field names wrong andidtyped as a string where the server sends a number. It also addsagentTypeKeys.history,useTemplateHistoryanduseRestoreTemplateVersion, all of which this PR adds too. - #8073 (TUI) reads a bare top-level array of
{ id, created_at, source }and drops every row whosesourcekey is missing — which, against this response, is every row.
Since this PR already ships the dashboard side, the cleanest resolution is to merge this one and close #8052, with #8073 rebased onto the real shape. Worth agreeing that explicitly before any of the three lands.
3. record_version's dedupe read uses .ok() and runs outside the transaction.
let latest: Option<String> = conn.query_row(…).ok();OptionalExtension is already imported at the top of the file — .optional().map_err(LibreFangError::memory)? distinguishes "no rows" from a real failure. As written, a missing table or an I/O error reads as "no previous version" and falls through to the insert, which then fails with a much less useful message.
Separately, the read happens on conn before tx begins, so two concurrent record_version calls for the same template can both observe the same latest row and both insert. The doc comment presents the dedupe as a property ("Skips the insert when the TOML is byte-identical"), so it should hold under concurrency — move the SELECT inside the transaction.
4. timestamp and created_at are the same value, and only one is used.
timestamp TEXT NOT NULL DEFAULT (datetime('now')),
…
created_at TEXT NOT NULL DEFAULT (datetime('now'))Every SELECT in the store reads timestamp; created_at is never read, never serialized, and never set explicitly. Drop it before the migration ships — removing a column afterwards costs a table rebuild.
5. datetime('now') is not RFC 3339, and it reaches the browser verbatim.
SQLite renders 2026-09-01 12:00:00 — space separator, no offset. That string is serialized as timestamp and rendered client-side; new Date() on it is implementation-defined (Safari returns Invalid Date), and nothing tells the reader it is UTC. strftime('%Y-%m-%dT%H:%M:%SZ', 'now') in the DDL, or writing an RFC 3339 string from Rust, fixes it for every consumer at once — including #8073's TUI, which formats it as-is.
6. Nothing removes history when the agent type is deleted.
DELETE /api/templates/{name} leaves every snapshot in place, and each snapshot is a full manifest — workspace paths, the env var names holding provider credentials, command allowlists, system-prompt text. Two consequences worth deciding deliberately rather than by omission:
- The data outlives the object it describes, with no UI that can reach it.
- Creating a new agent type with a previously used name silently inherits the old one's history, so the operator sees snapshots of a configuration that was never theirs.
Either delete the rows with the type, or keep them on purpose and say so in the module doc. #7986 ("purge every trace of an agent") is in flight against the same expectation, so this should agree with it.
CI
- OpenAPI Drift is red.
openapi.jsonandxtask/baselines/openapi.sha256are both updated here, so this is almost certainly a stale base —mainhas moved since 2026-08-31. Rebase and regenerate both; recompute the baseline rather than resolving it by picking a side, since neither hash describes the merged file. - Security is red on
cargo audit, not on anything in this diff:lru0.18.0 (RUSTSEC-2026-0253) andrand0.7.3 (RUSTSEC-2026-0097), both transitive and both predating this branch. Many open PRs are red on the same pair. Not yours to fix here.
Non-blocking
record_template_versionbuilds a freshTemplateVersionStoreper call. That is just a pool-handle clone, so it is cheap, but holding one onAppStatewould make the dependency visible and testable.MAX_VERSIONS_PER_TEMPLATE = 50is invisible to clients:?limit=200silently returns at most 50 with no indication the history is capped. Worth returning the cap (or atruncatedflag) so a UI can say "showing the most recent 50 of an unbounded history".
|
Broadcast to the four PRs holding All four bump
The rulePer Suggested order, by how close each is to mergeable
Nothing about this order is binding; it is a default so the four of you are not each waiting on the other three. If a different one becomes ready first, it should take 55 and the rest shift. Related#8068 proposes forward-compatible stub migrations for v55-v58 and would pre-empt all four of these — a DB that ran those stubs sits at |
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 = librefang#7991 (sessions.parent_session_id), 56 = librefang#8041 (agents.manifest_versions), 57 = this PR (task_queue.timeout_secs), 58 = librefang#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.
…ersion-history # Conflicts: # xtask/baselines/openapi.sha256
… restore Move the byte-identical-Toml dedupe SELECT inside an Immediate transaction so concurrent record_version calls for one template cannot double-insert. Drop the unused created_at column and default timestamp to RFC 3339 UTC. Add TestServer integration tests for GET /history and POST /restore, covering the happy path plus version_mismatch / version_not_found / template_not_editable. Tag the changelog fragment with its PR reference.
|
Thanks — all six points from the review are addressed on the branch (pushed to the fork):
Tests: |
houko
left a comment
There was a problem hiding this comment.
Everything blocking is resolved, and one of my six points was wrong. Taking that one first.
#6 was my mistake. I wrote "Nothing removes history when the agent type is deleted". agent_templates.rs:697-701 already did:
// Best-effort cascade: delete version history for the removed template.
if let Err(e) = store.delete_for_template(&name) {
tracing::warn!(template = %name, error = %e, "Failed to cascade-delete template version history");backed by TemplateVersionStore::delete_for_template. I checked it against the pre-review commit f7dadb9f2 — both the call site and the store method were there when I reviewed, so the data-outliving-the-object and name-reuse-inherits-history concerns I raised did not exist. Best-effort with a WARN rather than transactional is the right call for a delete path that must not fail on a history write, and it agrees with #7986's expectation. Nothing to change.
The rest, verified against the head:
#1 — integration tests. agent_types_routes_integration.rs gains the two I asked for. history_and_restore_round_trip creates, edits, asserts two rows with template_name and the dashboard/create change sources in the right order, then restores and asserts the TOML came back. restore_rejects_foreign_versions_and_live_agents pins all three negative codes — version_mismatch (400), version_not_found (404), template_not_editable (409). That covers the logic this handler owns and nothing else does.
#3 — dedupe. The SELECT is now inside a TransactionBehavior::Immediate transaction and uses .optional().map_err(…)?. The comments state both reasons — why Immediate (two concurrent calls cannot both observe the same latest row) and why .optional() over .ok() (a missing table or I/O error would otherwise read as "no previous version"). The doc comment's dedupe claim now actually holds under concurrency.
#4 — created_at is gone from the DDL, before the migration shipped, so no table rebuild.
#5 — timestamps are RFC 3339. strftime('%Y-%m-%dT%H:%M:%SZ', 'now') in the DDL fixes it once for every consumer rather than per client, which was the point.
#2 — sibling coordination resolved as discussed. #8052 is closed. #8073 is still open and still reads a bare top-level array of { id, created_at, source }, which does not match this response; that is #8073's rebase to do, not a blocker here, and I will raise it there rather than hold this.
CI Gate is green, so the OpenAPI Drift red I flagged was the stale base as expected.
The two non-blocking items stand and neither is worth holding for: record_template_version still constructs a store per call (a pool-handle clone, cheap), and MAX_VERSIONS_PER_TEMPLATE = 50 is still invisible to clients, so ?limit=200 silently returns at most 50. The second is the one worth a follow-up issue if a UI ever needs to say "showing the most recent 50" — it is a response-shape addition, so better done deliberately than squeezed in here.
Approving.
PR librefang#8041 and sibling PR librefang#8047 both claimed SCHEMA_VERSION = 55 with different migrate_v55 bodies (manifest_versions vs template_versions). run_step! is version-gated, so whichever merged second would silently no-op on any database that already ran the other: user_version sits at 55, the second feature's table is never created, and its store fails at runtime with "no such table". Renumber this PR's step to 56 so the two land independently: run_step! line, migrate_v56 fn name, SCHEMA_VERSION constant, the INSERT INTO migrations audit row, and the store's doc reference. Number is free against origin/main (54) and librefang#8047 (55). The audit-row ladder requires contiguity, so the isolated branch shows a hole at 55 until librefang#8047's template_versions step lands; verified green on a merge with librefang#8047's head.
Resolves the migration.rs conflict by keeping origin/main's v55 (template_versions, landed librefang#8047) and renumbering this PR's sessions.parent_session_id step to v56. The renumber covers all four sites: run_step!, fn migrate_v56, the SCHEMA_VERSION constant, and the audit-row VALUES(56). Contiguity is required, not stylistic: test_every_migration_records_audit_row asserts every version 1..=user_version holds an audit row, so a ladder that jumped 55 to 58 would fail it and the backfill check would flag placeholder rows. Landing order may force one more renumber: librefang#8041 holds v56 (manifest_versions) and librefang#7974 holds 57, so whichever schema PR merges first keeps its number and the rest shift on their next rebase. Refs: librefang#7752
Main moved past 07cb6fc (template_versions v55 landed via librefang#8047, Rich Markdown telegram outbound, model snapshot refresh) since this branch's last merge. Conflict resolution: crates/librefang-memory/src/migration.rs keeps upstream's migrate_v55 byte-identical and adds this PR's task_queue.timeout_secs as v57; openapi.json, the four SDKs and xtask/baselines are regenerated from the merged tree with cargo xtask codegen --openapi + scripts/codegen-sdks.py + cargo xtask schema-check gen, so xtask/baselines/openapi.sha256 is recomputed rather than conflict-resolved.
PR librefang#8041 and sibling PR librefang#8047 both claimed SCHEMA_VERSION = 55 with different migrate_v55 bodies (manifest_versions vs template_versions). run_step! is version-gated, so whichever merged second would silently no-op on any database that already ran the other: user_version sits at 55, the second feature's table is never created, and its store fails at runtime with "no such table". Renumber this PR's step to 56 so the two land independently: run_step! line, migrate_v56 fn name, SCHEMA_VERSION constant, the INSERT INTO migrations audit row, and the store's doc reference. Number is free against origin/main (54) and librefang#8047 (55). The audit-row ladder requires contiguity, so the isolated branch shows a hole at 55 until librefang#8047's template_versions step lands; verified green on a merge with librefang#8047's head.
PR librefang#8041 and sibling PR librefang#8047 both claimed SCHEMA_VERSION = 55 with different migrate_v55 bodies (manifest_versions vs template_versions). run_step! is version-gated, so whichever merged second would silently no-op on any database that already ran the other: user_version sits at 55, the second feature's table is never created, and its store fails at runtime with "no such table". Renumber this PR's step to 56 so the two land independently: run_step! line, migrate_v56 fn name, SCHEMA_VERSION constant, the INSERT INTO migrations audit row, and the store's doc reference. Number is free against origin/main (54) and librefang#8047 (55). The audit-row ladder requires contiguity, so the isolated branch shows a hole at 55 until librefang#8047's template_versions step lands; verified green on a merge with librefang#8047's head.
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 = librefang#7991 (sessions.parent_session_id), 56 = librefang#8041 (agents.manifest_versions), 57 = this PR (task_queue.timeout_secs), 58 = librefang#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.
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 = librefang#7991, 56 = librefang#8041, 57 = this PR, 58 = librefang#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 librefang#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.
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 = librefang#7991 (sessions.parent_session_id), 56 = librefang#8041 (agents.manifest_versions), 57 = this PR (task_queue.timeout_secs), 58 = librefang#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.
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 = librefang#7991, 56 = librefang#8041, 57 = this PR, 58 = librefang#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 librefang#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.
PR librefang#8041 and sibling PR librefang#8047 both claimed SCHEMA_VERSION = 55 with different migrate_v55 bodies (manifest_versions vs template_versions). run_step! is version-gated, so whichever merged second would silently no-op on any database that already ran the other: user_version sits at 55, the second feature's table is never created, and its store fails at runtime with "no such table". Renumber this PR's step to 56 so the two land independently: run_step! line, migrate_v56 fn name, SCHEMA_VERSION constant, the INSERT INTO migrations audit row, and the store's doc reference. Number is free against origin/main (54) and librefang#8047 (55). The audit-row ladder requires contiguity, so the isolated branch shows a hole at 55 until librefang#8047's template_versions step lands; verified green on a merge with librefang#8047's head.
PR librefang#8041 and sibling PR librefang#8047 both claimed SCHEMA_VERSION = 55 with different migrate_v55 bodies (manifest_versions vs template_versions). run_step! is version-gated, so whichever merged second would silently no-op on any database that already ran the other: user_version sits at 55, the second feature's table is never created, and its store fails at runtime with "no such table". Renumber this PR's step to 56 so the two land independently: run_step! line, migrate_v56 fn name, SCHEMA_VERSION constant, the INSERT INTO migrations audit row, and the store's doc reference. Number is free against origin/main (54) and librefang#8047 (55). The audit-row ladder requires contiguity, so the isolated branch shows a hole at 55 until librefang#8047's template_versions step lands; verified green on a merge with librefang#8047's head.
…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
template_versionstable (migration v55).TemplateVersionStoreinlibrefang-memorywith record, list, get and cascade-delete; deduplication on insert (skip if latest TOML is byte-identical); retention cap at 50 versions per template, trimmed in the same transaction.GET /api/templates/{name}/historylists version snapshots;POST /api/templates/{name}/history/{id}/restorewrites the stored TOML back to disk and records the restore as a new version.keys.ts, colocated invalidation)..evolution.jsonand are unchanged.Verification
cargo test -p librefang-memory --lib— 454 tests pass (includes 6 newtemplate_version_storetests covering record/list, dedup, trim, cascade delete, get by id, separate templates).cargo clippy -p librefang-api -p librefang-memory -- -D warnings— zero warnings.cargo check -p librefang-memory -p librefang-api --lib— compiles clean.pnpm install --ignore-scripts && pnpm run build— builds successfully.Files changed
crates/librefang-memory/src/template_version_store.rs(new) — store implementation + unit testscrates/librefang-memory/src/lib.rs— module + re-exportscrates/librefang-memory/src/migration.rs— v55 migration,SCHEMA_VERSIONbumpcrates/librefang-api/src/routes/agent_templates.rs— history/restore endpoints, version recording in create/update/delete handlerscrates/librefang-api/dashboard/src/api.ts—TemplateVersionEntrytype,getTemplateHistory,restoreTemplateVersioncrates/librefang-api/dashboard/src/lib/http/client.ts— re-exportscrates/librefang-api/dashboard/src/lib/queries/keys.ts—history()factory keycrates/librefang-api/dashboard/src/lib/queries/agentTypes.ts—useAgentTypeHistoryhookcrates/librefang-api/dashboard/src/lib/mutations/agentTypes.ts—useRestoreTemplateVersionhookcrates/librefang-api/dashboard/src/pages/AgentTypesPage.tsx—TemplateHistoryModalcomponentchangelog.d/added/template-version-history.md— changelog fragment