fix-invalid-blocks! silently mutates blocks during "Validate graph" — the error is logged, the repair never is
Nobody has claimed this yet.
- Dominant language
- No language data
- Stars
- 28
- Forks
- 2
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
`validate-db` has a sixth mutating pass that is easy to miss: when `fix` is true
(its default), the validation phase itself runs
`validate-and-fix-invalid-blocks!`, which calls `fix-invalid-blocks!` on every
entity the validator flagged, in a loop, until no further fixes are produced.
`fix-invalid-blocks!` retracts attributes and, in some branches, deletes whole
entities. **It logs nothing.** The validation *errors* are printed at `:debug`;
the *repairs* are not printed at all. The final report shows `:errors nil`
precisely because the loop already fixed them — so the user sees a clean bill of
health for a run that modified their data.
This is a companion to logseq/db-test#1068, which covers the same silence in the five
`when fix` passes. This issue is about the sixth site, which that issue does not
reach.
## Version
Logseq DB version, desktop macOS arm64 (build 2287).
`db-worker-node` build-time `2026-07-28T15:15:45Z`, revision `9a11243-dirty`.
## Observed
A block in my graph carried a legacy `:block/pre-block? true` plus a
`:block/properties` value. Validation flagged it:
```
:debug :entity {:block/tx-id …, :block/pre-block? true,
:block/uuid #uuid "…",
:block/properties [[{:db/ident :logseq.property/created-by-ref,
:db/valueType :db.type/ref,
:db/cardinality :db.cardinality/one,
:logseq.property/type :entity} 175]],
:block/title "…"}
:error {:block/pre-block? {"normal-block" ["disallowed key"],
"whiteboard-block" ["disallowed key"]}}
```
Running `validate --fix` on a copy of that graph, the block **survived** (it has
`:block/page` and `:block/parent`) but **both `:block/pre-block?` and
`:block/properties` were retracted from it**.
The command reported:
```
result {:errors nil :invalid-entity-ids () …}
```
Nothing in the log records the retraction. The only way I found it was by
querying DataScript for the entity before and after.
The repair itself may well be correct — `:block/pre-block?` is a file-graph
legacy key that has no place in a DB graph. The problem is that a command
called "Validate" modified stored data and then reported no errors, with no
record of what it changed.
## Why the silence matters more here than elsewhere
`fix-invalid-blocks!` contains branches that go considerably further than
stripping a legacy key:
```clojure
;; whole-entity deletion — every one of these is a :db/retractEntity
(and (:block/title entity) (nil? (:block/page entity))
(nil? (:block/parent entity)) (nil? (:block/name entity)))
(and (:block/tx-id entity) (nil? (:block/title entity)))
(and (= "External URL" (:block/title entity)) (nil? (:block/tags entity)))
(and (:logseq.property/created-by-ref entity)
(not (de/entity? (:logseq.property/created-by-ref entity))))
(vector? (:logseq.property/value entity))
(= :whiteboard-shape (:logseq.property/ls-type entity))
(and (:logseq.property.asset/remote-metadata entity)
(nil? (:logseq.property.asset/type entity)))
```
Two of these are worth singling out. `(= "External URL" (:block/title entity))`
deletes a whole entity on an **exact title-string match** — any untagged block
titled "External URL" is destroyed. And `(and (:block/tx-id entity) (nil?
(:block/title entity)))` covers every title-less entity with a transaction id,
which is a very large class of internal records.
`:db/retractEntity` also removes **inbound reference datoms**. So deleting one
flagged entity silently strips property assertions off other blocks — blocks
that were never flagged, are never named in any log line, and whose
`:block/updated-at` is not bumped. The blast radius of a single validation error
is not the erroring entity.
And a sweep over every datom in the database whose attribute namespace is
`user.class`:
```clojure
(->> (d/datoms db :eavt)
(filter (fn [d] (= (namespace (:a d)) "user.class")))
(mapcat (fn [d]
(let [class-title (:block/title (d/entity db (:a d)))
property (get-property-by-title db class-title)]
(if property
[[:db/retract (:e d) (:a d) (:v d)]
[:db/add (:e d) (:db/ident property) (:v d)]]
[[:db/retract (:e d) (:a d) (:v d)]]))))) ; ← retract, no re-add
```
The lookup is keyed on **title**. When it misses — a renamed property, a
title-cased differently — the datom is retracted with no replacement. That is
unconditional data loss on a title match, and it is likewise unlogged.
I have not observed either of these branches fire. I am reporting them because
the one branch I *did* observe proved the pass mutates data without saying so,
which makes the more destructive branches worth a look before someone hits them.
## The loop
```clojure
(defn- validate-and-fix-invalid-blocks!
[conn]
(loop [{:keys [errors] :as result} (validate-db-result @conn)]
(log-validation-errors! errors)
(if (and (seq errors) (fix-invalid-blocks! conn errors))
(recur (validate-db-result @conn))
result)))
```
Each round re-validates and re-fixes, so effects can compound within one
invocation — and only the *last* round's `errors` reaches the report. A run that
fixed several rounds' worth of problems still reports `:errors nil`.
## Steps to reproduce
1. Have a graph containing a block with a disallowed legacy key — in my case
`:block/pre-block? true`, which appears on blocks carried over from earlier
graph versions.
2. Query DataScript for that entity's datoms and record them.
3. Run **Validate graph** (or `logseq graph validate --fix`).
4. Query the entity again.
**Expected:** either the block is left alone and reported as invalid, or it is
repaired and the repair is reported.
**Actual:** attributes are retracted, the report says `:errors nil`, and no log
line describes the change.
## Suggested fixes
1. **Log every fix `fix-invalid-blocks!` applies**, at the same level as the
errors, and include a summary in the report the user sees.
2. **Report the errors that were fixed**, not just those remaining after the
loop. `:errors nil` after repairs is actively misleading.
3. **Reconsider `:db/retractEntity` in a pass called "fix"** — or at minimum
log which inbound references were severed, since those land on blocks that
were never flagged.
4. **Do not key the `class-as-properties` migration on `:block/title`**, or
preserve the datom when the lookup misses instead of retracting it.
## Related
- logseq/db-test#1068 — the same silence in the five `when fix` passes
- logseq/db-test#1069 — the property-retype state that triggered the loss in logseq/db-test#1068
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with validate-and-fix-invalid-blocks!, validate-db-result, and fix-invalid-blocks!, then trace how log-validation-errors! and the validation command produce the final report. Reproduce the issue by comparing DataScript datoms before and after Validate graph --fix. Done means repairs, including destructive or inbound-reference effects, are recorded and the report does not hide fixes behind a final nil error list.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- clojure
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100