You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Rendition jobs now survive daemon restarts and short-lived catalog failures without resubmitting provider work or publishing half a build. One fenced job owns each source/profile build, authorized waiters attach to it, and provider handles plus sealed execution authority survive backup and restore.
If a provider call is ambiguous and no handle was checkpointed, the job stops for operator resolution instead of guessing. Previous rendition and lexical heads remain active until every authorized attachment can publish atomically. Purge, prune, garbage collection, and the daemon operation gate now respect the same job lifecycle.
Why
Hosted conversion can be expensive and non-idempotent after an uncertain response. Restarting or retrying must resume the known job when possible, never duplicate a paid submission, and never move serving authority to an incomplete build.
Usage
The process-local RenditionRuntimeRegistry is the integration seam: an application runtime registers exact descriptor fingerprints before the daemon worker can start, and matching authorized requests then join or resume the fenced job.
The registry is intentionally empty in this tranche. S1–S3 add provider construction, registration, enqueue, operator-resolution, API, and CLI surfaces; until then the worker gate stays closed.
Verdict: Changes requested — one high-severity crash risk and five medium-severity reliability or authorization issues remain.
High
XML parsing can panic on nested documents — internal/processing/source_metadata.go:335
The XML stack stores strings.Builder values directly. Slice growth can copy initialized builders, causing subsequent writes to panic on ordinary pretty-printed nested XMP/OOXML. Metadata extraction then repeatedly fails on the same document. Fix: Store pointers to stack elements/builders, enforce XML depth and text limits, and test nested XML containing inter-element whitespace.
Medium
Rendition worker never starts — cmd/docbank/daemon.go:153
The rendition registry is created empty and checked before any adapters are registered. Ready() is therefore always false, preventing durable rendition jobs from progressing. Fix: Register configured adapters before checking readiness, or inject a pre-populated registry during daemon startup.
Poll exhaustion can duplicate remote submissions — document/docling/client.go:217; document/bridge/client.go:164
After a remote job is accepted, poll exhaustion is reported as retryable Capacity. The worker clears non-resumable provider state and submits again even though the original job may still be running. Fix: Return AmbiguousSubmission unless cancellation is confirmed, or persist the remote job ID and resume polling.
Datalab deadline incorrectly becomes a terminal failure — document/datalab/client.go:247
Once a request ID exists, the local total deadline is mapped to terminal Canceled or PolicyRejected. With the default timing, this can abandon a running remote job and permanently fail its durable job. Fix: Treat locally imposed deadline or authorization expiry after submission as ambiguous, or persist the request ID for resumed polling.
Deleting the selected waiter can strand remaining work — internal/store/rendition_delete.go:60
If the selected waiter is deleted before provider egress, reconciliation fails the shared job while leaving other waiters in waiting. Failed jobs are not claimable, so the remaining authorized work is stranded. Fix: Fence the current claim, clear the selected authority, and requeue the job when other waiters remain and doing so is safe.
Bridge operations can outlive authorization expiry — document/bridge/client.go:124 Render validates authorization only at entry and then uses client.totalTimeout. A delayed or compromised provider can complete after authorization expiry while supplying receipt timestamps that appear valid. Fix: Bound the operation context by the earlier of authorization.ExpiresAt and the configured timeout, then check the trusted local clock immediately before accepting completion.
Code review found three medium-severity issues; no high or critical findings.
Medium
cmd/docbank/daemon.go:153 — The daemon creates an empty rendition runtime registry and checks Ready() without registering an adapter. As a result, process:renditions can never start, leaving configured or restored rendition jobs unprocessed.
Fix: Register configured provider runtimes before checking readiness, and reject unsupported profile configurations instead of accepting inert ones.
internal/store/source_metadata.go:194 — Historical version metadata is combined with the node’s current name/path and latest active provenance. After a rename, move, or content replacement, an older immutable version may be attributed the newer filename, source path, modification time, and ingest time.
Fix: Resolve provenance and attachment facts as of the requested version’s introduction, or omit facts that cannot be reconstructed historically.
internal/processing/rendition_worker.go:500 — Lease-renewal retries have no deadline tied to claim.LeaseExpires. Persistent catalog contention may let the lease expire while provider execution continues, allowing another worker to reclaim the job and overlap provider egress.
Fix: Track the expiration returned by each renewal, bound retries by that expiration with a safety margin, and cancel the provider context before the lease becomes reclaimable.
High-severity daemon wiring prevents rendition processing; three medium-severity extraction issues can cause retry loops, permanent backfill failures, or resource exhaustion.
High
cmd/docbank/daemon.go:153 — The daemon checks Ready() on an empty rendition runtime registry without registering production runtime adapters. The rendition worker therefore never starts, leaving queued or restored jobs unable to advance.
Fix: Register adapters for configured processing profiles, including their exact descriptor fingerprints, before checking Ready().
Medium
internal/store/processing_migration.go:239 — Current-version ExtractionFailed results are treated as ineligible and immediately requeued. Terminal failures such as oversized or invalid text can be processed repeatedly, potentially starving other work.
Fix: Treat current-version terminal failures as satisfied and clear their queue entries; requeue only missing or stale results.
internal/processing/source_metadata.go:174 — List-valued metadata does not enforce the canonical codec’s per-value or aggregate size limits. Oversized fields may extract successfully but fail canonicalization indefinitely during backfill.
Fix: Enforce per-value and cumulative encoded-size limits, omitting excess values with a warning.
internal/processing/source_metadata.go:337 — XML character data is copied into every open ancestor buffer without nesting-depth or cumulative-text limits. Deeply nested XML can amplify bounded input into excessive CPU and memory use.
Fix: Bound nesting and collected text, or collect only recognized leaf content and stop with a warning when limits are exceeded.
cmd/docbank/daemon.go:153 — The rendition worker never starts because a fresh runtime registry is checked with Ready() before any production adapter is registered. Register or inject configured provider adapters before checking readiness and starting the worker.
internal/store/schema.sql:778 — Build uniqueness omits execution_identity_fingerprint, causing valid jobs with different output-affecting execution identities to collide during staging. Persist the fingerprint and include it in build uniqueness, serialization, normalization, and validation.
Medium
document/bridge/client.go:124 — Bridge operations can continue after authorization expires because the operation context is bounded only by totalTimeout. Bound it by the earliest caller deadline, total timeout, and Authorization.ExpiresAt; recheck expiry before every upload, poll, artifact, and credential-bearing request.
The change has three medium-severity issues that should be addressed before merge.
Medium
cmd/docbank/daemon.go:153 — The daemon checks Ready() on an empty rendition runtime registry, but no production path registers provider adapters. The rendition worker therefore never starts, leaving queued or restored jobs unprocessed. Register configured provider runtimes before the readiness check and add daemon-level coverage proving a configured rendition job executes.
cmd/docbank/backfill_retry.go:38 — Expired retry entries cause waitDelay to return zero. If a target becomes ineligible after deletion or pruning, it is neither returned by the target query nor removed from the retry map, causing continuous SQLite queries; checksum backfills may also never complete. Reconcile stale retry entries after each complete scan and retain a nonzero fallback delay.
internal/processing/source_metadata.go:321 — XML character data is copied into every open ancestor without nesting-depth or accumulated-text limits. Deeply nested OOXML properties or PDF XMP documents can cause quadratic memory growth and potentially terminate the daemon. Collect text only for relevant elements and enforce strict depth and accumulated-text limits.
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
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.
What changed
Rendition jobs now survive daemon restarts and short-lived catalog failures without resubmitting provider work or publishing half a build. One fenced job owns each source/profile build, authorized waiters attach to it, and provider handles plus sealed execution authority survive backup and restore.
If a provider call is ambiguous and no handle was checkpointed, the job stops for operator resolution instead of guessing. Previous rendition and lexical heads remain active until every authorized attachment can publish atomically. Purge, prune, garbage collection, and the daemon operation gate now respect the same job lifecycle.
Why
Hosted conversion can be expensive and non-idempotent after an uncertain response. Restarting or retrying must resume the known job when possible, never duplicate a paid submission, and never move serving authority to an incomplete build.
Usage
The process-local
RenditionRuntimeRegistryis the integration seam: an application runtime registers exact descriptor fingerprints before the daemon worker can start, and matching authorized requests then join or resume the fenced job.The registry is intentionally empty in this tranche. S1–S3 add provider construction, registration, enqueue, operator-resolution, API, and CLI surfaces; until then the worker gate stays closed.
Part of #176 (R10). Stacks on #201.