Guard against silent anonymous-block graph collisions on push#2
Merged
Conversation
Follow-up to the validate.js fix (PR #1), addressing the root cause Ben flagged: a blank line between a <!-- databook:id: ... --> annotation and its code fence makes the block anonymous, and anonymous blocks resolving to the default graph get PUT sequentially — each replacing the last, with no error or warning. Bit the UN-GGCE QA pass twice (satellite-orbits in June, capability-maturity this week). Two changes, addressing both the trigger and the underlying hazard: 1. lib/parser.js — tolerate a single blank line in the pre-fence backward walk. There's no ambiguity in this case (nothing else could plausibly own the annotation), so this closes the specific trigger at no cost. 2. commands/push.js (the structural fix) — refuse to push, rather than silently overwrite, when two or more pushable blocks resolve to the same graph IRI under PUT semantics. This catches the collision regardless of *why* a block ended up anonymous (blank line, malformed annotation, missing frontmatter id, etc.), not just this one trigger. --merge (POST, additive) or unique per-block ids opt out. Verified locally: - Blank-line annotation now resolves to its intended block id. - Two anonymous blocks under default PUT semantics now die with a clear message before any network call is attempted. - Named blocks and --merge mode are unaffected (no false positives).
This was referenced Jul 8, 2026
kosmika
pushed a commit
to kosmika/w3c-cg_holon
that referenced
this pull request
Jul 8, 2026
Follow-up to the fix in kurtcagle/databook (PR kurtcagle/databook#2), adapted for this repo's older, divergent implementations/js/ copy of the CLI. This copy's lib/parser.js predates pre-fence <!-- databook:id: ... --> annotations entirely (inline-only), so the blank-line trigger Ben hit doesn't apply here — no parser change needed. But the same class of bug exists via a different path: this push.js's resolveGraphIri() falls back to `${docId}#${block.id ?? block.label}` for blocks with no id, so two or more anonymous blocks sharing a fence label (e.g. two 'turtle' blocks) resolve to the identical graph IRI and silently clobber each other under PUT. Adds the same guard as the canonical fix: refuse to push, rather than silently overwrite, when 2+ pushable blocks resolve to the same graph IRI under PUT semantics. --merge (POST, additive) or a unique id per block opts out. Verified locally: - Two anonymous same-label blocks now die with a clear message before any network call (caught via the #turtle fragment collision, not a default-graph collision — the correct path for this implementation's fallback logic). - Named (inline-annotated) blocks and --merge mode are unaffected.
kosmika
pushed a commit
to kosmika/w3c-cg_holon
that referenced
this pull request
Jul 8, 2026
Follow-up to Paul Trevithick's pre-fence annotation support (PR w3c-cg#1, merged 2026-07-08), which introduced the same class of bug this whole thread has been chasing: the buffer reset on ANY non-comment line included blank lines, so <!-- databook:id: ... --> annotations separated from their fence by a blank line silently become anonymous — the exact trigger that bit the UN-GGCE QA pass twice in the canonical repo (see kurtcagle/databook#2). This keeps Paul's forward-scan design intact and just makes the reset condition tolerate one blank line before giving up, rather than switching to the canonical repo's backward-walk approach — no reason to rewrite a working, just-merged contribution when a small, targeted change closes the gap. Verified with four cases: single blank line (tolerated), no blank line (control), two consecutive blank lines (still resets — deliberately not loosened further), and a real content line between annotation and fence (still resets, correctly, since that's never ambiguous).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1 (validate.js), addressing the root cause Ben flagged in review:
Bit the UN-GGCE QA pass twice (satellite-orbits in June, capability-maturity this week).
Two changes — one closes the specific trigger, the other closes the class of bug:
lib/parser.js— the pre-fence annotation walk now tolerates a single blank line before giving up. There's no ambiguity in this case (nothing else could plausibly own the annotation), so this is a safe, low-risk relaxation.commands/push.js(the structural fix) — before executing any push, refuse (rather than silently overwrite) when two or more pushable blocks resolve to the same graph IRI under PUT semantics. This is deliberately independent of Fix databook validate: ESM require crash, silent engine fallback, error masking #1: it catches the collision regardless of why a block ended up anonymous — blank line, malformed annotation, missing frontmatter id, or anything else — rather than only patching this one trigger.--merge(POST, additive) or giving each block a unique id opts out.Verified locally against three cases before pushing:
--mergemode are unaffected — no false positives.Rationale for doing both rather than just the parser fix: the parser fix alone would have given a false sense that this class of bug was closed, when in fact any other path to an anonymous block (typo in the annotation key, missing
idin frontmatter, etc.) would still silently corrupt data on push.