Skip to content

Make rendition jobs resumable and publication-fenced - #202

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/rendition-jobs
Open

Make rendition jobs resumable and publication-fenced#202
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/rendition-jobs

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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 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.

Part of #176 (R10). Stacks on #201.

@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (216957c)

Verdict: Changes requested — one high-severity crash risk and five medium-severity reliability or authorization issues remain.

High

  • XML parsing can panic on nested documentsinternal/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 startscmd/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 submissionsdocument/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 failuredocument/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 workinternal/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 expirydocument/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.


Reviewers: 2 done | Synthesis: codex, 17s | Total: 24m18s

@salmonumbrella salmonumbrella changed the title feat: orchestrate resumable rendition jobs Make rendition jobs resumable and publication-fenced Aug 25, 2026
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (dcbd64f)

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.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 17m58s

@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (699a90c)

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.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 16m54s

@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (9bdfba3)

High

  • 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.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 11m37s

@salmonumbrella
salmonumbrella force-pushed the feat/rendition-jobs branch 6 times, most recently from f9b75f5 to d1a6542 Compare August 25, 2026 12:51
@roborev-ci

roborev-ci Bot commented Aug 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (9719e5f)

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.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 21m57s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant